Guard every async socket handler against an escaping rejection

scripts/ci-test-plan.js

@@ -226,6 +226,12 @@ const structuralTestsFor = (changedFiles, trackedSet) => {
   if (changedFiles.some((path) => /^server\/lib\//.test(path))) {
     add('server/lib/index.test.js');
   }
+  // The socket guard readdir-scans server/sockets/ rather than importing it, so
+  // no import edge reaches it — a handler added there would otherwise only be
+  // checked on a full suite.
+  if (changedFiles.some((path) => /^server\/sockets\//.test(path))) {
+    add('server/sockets/asyncHandlerGuard.test.js');
+  }
   if (changedFiles.some((path) => /^client\/src\/lib\//.test(path))) {
     add('client/src/lib/index.test.js');
   }

server/AGENTS.md

@@ -15,6 +15,10 @@ These apply to Express/service/model code. Universal constraints (no try/catch o
 
 - **An `async` timer callback must own its rejections.** Inside `setTimeout(async …)` / `setInterval(async …)`, every `await` needs either an enclosing `try { … } catch` or a `.catch(…)` on the awaited chain. A timer fires outside the request lifecycle, so nothing holds the promise the callback returns and a rejection escapes as an unhandled rejection — fatal on Node >= 15. The CoS runner shipped that at boot: its orphan-cleanup timer awaited a state-file read/write, so one truncated `agents` file killed the runner seconds after start and PM2 restart-looped it against the same file (#5668). Log and continue in the catch unless the callback genuinely owns a failure verdict. `server/timerCallbackConventions.test.js` scans the whole server tree for this and carries no allowlist; the worked example is the watchdog in `server/services/mediaJobQueue/index.js`.
 
+## Socket handlers
+
+- **An `async` Socket.IO handler must own its rejections.** Same rule as the timer callbacks above, same reason: Socket.IO hands the promise a listener returns to nobody, so a rejection escapes as an unhandled rejection and Node terminates the process — taking every agent run, PTY session and media job with it. Every `await` in an `async` handler under `server/sockets/` needs an enclosing `try { … } catch` (the shape `voice.js` uses on whole handler bodies) or a `.catch(…)` on the awaited chain (the shape `logs.js` uses on the lookups it falls back from). `server/sockets/asyncHandlerGuard.test.js` scans the directory for this and carries no allowlist; it names the file and the event string of any offender. Precedent: `voice:call:detach` shipped unguarded (#5661).
+
 ## Tests and peer fan-out
 
 - **Record-creating tests and peer fan-out.** Record create/update paths reach peer-sync only through the subscription adapter in `server/services/sharing/recordEvents.js` (`autoSubscribeRecordToAllPeers` etc.) — a silent no-op until `peerSync.js` registers the real implementation at module load. A suite that never loads `peerSync.js` therefore gets no fan-out without mocking anything. Suites that DO load the peer-sync graph (importing `peerSync.js` or `sharing/index.js`, directly or transitively) must still mock `services/instances.js` with `mockNoPeers()` from `server/lib/mockPathsDataRoot.js` and `services/sharing/peerSync.js` with `mockNoPeerSync()` — the mock keeps the real module's registration side effect from wiring live fan-out into the adapter. To assert auto-subscribe behavior in a domain test, register a test double via `registerSubscriptionAdapter({...})` and clean up with `__resetSubscriptionAdapter()`.

server/lib/README.md

@@ -434,6 +434,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub
 | `apiAccessPolicy.js` | Shared always-public path and gated non-`/api` prefix policy consumed by both `authGate` and API discovery. |
 | `apiCatalog.js` | Searchable projection of the generated Express route manifest: domain, access, side-effect, contract coverage, summaries, and Express-to-OpenAPI path conversion. |
 | `socketEventCatalog.js` | Searchable projection of the cached Socket.IO inventory: direction, domain, and runtime-schema coverage. |
+| `sourceScan.js` | Lexer-assisted primitives shared by the whole-tree source-scan guard suites, so the timer rule and the socket rule cannot drift on what "owns its rejection" means. `blankLiterals(src)` blanks comment/string/template/regex CONTENT to spaces while preserving length, so a brace inside a literal cannot skew a bracket walk and a caller can still read a literal (a socket event name) out of the original string at the same offset; `matchBracket(src, open)` returns the index past the matching `)`/`]`/`}`; `parseCallbackAt(blanked, from, limit)` parses the function expression at `from` (`async`/`function`/either arrow spelling, skipping the parameter list as a unit so a `= {}` default is not mistaken for the body) and returns `{isAsync, start, text}`; `unguardedAwaits(body)` returns the awaited chains that neither sit inside a `try`/`catch` nor END in `.catch(…)`. `blankComments(src)` is the weaker LINE-based stripper the per-line `child_process` rule needs. Callers: `childProcess.guards.test.js`, `server/timerCallbackConventions.test.js`, `server/sockets/asyncHandlerGuard.test.js`. |
 | `apiOperationContracts.js` | Detailed operation metadata for intentionally public APIs. It consumes the canonical route Zod contracts and feeds both public and internal OpenAPI documents. |
 | `apiRegistry.js` | Single source of truth for which PortOS services are externally-callable HTTP APIs (`voice`, `sdapi`). `API_REGISTRY` declares each API's `publicPrefixes` (read/compute-safe surface only) + defaults; `isRegistryPublic(settings, path)` tells `authGate` when an `exposed && !requireAuth` API re-opens its prefix; `resolveApiAccess(settings)` merges persisted `apiAccess` flags for the Settings UI + OpenAPI docs. |
 | `arrayUtils.js` | `shuffle(arr)` — Fisher-Yates shuffle (new array, never mutates). The canonical uniform shuffle — never `arr.sort(() => Math.random() - 0.5)`, which is biased. Shared by `meatspacePostCognitive.js` (Schulte table / mental rotation) and `meatspacePostMemory.js` (memory drill generators). |

server/lib/childProcess.guards.test.js

@@ -1,6 +1,7 @@
 import { describe, it, expect } from 'vitest';
 import { join } from 'node:path';
 import { collectServerSources, readServerSource, SERVER_DIR } from './testHelper.js';
+import { blankComments } from './sourceScan.js';
 
 // Windows hands a newly allocated console off to Windows Terminal when a
 // console-less parent (every PM2 fork, which is all of PortOS) spawns a console
@@ -30,20 +31,6 @@ const SIBLING_PACKAGES = ['../autofixer', '../browser'];
 
 const SPAWN_FNS = ['spawn', 'spawnSync', 'fork', 'exec', 'execSync', 'execFile', 'execFileSync'];
 
-/**
- * Blank comment lines while preserving line count, so a rule can be *described*
- * in a comment without the guard flagging the description as a violation
- * (`cosHealthMonitor.js` explains the pm2 rule using the banned pattern).
- * Line-based on purpose: every real `child_process` mention outside the wrapper
- * is a JSDoc `@param {import('child_process').ChildProcess}` line, and a false
- * positive here is loud and one line to fix.
- * @param {string} src
- * @returns {string[]}
- */
-function blankComments(src) {
-  return src.split('\n').map((line) => (/^\s*(\/\/|\*|\/\*)/.test(line) ? '' : line));
-}
-
 /**
  * Extract whole call expressions by name, brace-balanced so a call wrapped
  * across several lines is captured entire. A per-line scan misses exactly the

server/lib/index.js

@@ -409,6 +409,7 @@ export * from './viteAllowedHosts.js';
 export * from './apiAccessPolicy.js';
 export * from './apiCatalog.js';
 export * from './socketEventCatalog.js';
+export * from './sourceScan.js';
 export * from './apiOperationContracts.js';
 export * from './apiRegistry.js';
 export * from './arrayUtils.js';

server/lib/sourceScan.js

@@ -0,0 +1,312 @@
+/**
+ * Text-level primitives shared by the whole-tree source-scan guard suites —
+ * `childProcess.guards.test.js`, `timerCallbackConventions.test.js` and
+ * `sockets/asyncHandlerGuard.test.js`.
+ *
+ * Those guards enforce structural rules no runtime test can reach ("every spawn
+ * goes through the wrapper", "every await outside the request lifecycle owns its
+ * rejection"), so each one reads source as text and each one needs the same
+ * three things: literals and comments neutralised, so a rule *described* in
+ * prose can neither satisfy nor defeat the scan and a `}` inside a string cannot
+ * skew a brace walk; a bracket matcher, so a construct split across lines is
+ * captured WHOLE rather than by a fixed character window (a window reads the
+ * next statement and attributes it to the current one); and one definition of
+ * "this await owns its rejection", so the timer rule and the socket rule cannot
+ * drift apart.
+ *
+ * Lexer-assisted, not an AST pass: a real parser would be more precise, but
+ * these guards run over ~1500 files on every `npm test`, and their false
+ * positives are loud and one line to fix. The limits are listed on
+ * `unguardedAwaits` and in `timerCallbackConventions.test.js`'s header.
+ */
+
+/**
+ * Blank comment LINES while preserving line count, so a rule can be *described*
+ * in a comment without the guard flagging the description as a violation
+ * (`cosHealthMonitor.js` explains the pm2 rule using the banned pattern).
+ *
+ * Line-based, and so weaker than `blankLiterals` — it is the right tool only for
+ * a rule matched per line, where every real hit outside the wrapper is a JSDoc
+ * `@param {import('child_process').ChildProcess}` line and a false positive is
+ * loud and one line to fix.
+ * @param {string} src
+ * @returns {string[]} one entry per input line, comment lines replaced by ''
+ */
+export function blankComments(src) {
+  return src.split('\n').map((line) => (/^\s*(\/\/|\*|\/\*)/.test(line) ? '' : line));
+}
+
+/**
+ * True when a `/` at this position opens a regex literal rather than division.
+ * A regex can only follow a position where an operand cannot: an operator, an
+ * opening bracket, a statement boundary, or the start of the file.
+ */
+function regexCanStartAfter(prev) {
+  return prev === '' || '(,=:[!&|?{};+-*%~^<>'.includes(prev);
+}
+
+/**
+ * Replace the contents of comments, string/template literals, and regex
+ * literals with spaces, preserving LENGTH so every index still maps back to the
+ * original source. Everything downstream (bracket walks, `await` matching) then
+ * sees code only — and a caller that needs a literal's text (a socket event
+ * name, say) can still read it out of the original string at the same offset.
+ * @param {string} src
+ * @returns {string} same length as `src`
+ */
+export function blankLiterals(src) {
+  const out = src.split('');
+  const n = src.length;
+  const blank = (i) => { if (i < n && src[i] !== '\n') out[i] = ' '; };
+  // Brace depths of the code regions opened by `${` inside template literals,
+  // so a nested template resumes correctly at its closing `}`.
+  const templateStack = [];
+  let inTemplate = false;
+  let braceDepth = 0;
+  // A `/` starts a regex only where a value cannot precede it. Tracking the last
+  // significant character is the standard heuristic and is what keeps a
+  // character class like /["']/ from being read as a string opener.
+  let prevSignificant = '';
+  let i = 0;
+
+  while (i < n) {
+    const c = src[i];
+
+    if (inTemplate) {
+      if (c === '\\') { blank(i); blank(i + 1); i += 2; continue; }
+      if (c === '`') { blank(i); i += 1; inTemplate = false; prevSignificant = '`'; continue; }
+      if (c === '$' && src[i + 1] === '{') {
+        blank(i); blank(i + 1); i += 2;
+        templateStack.push(braceDepth);
+        braceDepth = 0;
+        inTemplate = false;
+        prevSignificant = '{';
+        continue;
+      }
+      blank(i); i += 1; continue;
+    }
+
+    if (c === '/' && src[i + 1] === '/') {
+      while (i < n && src[i] !== '\n') { blank(i); i += 1; }
+      continue;
+    }
+    if (c === '/' && src[i + 1] === '*') {
+      while (i < n && !(src[i] === '*' && src[i + 1] === '/')) { blank(i); i += 1; }
+      blank(i); blank(i + 1); i += 2;
+      continue;
+    }
+    if (c === "'" || c === '"') {
+      const quote = c;
+      blank(i); i += 1;
+      while (i < n && src[i] !== quote) {
+        if (src[i] === '\\') { blank(i); i += 1; }
+        blank(i); i += 1;
+      }
+      blank(i); i += 1;
+      prevSignificant = quote;
+      continue;
+    }
+    if (c === '`') { blank(i); i += 1; inTemplate = true; continue; }
+    if (c === '/' && regexCanStartAfter(prevSignificant)) {
+      blank(i); i += 1;
+      let inClass = false;
+      while (i < n && src[i] !== '\n') {
+        if (src[i] === '\\') { blank(i); blank(i + 1); i += 2; continue; }
+        if (src[i] === '[') inClass = true;
+        else if (src[i] === ']') inClass = false;
+        else if (src[i] === '/' && !inClass) break;
+        blank(i); i += 1;
+      }
+      blank(i); i += 1;
+      // Blank the flags too so `gi` can't be read as an identifier.
+      while (i < n && /[a-z]/.test(src[i])) { blank(i); i += 1; }
+      prevSignificant = ')';
+      continue;
+    }
+
+    if (c === '{') braceDepth += 1;
+    else if (c === '}') {
+      if (braceDepth === 0 && templateStack.length > 0) {
+        blank(i); i += 1;
+        braceDepth = templateStack.pop();
+        inTemplate = true;
+        continue;
+      }
+      braceDepth -= 1;
+    }
+    if (!/\s/.test(c)) prevSignificant = c;
+    i += 1;
+  }
+
+  return out.join('');
+}
+
+/**
+ * Index just PAST the bracket matching the `(`, `[` or `{` at `open`, or -1.
+ * Assumes `src` has been through `blankLiterals`, so a bracket inside a string
+ * or comment cannot unbalance the walk.
+ * @param {string} src
+ * @param {number} open
+ * @returns {number}
+ */
+export function matchBracket(src, open) {
+  const pairs = { '(': ')', '[': ']', '{': '}' };
+  const close = pairs[src[open]];
+  if (!close) return -1;
+  let depth = 0;
+  for (let i = open; i < src.length; i += 1) {
+    if (src[i] === src[open]) depth += 1;
+    else if (src[i] === close) {
+      depth -= 1;
+      if (depth === 0) return i + 1;
+    }
+  }
+  return -1;
+}
+
+/**
+ * Parse the function expression starting at `from` in `blanked` (already through
+ * `blankLiterals`), bounded by `limit`. Handles `async`, `function`, and both
+ * arrow spellings — parenthesized and bare-identifier parameters.
+ *
+ * The parameter list is skipped as a UNIT rather than searching for the first
+ * `{`, because `async (payload = {}) => {` — the shape half of `sockets/voice.js`
+ * uses — puts an object literal in a default value before the body ever opens.
+ *
+ * A concise arrow body (`async () => save()`) has no braces; everything up to
+ * `limit` is taken as the body, so an `await` in it is still seen.
+ * @param {string} blanked
+ * @param {number} from
+ * @param {number} limit - exclusive upper bound (the enclosing call's `)`)
+ * @returns {{isAsync: boolean, start: number, text: string}|null} null when the
+ *   argument is not a function expression (an identifier reference, say)
+ */
+export function parseCallbackAt(blanked, from, limit) {
+  let i = from;
+  const skipSpace = () => { while (i < limit && /\s/.test(blanked[i])) i += 1; };
+  skipSpace();
+
+  const isAsync = /^async[\s(]/.test(blanked.slice(i, limit));
+  if (isAsync) { i += 'async'.length; skipSpace(); }
+
+  if (blanked.startsWith('function', i)) {
+    i += 'function'.length;
+    skipSpace();
+    // Optional name.
+    while (i < limit && /[\w$]/.test(blanked[i])) i += 1;
+    skipSpace();
+    // Parameter list. Only the `function` spelling reaches this — an arrow's
+    // params are consumed below, and skipping a second `(` after its `=>` would
+    // step straight over a parenthesized concise body (`async () => (await f())`),
+    // hiding every await in it.
+    if (blanked[i] === '(') i = matchBracket(blanked, i);
+    if (i === -1) return null;
+    skipSpace();
+  } else {
+    // Arrow: parenthesized params, or a single bare identifier.
+    if (blanked[i] === '(') i = matchBracket(blanked, i);
+    else while (i < limit && /[\w$]/.test(blanked[i])) i += 1;
+    if (i === -1) return null;
+    skipSpace();
+    if (!blanked.startsWith('=>', i)) return null;
+    i += 2;
+    skipSpace();
+  }
+
+  if (blanked[i] === '{') {
+    const end = matchBracket(blanked, i);
+    if (end === -1) return null;
+    return { isAsync, start: i, text: blanked.slice(i, end) };
+  }
+  return { isAsync, start: i, text: blanked.slice(i, limit) };
+}
+
+/**
+ * `[start, end)` spans of every `try { … }` block in `body` that actually has a
+ * `catch` clause. A `try … finally` with no `catch` runs its cleanup and then
+ * re-throws, so it does NOT own the rejection — counting it would let the exact
+ * bug these guards exist for through.
+ */
+function tryBlockSpans(body) {
+  const spans = [];
+  for (const match of body.matchAll(/\btry\s*\{/g)) {
+    const open = body.indexOf('{', match.index);
+    const end = matchBracket(body, open);
+    // Comments between `}` and `catch` are already blanked to spaces.
+    if (end !== -1 && /^\s*catch\b/.test(body.slice(end))) spans.push([open, end]);
+  }
+  return spans;
+}
+
+/**
+ * The member/call chain awaited at `start` — e.g. for
+ * `await pm2.restart(name)\n  .catch(err => …)` it returns the whole thing,
+ * newline continuation included, so a trailing `.catch(` is visible.
+ */
+function awaitedChain(body, start) {
+  let i = start;
+  while (i < body.length && /\s/.test(body[i])) i += 1;
+  const begin = i;
+  while (i < body.length) {
+    const c = body[i];
+    if (/[\w$.?]/.test(c)) { i += 1; continue; }
+    if (c === '(' || c === '[') {
+      const end = matchBracket(body, i);
+      if (end === -1) break;
+      i = end;
+      continue;
+    }
+    if (/\s/.test(c)) {
+      // Only a `.` continuation may follow whitespace; anything else ends the chain.
+      let j = i;
+      while (j < body.length && /\s/.test(body[j])) j += 1;
+      if (body[j] === '.') { i = j; continue; }
+      break;
+    }
+    break;
+  }
+  return body.slice(begin, i);
+}
+
+/**
+ * True when the LAST link of an awaited chain is `.catch(…)`. It has to be the
+ * last one: `await work().catch(recover).then(rethrow)` handles a rejection from
+ * `work()` and then hands the awaited promise straight back to `then`, so the
+ * chain as a whole can still reject.
+ */
+function chainEndsInCatch(chain) {
+  const at = chain.lastIndexOf('.catch');
+  if (at === -1) return false;
+  let i = at + '.catch'.length;
+  while (i < chain.length && /\s/.test(chain[i])) i += 1;
+  if (chain[i] !== '(') return false;
+  const end = matchBracket(chain, i);
+  return end !== -1 && chain.slice(end).trim() === '';
+}
+
+/**
+ * Awaits in `body` (already through `blankLiterals`) that neither sit inside a
+ * `try`/`catch` nor end in `.catch(…)`. Returns the offending chain text for
+ * each, so a failure message points at the expression rather than at a line
+ * number that rebases away.
+ *
+ * Deliberately lexical rather than scope-aware: an `await` inside a nested
+ * `async` callback declared within the body is attributed to the body. That
+ * errs toward flagging — a fire-and-forget inner async callback has the same
+ * ownerless-rejection problem — but it means the fix may belong on the inner
+ * function. An await guarded by a helper the body calls, rather than by its own
+ * `try`, also reads as unguarded.
+ * @param {string} body
+ * @returns {string[]}
+ */
+export function unguardedAwaits(body) {
+  const spans = tryBlockSpans(body);
+  const offenders = [];
+  for (const match of body.matchAll(/\bawait\b/g)) {
+    if (spans.some(([from, to]) => match.index > from && match.index < to)) continue;
+    const chain = awaitedChain(body, match.index + match[0].length);
+    if (chainEndsInCatch(chain)) continue;
+    offenders.push(`await ${chain.replace(/\s+/g, ' ').trim()}`);
+  }
+  return offenders;
+}