Repository navigation
Serializer convergence: route mailbox write edge through submitToSession (serialize gated deliveries vs interrupt/escape) #1365
Description
Activity
On it! Working on this with the PIR protocol (plan + dev-approval gates before PR).
- added a commit that references this issue
on Aug 17, 2026 Architect plan review — 3-way consultation on the PIR plan (pre-gate)
Verdicts: gemini APPROVE · codex REQUEST_CHANGES · claude REQUEST_CHANGES (all HIGH confidence). Both REQUEST_CHANGES reviews ratify Part 1 — the evaluation is verified correct end-to-end (the architect independently confirmed the Ordering-2 mechanics against
message-write.ts/mailbox-delivery.ts:458,463) and CONVERGE is the right call. The changes requested are all in Part 2's design. Consolidated, deduplicated:Blocking (revise the plan before the gate)
- In-lock precheck must re-check the row's own status (codex + claude). A row can be dismissed/superseded while the delivery waits on the terminal lock; the proposed precheck re-validates only writability + ringToken, widening the dismiss→bytes-on-wire window from zero to the lock wait. Add the
getByIdstatus re-check to the precheck, and decide whatWriteResultexpresses for "stale row, no hold". - Head-of-line blocking is a real liveness regression as designed (codex + claude).
MailboxDrainer.tickawaits agents sequentially (mailbox-delivery.ts:644-664), so ONE agent blocked on a terminal lock stalls every other agent's deliveries, escalation and pruning; and worst-case lock hold is(lines−1)×10+80ms — seconds for a newline-heavy body (HTTP accepts 1 MiB), not "~50–130 ms". Claude's proposed remedy fits the architecture: asymmetric try-lock — the delivery path fail-fasts toaborted:'busy'on a contended terminal (it would have aborted at the precheck anyway; the backstop re-delivers), while interrupt/escape continue to block. Same guarantee, no stall. Adopt it or answer it. - State the echo-lag residual instead of overclaiming (codex + claude).
ringToken/bytesWrittentrack output; the in-lock precheck cannot see an interrupt's just-written, not-yet-echoed input. The plan's "the in-lock precheck is not a refinement — it is the fix" overclaims: serialization is the structural guarantee; the precheck narrows (not closes) the residual, which remains Render gate: fuller close of the gate→write input race (R7 staleness) and input-echo-lag residual #1473's territory. Write it that way, including in thesession-submit.tsboundary comment and the arch.md note. --escapeis a second instance of the same bug (claude): its +50 ms Enter can submit a truncated in-flight multi-line delivery. Fold escape into Q1/Q3 and the test matrix as a first-class case, not only a "preserved semantics" check.- Test-fake id hazard (claude):
tower-routes.test.ts'sgateSession()(~line 221) is un-annotated and reaches the realmailbox-wiring— a missingidcompiles and keys every lock onundefined, i.e. a silently global lock. Add that file to the change list, a runtime id guard insubmitMessagePaced, and a "deliveries to different terminals do not serialize" test.
Non-blocking correctness notes for the revision
- The "injected clock" as written cannot assert Enter-before-resolve ordering (pacing uses raw
setTimeout) — use fake timers or thread the clock throughsubmitMessagePaced. - File-list fixes:
pty-session.tslives undersrc/terminal/; arch.md §7 item 5 is ~line 1798;send-delivery.test.tsalso carries inlineports.writeMessageoverrides at :422/:604/:618. - Consider a hot-tier
arch-critical.mdfact for the agent→terminal lock-order invariant once landed.
Full review texts available from the architect on request. Revise Part 2, then re-request plan-approval — Part 1 can be treated as ratified-by-review pending the human gate.
- In-lock precheck must re-check the row's own status (codex + claude). A row can be dismissed/superseded while the delivery waits on the terminal lock; the proposed precheck re-validates only writability + ringToken, widening the dismiss→bytes-on-wire window from zero to the lock wait. Add the
- added 14 commits that reference this issue
on Aug 17, 2026 - added a commit that references this issue
on Sep 6, 2026
Summary
The gated mailbox delivery path and the
escape/interruptsubmission path take two disjoint locks, so a gated mailbox delivery can interleave with a concurrent--interrupt/--escapewrite to the same terminal. This is a known, deliberately-accepted boundary today (a gated delivery only ever writes onto a render-verified empty prompt, andinterrupt/escapeare explicit human gate-bypasses), but it is worth converging so the write edge is serialized end-to-end.Split off from PR #1330 (Spec 1313 maintainer round) as agreed — not in scope for that PR.
Detail
write-queue.ts, keyed byagentKey; completion-chains a message's text + Enter). Two mailbox deliveries to one agent cannot interleave.escape/interruptsubmissions take a per-terminal submission lock (submitToSession,session-submit.ts).escape/interruptto the same terminal.The boundary is already flagged in-code:
packages/codev/src/agent-farm/servers/session-submit.ts(the "Exactly what it covers — this is NOT blanket per-session atomicity" comment) documents that the mailbox delivery path is deliberately not covered by the submission lock, and why the residual cross-path race is accepted rather than a regression.packages/codev/src/agent-farm/servers/tower-routes.ts(the immediateescape/interruptwrite path, incl. the non-writable-sessionTERMINAL_NOT_WRITABLE503 handling).Proposed fix
Route the mailbox write edge through
submitToSessionso a gated delivery and anescape/interruptsubmission take the same per-terminal lock. The per-terminal lock would be acquired as a leaf inside the per-agent serializer, so there is no lock-cycle hazard (the ordering is always per-agent → per-terminal, never the reverse).Notes
interrupt/escapeare explicit operator actions — so the practical corruption surface is small. This is a robustness/convergence cleanup ("one mechanism, not two"), not a live corruption bug.Evaluate first (absorbed from #1480, 2026-08-17)
Before landing the routing change above, evaluate — end to end — how the three write paths compose, and let that evaluation ratify or supersede this issue's remedy. The interaction is currently a patchwork of separately-reasoned decisions rather than one coherent, documented model:
--interrupt/--escapetakes the per-terminal submission lock (session-submit.ts) and keeps its documented "claim-first" tradeoff.write-queue.ts, keyed byagentKey) — the accepted boundary documented insession-submit.ts's "Exactly what it covers" comment.--interrupt(Spec 1313 maintainer round): the timer fires only the Ctrl+C at due time; the body delivers through the normal gated drainer after the turn ends.Specific failure questions to answer:
Outcomes: if the evaluation ratifies convergence, implement as proposed above. If it concludes the accepted boundary is correct as-is, close this issue as wontfix with the analysis attached. Either way, land the conclusion as an updated
session-submit.tsboundary comment + arch.md §mailbox note, so the model lives in one place.Interlock: #1481 (
--interrupt-after) must build on whatever model this settles — sequence it after. Tracked by #1483 (workstream B).