[eric] ws: a session id we never issue now names its own producer instead of silently 404ing (ENG-205)

frontend/src/shared/ws/WebSocketManager.ts

@@ -1,4 +1,5 @@
 import { store } from '../state/store';
+import { warnIfNotCanonicalSessionId } from './sessionIdShape';
 import { unstable_batchedUpdates } from 'react-dom';
 import {
   updateSession,
@@ -1020,6 +1021,7 @@ export function seedSessionSeq(sessionId: string, seq: number): void {
 }
 
 export function createSessionWs(sessionId: string): WebSocketManager {
+  warnIfNotCanonicalSessionId(sessionId, 'createSessionWs');
   return new WebSocketManager(`${WS_BASE}/ws/agents/${sessionId}`, { sessionId });
 }
 
@@ -1032,6 +1034,7 @@ export function acquireSessionWs(sessionId: string): WebSocketManager {
     _backgroundedSessionWs = null;
     return ws;
   }
+  warnIfNotCanonicalSessionId(sessionId, 'acquireSessionWs');
   return new WebSocketManager(`${WS_BASE}/ws/agents/${sessionId}`, { sessionId });
 }
 

frontend/src/shared/ws/sessionIdShape.test.ts

@@ -0,0 +1,65 @@
+// Run: npm test
+//
+// ENG-205. A user saw `/api/agents/sessions/ae8813e9d5d20fb7.1 -> 404` plus a WS close. Measured
+// across 248 real sessions: zero ids of that shape, and neither side constructs one. The point of
+// this check is that the NEXT occurrence names its own producer.
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import {
+  isCanonicalSessionId,
+  warnIfNotCanonicalSessionId,
+  resetSessionIdWarnings,
+} from './sessionIdShape.ts';
+
+const REAL = 'ae8813e9d5d20fb7ae8813e9d5d20fb7';       // 32 hex, the shape we issue
+const REPORTED = 'ae8813e9d5d20fb7.1';                  // exactly what the user's console showed
+
+test('a real session id passes', () => {
+  assert.equal(isCanonicalSessionId(REAL), true);
+});
+
+test('the id from the report is rejected', () => {
+  assert.equal(isCanonicalSessionId(REPORTED), false, 'the reported id must be recognised as foreign');
+});
+
+test('the near misses are all caught, since the point is the SHAPE', () => {
+  for (const bad of [
+    'ae8813e9d5d20fb7',                       // 16 hex, half length
+    'ae8813e9d5d20fb7ae8813e9d5d20fb7.1',     // full length plus a suffix
+    'AE8813E9D5D20FB7AE8813E9D5D20FB7',       // uppercase
+    'ae8813e9d5d20fb7ae8813e9d5d20fbg',       // non-hex char
+    '',
+  ]) {
+    assert.equal(isCanonicalSessionId(bad), false, `${JSON.stringify(bad)} should not pass`);
+  }
+});
+
+test('warning fires once per id, so a reconnect loop cannot flood the console', () => {
+  resetSessionIdWarnings();
+  const seen: unknown[][] = [];
+  const orig = console.warn;
+  console.warn = (...a: unknown[]) => { seen.push(a); };
+  try {
+    warnIfNotCanonicalSessionId(REPORTED, 'createSessionWs');
+    warnIfNotCanonicalSessionId(REPORTED, 'createSessionWs');
+    warnIfNotCanonicalSessionId(REPORTED, 'createSessionWs');
+  } finally {
+    console.warn = orig;
+  }
+  assert.equal(seen.length, 1, `warned ${seen.length} times for one id`);
+  assert.match(String(seen[0][0]), /ENG-205/);
+  assert.match(String(seen[0][0]), /never issue/);
+});
+
+test('a good id never warns', () => {
+  resetSessionIdWarnings();
+  let warned = 0;
+  const orig = console.warn;
+  console.warn = () => { warned += 1; };
+  try {
+    assert.equal(warnIfNotCanonicalSessionId(REAL, 'createSessionWs'), true);
+  } finally {
+    console.warn = orig;
+  }
+  assert.equal(warned, 0, 'a legitimate id must be silent, or the warning becomes noise');
+});

frontend/src/shared/ws/sessionIdShape.ts

@@ -0,0 +1,43 @@
+// Is this actually one of our session ids? (ENG-205)
+//
+// A user's console showed `GET /api/agents/sessions/ae8813e9d5d20fb7.1 -> 404` plus a matching
+// "WebSocket closed before the connection was established". That id is 16 hex chars with a dotted
+// suffix; ours are 32 hex chars with no suffix. Measured across 248 real sessions on a dev machine:
+// zero filenames of that shape and zero `sdk_session_id` values containing a dot. Neither the
+// frontend nor the backend constructs one, so the value arrives from somewhere as the sessionId and
+// nothing on the way to the socket would notice.
+//
+// The bug is not the 404, it is that nobody can say who produced the id. So the shape check exists
+// to make the next occurrence name its own producer instead of costing another round of guessing.
+
+/** Our session ids: 32 lowercase hex characters, nothing else. */
+const P_CANONICAL = /^[0-9a-f]{32}$/;
+
+export function isCanonicalSessionId(id: string): boolean {
+  return P_CANONICAL.test(id);
+}
+
+/**
+ * Warn once per offending id, with a stack, when something asks us to open a socket for an id we
+ * could never have issued. Deliberately does NOT refuse: 248 sessions on one machine is not enough
+ * evidence to start dropping connections, and a diagnostic that changes behaviour is a second bug.
+ */
+const p_warned = new Set<string>();
+
+export function warnIfNotCanonicalSessionId(id: string, where: string): boolean {
+  if (isCanonicalSessionId(id)) return true;
+  if (p_warned.has(id)) return false;
+  p_warned.add(id);
+  // eslint-disable-next-line no-console
+  console.warn(
+    `[ENG-205] ${where} received a session id we never issue: ${JSON.stringify(id)} `
+    + `(expected 32 hex chars). The 404 and the WS close that follow are consequences, not the bug. `
+    + `Stack names the producer:`,
+    new Error('session-id origin').stack,
+  );
+  return false;
+}
+
+export function resetSessionIdWarnings(): void {
+  p_warned.clear();
+}