Post-merge review of #358: bound the shape of an echoed refusal, refuse a second meeting out loud, make the zero-failure row fail

[lilseyi]

What this is

A post-merge adversarial review of #358 (f276c58), which was merged by its author while a separate review was in flight. Nothing in #358 should have been reverted — the four things it built are right and its own accounting is honest — but a pre-merge review would have caught four things, and this fixes them forward.

Verdict on #358

• Nothing merged should not have. The chunk-id change is correct and safe against already-stored ids; assertOwnTranscript is placed where it claims to be (first statement in finalize, before the workspace resolve and the render, and the session objects it reads are rebuilt by an immutable fold, so there is no stale value to satisfy it with); the phone's only meeting-note write really is that one finalize; a late segment is folded before end() detaches, so nothing legitimate is refused.
• Degrade-to-bypass: yes, and it is not phone-specific. Every guard here reads what an id says, and an id that says nothing is accepted at every door — the gateway's included. A client is not stopped by any of these by lying; it is not stopped by declining to say. So "a boundary no client can talk past" is exact about misaddressed words and not about unaddressed ones. Argued in the decision doc, including why the acceptance still stays (the marker that would separate old data from a client that stopped addressing is written by the same binary whose honesty is in question, and refusing on it would fail in the one direction the contract forbids).
• Barrier count. Three client-side on the phone, not two — applyMeetingEvent's terminal-state check is one of the four the section above counts for the desktop, and the phone has had it all along. The count was never the point: exactly one guard in this feature is enforced by a party other than the client, and that is the whole gap.
• What the server sees when a phone files a meeting note: a workspace id, a path and a body, through files.writeNote. Nothing else. A rendered note carries meeting-id in frontmatter and turns as **[clock] speaker** — text with no per-row ids, so nothing anywhere — at write time or afterwards — could detect two meetings' words in one note.

The four fixes

1. describeEventType bounded the length and not the shape. The sentence it builds is written to a customer's log file by apps/desktop as the tail of one line (meeting_write_refused session=… kind=… status=… code=…: <sentence>). A newline forges a second line in one character and an escape sequence rewrites the printed one in four; forty characters is no defence against either. The shape is now checked first, the way assertSafeEtag does one folder over. An identifier-shaped type is still named and truncated, because that is the one thing a client author can act on.

2. A second meeting was silently swallowed. MeetingsListScreen hides its record button while a meeting is live; the console's Record key (ConsoleBottomBar) does not, so controller.start() really can be called for a second meeting while the first is recording. Both recorders returned silently from start() and went on minting the first meeting's chunk ids. Before #358 that was contamination with no name on it; since #358 apply correctly refuses every one of those segments and the second meeting records nothing at all and says nothing. The recorder now refuses (ALREADY_RECORDING), which controller.start handles like a denied microphone — notepad keeps working, sentence on the live screen. Same meeting twice is still one start.

3. The zero-failure sabotage row now fails. #358's acceptsTranscript test drives every state the table names, but both sides read the same table, so reverting to the old denylist stayed green — reported honestly as 0. The derivation itself is now pinned: a third terminal state is added to a copy of the table through the module registry and the answer for it has to come out of the table. Reverting the source fails here — 1.

4. Two half-closed residues. The gateway's own mirror of acceptsTranscript (ingest.js naming complete/empty by hand in three places) was named in #358's commit message and not closed; it cannot be derived as cheaply because each refusal carries its own sentence per state, so the coupling is checked instead — the day the table grows a third terminal state the suite says which three functions must learn about it. And MAX_CHUNK_ID_LENGTH's comment in apps/convex still described the ids as <Date.now()>-<index>, which is exactly what #358 stopped being true.

Every remaining subscription in the phone's recording path

Checked, and there is nothing further to report as a leak: controller.listenToRecorder holds both unsubscribes and detachRecorder runs from end(), discard(), reset() and the next attach; audio.ts's openDevice calls releaseDevice first, so the recordingStatusUpdate subscription can never be overwritten while attached; capture/desktop.ts's attach() detaches first and keeps its offs; useAudioLevel and every effect in useMeetings/useMeetingFlow return cleanups. The one place listeners are never explicitly removed is audio.web.ts's watch(track) — three per track — and every path out (stop, abandon, the ended handler, a failed openChunk) calls releaseStream(), which stops the tracks, so they are inert rather than detached. Noted, not changed.

Sabotage, measured

Each guard reverted one at a time on this branch, its file run, then restored and re-verified green.

Guard	Sabotage	FAIL
describeEventType shape check	dropped, length bound kept	2
Terminal-state coupling	a third terminal state added to MEETING_TRANSITIONS	1
ALREADY_RECORDING (audio.ts)	silent return for a different meeting again	1
ALREADY_RECORDING (audio.web.ts)	same	1
Same-meeting no-op (audio.ts)	throw for the same meeting too	1
Same-meeting no-op (audio.web.ts)	same	1
acceptsTranscript derivation	back to the old denylist	1 (was 0)

#358's own seven rows were re-run first on f276c58 and reproduced exactly as reported: 3, 3, 1, 1, 2, 2, and 0.

Verified

pnpm install --frozen-lockfile, then on f276c58 (the merged baseline) and again on this branch after rebasing onto the current main (e47a770, unrelated calendar work, no conflicts):

• apps/mobile — baseline 220 suites / 4078 passed / 1 todo; this branch 221 suites / 4103 passed / 1 todo. No flake in any run here (the suite is separately known to have one editor test that flakes only under parallel load).
• apps/mcp — ALL PASS, 3073 → 3077 PASS lines.
• apps/desktop — ALL PASS, 1716 PASS lines, unchanged.
• packages/meetings — ALL PASS, 778 PASS lines, unchanged.
• apps/convex — 70 files, 2194 passed (one comment touched there).
• turbo run typecheck — 12/12 clean.
• CI on the rebased head: 24 success, 2 skipped, 0 failed.
• One local-only note: turbo run lint fails @context/shared#lint (eslint . → "No files matching the pattern"). packages/shared is byte-identical to main on this branch, and CI's own Lint job is green, so it is pre-existing and untouched here.

🤖 Generated with Claude Code

https://claude.ai/code/session_011Ht7GUWMd9t4xDNED9NDEz