[Spec 1313] afx send: mailbox-first delivery (never force-inject) by mohidmakhdoomi · Pull Request #1330 · cluesmith/codev

mohidmakhdoomi · 2026-08-01T17:56:50Z

Summary

Replaces afx send's timer-based, in-memory, force-flushing delivery (SendBuffer, Spec 403) with a mailbox-first pipeline. Every message is persisted to global.db before the send returns, and its body is only ever written to a prompt a headless-terminal render-gate proves is empty. Corruption is eliminated by construction — a message can never fuse with a draft, menu, dialog, or wrapper screen — and there is no force path: no timeout, valve, or fallback ever writes onto a non-clean screen. A busy line means a human is present; the response is to escalate visibility (indicator + afx inbox), never to fight for the keyboard.

Closes #1313

Changes (9 phases, single PR)

Mailbox store — durable mailbox table (migration v15, agent-addressed rows); kills silent loss.
Render-empty gate — render-gate.ts replays the output ring through @xterm/headless; claude/codex profiles. Idle-gate cost well under the ≤ ~50ms budget.
agy profile (blocking) — net-new empirical measurement; agy's normal-intensity gray hint needed a color-keyed placeholderFgPalette rule (dim/bold couldn't separate idle from draft). Trust dialog classifies not-clean (no blind Enter confirms filesystem trust).
Delivery orchestration — handleSend rewrite: persist → serialize → gate → deliver/hold. Retires SendBuffer and every force path. Per-PTY write serialization (write-queue.ts). Response vocab delivered | held+reason (busy/no-profile/no-live-pty), surfaced client-side (single + --all).
Fast delivery triggers — submit + output-quiescence triggers via a single handleUserInput chokepoint on PtySession; held mail delivers near-immediately once the line clears.
Cron rerouting — the most-unguarded writer joins the one gated path; per-task supersede; honest run-log outcomes.
afx inbox + broadcasts + escalation — CLI (list/show/dismiss, workspace-scoped), overview-changed held-count SSE, mailbox-escalation visibility event (never delivery), liveness telemetry.
Dashboard + VSCode indicators — count-only held indicators + attention state on escalation (read-only; dismissal stays CLI-only).
Docs + skeleton mirror — send vocab, afx inbox, mailbox config; CLAUDE.md/AGENTS.md byte-identical; skeleton mirrored.

Post-gate hardening (folded into this PR)

After the first pr-gate approval, live testing on installed code surfaced real defects; the architect authorized a verify→implement rollback to fold the fixes into this PR (7 additional CMAP rounds):

Architect-identity resolution — afx send architect was always held(no-profile) (architect sessions had no persisted command). Fixed with a restart-safe identity SSOT on the session row (migration v16) + a legacy self-heal that honors TOWER_ARCHITECT_CMD precedence.
Render-gate whole-ring rewrite — the classifier had only ever been validated against a synthesized claude-idle fixture, so real-output false-busy bugs (a bg-task panel displacing the composer boundary; a >1MB ring torn by a fixed tail-slice) shipped latent. It now renders the whole ring at any size (no tail slice, no delivery-blocking cap); an over-ceiling permanent-hold outage was removed; a per-ringToken verdict memo + backstop backoff keep it cheap. Captured real gzipped rings as fixtures.

Testing

Automated e2e for the afx send can corrupt in-progress user input: idle-time is a bad proxy for line-occupancy #1265 repro: draft → held(busy) → submit → clean delivery (subprocess harness).
Unit: mailbox lifecycle, gate classification vs captured fixtures (claude/codex/agy: idle/draft/menu/picker/trust/wrapper), write serialization, inbox routes (incl. 405 method guard), cron supersede, escalation-is-visibility-only, restart-safe identity, whole-ring render vs real captured rings.
Playwright: dashboard held-count indicator (4/4 real chromium, incl. live update).
Last verified green: packages/codev ~4267 passing / 48 skipped; VSCode 667; dashboard 328.
Live verification completed (architect ran the built + installed code): idle→delivered; draft→held(busy)→delivers on clear; bg-task/monitor→delivered (no false-busy); real >1MB rings classify clean; agy trust-dialog held. No held-message regressions.
See the review's Flaky Tests section for an environmental temp-dir/chdir race under concurrent-builder load (not a code defect; green on direct run + retry) and the CI-aware render-gate perf bound.

Notes for reviewers

Phase 7 ended on a porch force-advance at the 3-iteration ceiling: the iter-3 Codex fix (a real POST-only method guard on /api/inbox/:id/dismiss + regression test) landed and Claude approved, but there was no iter-4 re-consult. Flagged honestly in the review; this PR gate is the backstop.
spec-1280 T16 manifest guard was re-scoped by this PR (its unscoped origin/main...HEAD predicate mis-fires on any branch touching a prompt surface after merging main); flagged for the 1280 owner's ack in the review's Technical Debt.

Spec / Plan / Review

Spec: codev/specs/1313-afx-send-mailbox-first-delivery.md
Plan: codev/plans/1313-afx-send-mailbox-first-delivery.md
Review: codev/reviews/1313-afx-send-mailbox-first-delivery.md

🤖 Generated with Claude Code

Draft spec for afx send mailbox-first delivery (never force-inject), triaged against a 3-way consistency review: cron supersede made unconditionally per-task, --interrupt reframed as deliberate sender bypass reconciled with the no-force-path decision, canonical held-reason tokens, enqueue added to delivery moments, ok:true on held, per-PTY write serialization with per-agent enqueue ordering, two named broadcast events, and area/cross-cutting per label policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address 3-way spec consultation (Gemini APPROVE, Codex REQUEST_CHANGES, Claude COMMENT — all HIGH confidence): - Add missing ## Expert Consultation section (unanimous ask) - Decision 8: make afx inbox workspace-scope + dismiss authorization explicit; note attention-state visual is plan-level - Decision 6: state supersede keys are cron-only - Test scenarios: add #16 escalation-age-threshold scenario No baked decisions changed. Also seeds the cohort thread file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Documents resolution of all 3-way feedback (Codex REQUEST_CHANGES fully addressed; Gemini APPROVE; Claude COMMENT). No baked decisions changed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…complete) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

9-phase decomposition: mailbox store → rendered-empty gate (claude/codex) → agy profile (blocking) → delivery orchestration + write serialization → fast delivery triggers → cron rerouting → afx inbox + SSE/escalation → dashboard/VSCode indicators → docs + skeleton mirror. Grounded in real architecture (UIs use SSE/overview, not WebSocket; VSCode badge precedent; migration v15). Offers phase-merge knobs for the gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address 3-way plan consult (Gemini APPROVE, Codex REQUEST_CHANGES, Claude APPROVE — all HIGH): - Phase 4: client-side send contract (tower-client.ts + commands/send.ts), automated e2e for #1265, pruneTerminal invocation, liveness telemetry - Phase 7: name lib/config.ts loader for escalation-age; liveness surfacing - Phase 1: name lib/config.ts for retention-days - Phase 5: drain-coalescing test - Exec summary WS->SSE; optional Phase 7 split offered No phase scope changed. Includes rebuttal + thread. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Address iteration-2 plan consult (Gemini APPROVE, Codex REQUEST_CHANGES, Claude APPROVE — all HIGH; Gemini+Claude verified all iter-1 fixes landed): - Phase 4: dead-session targeting seam (agent-registry fallback in resolveTarget + handleSend restructure to persist no-live-pty instead of 404); --all honest reporting (sendToAll); fix tower-client shape desc - Phase 2: PtySession app-identity seam (command/args private today) for resolveProfile All Codex points verified against code. No scope change. Includes rebuttal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… layer (v15) Phase 1 of mailbox-first delivery: a durable home for every 'afx send' so nothing is lost to a Tower crash/restart/shutdown. Pure, delivery-decoupled data operations that Phase 4 wires against. - schema.ts: additive mailbox table + 3 indexes in GLOBAL_SCHEMA (agent-addressed via to_agent, not PTY-keyed). - index.ts: migration v15 (GLOBAL_CURRENT_VERSION 14->15) creating the table on existing installs; re-export new row types. - types.ts: DbMailbox row + MailboxStatus/MailboxReason unions. - db/mailbox.ts: repository (enqueue/getById/listHeld/findHeldForAgent/markDelivered/dismiss/supersede/pruneTerminal). State machine enforced (transitions touch only held rows); timestamps injectable for tests. - Tests: mailbox.test.ts (lifecycle, per-agent ordering, crash/reopen recovery) and spec-1313-migration.test.ts (v15; fresh vs migrated shapes converge). Refs #1313. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…e/codex profiles Phase 2 of mailbox-first delivery: the sole authority that proves a composer is a clean, empty prompt before any message body is written to it. Corruption is eliminated by construction — a message can never fuse with a draft because it is never delivered while one exists. Unreferenced by any live path until Phase 4. - render-gate.ts: classifyScreen(snapshot, profile) replays a seed-capped ring snapshot (the production reconnect-replay data path) through a transient @xterm/headless terminal and inspects the composer region. CLEAN requires a composer marker AND zero normal-intensity, non-whitespace, non-chrome cells (dim placeholder/hint text is OK — an SGR attribute both TUIs use). Anything unrecognized → not clean → held. No force path. Direct port of the spike-1265 G-lite classifier. - gate-profiles.ts: claude + codex profiles (marker + region-end patterns from spike facts) and strict resolveProfile(identity) → profile | null via detectHarnessFromCommand. Deliberately no claude fallback: agy/gemini/opencode/unknown/wrapped launches resolve to null (held with no-profile), never a guessed profile. - pty-session.ts: app-identity seam — read-only command/launchArgs getters so resolveProfile has an authoritative source instead of guessing from the label. - package.json: promote @xterm/headless (^6.0.0) from spike-only to a dependency. - Fixtures + tests: render-gate.test.ts over REAL captured claude/codex byte streams (idle/draft/menu/picker) plus synthetic branch coverage, a seed-cap performance bound, and strict resolveProfile identity cases. Fixtures README documents provenance (claude-idle synthesized to the spike's real-claude dim-placeholder measurement, since the sandbox claude shim renders the idle placeholder without de-emphasis). Refs #1313. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Refs #1313. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…cker, perf bound, CJS interop) Iteration 2 of Phase 2, addressing the iter-1 3-way review (Gemini + Claude APPROVE, Codex REQUEST_CHANGES): - Codex #1 (missing claude-picker fixture): add a synthesized claude `/model` picker fixture (claude-picker.busy.txt) whose highlighted row starts with the same ❯ glyph as the composer marker, with normal-intensity model names. Pins that a picker's selection-cursor + list classifies busy via the user-text path, never a false-clean; mirrors the real codex-picker capture (`› 1. …`). Wired into the required-states assertion; suite now 23/23. Documented as synthesized in the fixtures README (sandbox claude is the ez-cli shim, same reason as claude-idle). - Codex #2 (perf assertion too loose): replace the single cold-run < 500ms with warm-up + best-of-5 min < 75ms. The min strips JIT/GC/scheduling noise (42.7ms cold vs 14.5ms native steady-state here), so it validates the spec's ≤~50ms seed-cap budget (measured best-of-5 = 19.2ms) instead of flaking. 5x tighter than before; 75ms is the CI-noise ceiling, not a near-budget claim (the logged value is the evidence). - Bonus latent production bug, found while grounding the perf measurement against the compiled dist under native node: @xterm/headless resolves to its CommonJS entry (no exports map / type:module) with non-analyzable named exports, so `import { Terminal }` throws "Named export 'Terminal' not found" under native-node ESM — how the compiled bins run in production. Masked by vitest (vite interop) and dormant until Phase 4 wires the gate. Switch to the default-import form (codebase convention, cf. `import Database from 'better-sqlite3'`) plus a type-only alias for the one type-position use. Refs #1313. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Refs #1313. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…erf, CJS interop) Refs #1313. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… iter 3)

…h + review rewrite

…essons routing Rewrite codev/reviews/1313-afx-send-mailbox-first-delivery.md from scratch off the SPIR review template (the prior file was stale, authored pre-rollback and only half-swept by the post-rollback implement commits). Reconstruct every section against the CURRENT implementation and git history: Summary, Spec Compliance (11/11 SC met), Deviations, Key Metrics, Timelog, Consultation Iteration Summary + full Consultation Feedback for every phase/round/model (cross-checked against the per-iteration evidence files), Lessons, Architecture/Lessons Updates, Technical Debt, Flaky Tests, Follow-ups. Governance routing for the post-rollback render-gate change: - arch.md sec 7 Message Delivery: "seed-capped output ring" -> whole-ring render at any size (over-ceiling hold removed) + ringToken verdict memo + backstop backoff. - lessons-learned.md (cold, Testing): validate a screen/output classifier against REAL captured output, not synthesized fixtures (the render-gate false-busy saga). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…us fields + round-2 record) Fresh 3-way Review consult (round 2, post-rewrite): Gemini APPROVE, Claude APPROVE, Codex COMMENT (non-blocking). Addressed Codex's hygiene comments: - spec Status draft -> specified; plan Status draft -> approved (both already carry approval frontmatter). - Recorded Review round 2 in the review doc's Consultation Feedback + Iteration Summary. PR #1330 body refreshed separately (stale 4162->~4267 test count, agy "deferred"-> live-verified, + the missing post-gate hardening arc). The spec-1280 T16 re-scope stays flagged for its owner; transient consultation artifacts remain deliberately untracked. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…passed) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mohidmakhdoomi · 2026-08-03T07:37:19Z

Architect Integration Review — Changes Requested

3-way integration CMAP on this PR: Gemini APPROVE · Claude COMMENT · Codex REQUEST_CHANGES (HIGH). I verified all three Codex claims against source (Claude and Codex contradicted each other on the TOCTOU point, so I read the code, not the summaries). Net: one blocking item + two cleanups. The gate stays parked — not approved.

🔴 Must fix — dropped PTY write reported as `delivered` (silent loss)

Problem. A message can be marked delivered while zero bytes reached the terminal — the exact silent-loss failure this spec exists to eliminate.

Root cause (verified). PtySession.write() returns false on dropped shellper input (#1198, pty-session.ts:477), but WritableSession.write() is typed : void (message-write.ts:10) — the boolean is discarded. writeMessageToSession fires text/lines/Enter via setTimeout; writeMessagePaced resolves on a pure timer; deliverAgentMail calls markDelivered unconditionally after the await. The !session.writable precheck runs at t=0 while the writes fire 10–130 ms+ later, so a socket that dies in that window (or the #1198 connected-but-dropping state) loses the message silently. The in-code comment says "the spec requires an errored PTY write to leave the row held" — but the implementation doesn't enforce it, and this path is not in the review's disclosed Technical Debt (so it was never a conscious risk-accept).

Fix. Thread the boolean — WritableSession.write(): boolean; surface a failed write out of writeMessagePaced; have deliverAgentMail leave the row held('no-live-pty') (retries next clean pass) instead of markDelivered. Add tests for both the initial write and the delayed Enter/multiline writes.

🟡 Cleanup 1 — vestigial spec-1280 test guard

packages/codev/src/__tests__/spec-1280-phase-manifest.test.ts — the branch-name predicate makes a main-resident guard silently no-op. Spec 1280 is already integrated; delete the vestigial guard (cleaner than scoping). Flagged by both Claude and Codex.

🟡 Cleanup 2 — stale `SendBuffer` comments

packages/codev/src/agent-farm/servers/session-submit.ts (≈ lines 22, 48) still describe SendBuffer / deliverBufferedMessage as current architecture. Update to the mailbox-delivery model. (Codex #3.)

Not required now

Codex #1 — the gate→write input-echo race — is real but already documented in the review's Follow-up Items and architect-ratified. Leave it as the tracked follow-up; do not widen scope here.

After the fix

Update the review doc's Consultation Feedback (record this architect integration round + Codex's silent-loss finding) and Technical Debt, re-verify, and re-park at the pr gate. I'll re-run the integration consult on the fix before we approve. (PR is also still a draft — mark ready only after gate approval.)

Architect integration review

mohidmakhdoomi and others added 30 commits July 31, 2026 21:39

chore(porch): 1313 init spir

b0bc028

chore(porch): 1313 specify build-complete

c726f0e

chore(porch): 1313 spec-approval gate-requested

ba58c14

[Spec 1313] Rebuttal to iteration-1 spec consultation

6697add

Documents resolution of all 3-way feedback (Codex REQUEST_CHANGES fully addressed; Gemini APPROVE; Claude COMMENT). No baked decisions changed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

[Spec 1313] Thread: record spec-approval gate reached (specify phase …

c0ef095

…complete) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

chore(porch): 1313 spec-approval gate-approved

444cd76

chore(porch): 1313 plan phase-transition

44c8a6e

chore(porch): 1313 plan build-complete

89b8753

chore(porch): 1313 plan re-iter (iter 2)

17c83be

chore(porch): 1313 plan build-complete

33aba39

chore(porch): 1313 plan-approval gate-requested

a486cad

[Spec 1313] Thread: plan-approval gate reached (plan phase complete)

56da958

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

[Spec 1313] Plan iter-2 consultation context (porch artifact)

c35142a

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

chore(porch): 1313 plan-approval gate-approved

36582b2

chore(porch): 1313 implement phase-transition

5f9f9b5

[Spec 1313] Thread: Phase 1 (mailbox store) implemented + verified green

d4e291d

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

chore(porch): 1313 implement build-complete

66e35eb

chore(porch): 1313 advance plan phase → phase_2

638d568

[Spec 1313] Thread: Phase 2 (render-gate) committed + verified green

2803f1f

Refs #1313. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

chore(porch): 1313 implement build-complete

0d81411

[Spec 1313] Phase 2 iter-1 rebuttal (porch artifact)

041203d

Refs #1313. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

[Spec 1313] Thread: Phase 2 iter-1 review addressed (claude-picker, p…

661c4bf

…erf, CJS interop) Refs #1313. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

mohidmakhdoomi and others added 29 commits August 3, 2026 01:29

chore(porch): 1313 implement build-complete

9cd20aa

chore(porch): 1313 implement re-iter (iter 2)

f0b451a

chore(porch): 1313 implement build-complete

875baba

chore(porch): 1313 advance plan phase → phase_6

1538ba1

chore(porch): 1313 implement build-complete

1cefe53

chore(porch): 1313 advance plan phase → phase_7

943db48

chore(porch): 1313 implement build-complete

273a0ad

chore(porch): 1313 implement re-iter (iter 2)

78fff81

chore(porch): 1313 implement build-complete

71548cc

chore(porch): 1313 implement re-iter (iter 3)

9913665

chore(porch): 1313 implement build-complete

80333e6

chore(porch): 1313 implement force-advance (safety ceiling reached at…

61680e1

… iter 3)

chore(porch): 1313 advance plan phase → phase_8

3d80922

chore(porch): 1313 implement build-complete

fcf70c7

chore(porch): 1313 implement re-iter (iter 2)

0ee7f52

chore(porch): 1313 implement build-complete

102d928

chore(porch): 1313 advance plan phase → phase_9

5a5bf98

chore(porch): 1313 implement build-complete

af55453

chore(porch): 1313 implement re-iter (iter 2)

7babed7

[Spec 1313][Phase: implement] docs: thread — resume for phase_9 finis…

3697ca7

…h + review rewrite

chore(porch): 1313 implement build-complete

a984a58

chore(porch): 1313 all plan phases complete → review

93246d1

chore(porch): 1313 review build-complete

9a9dc52

chore(porch): 1313 review re-iter (iter 2)

4956aba

chore(porch): 1313 review build-complete

bd371f3

chore(porch): 1313 pr gate-requested

98cb13b

[Spec 1313][Phase: review] docs: thread — STOPPED at pr gate (review …

308f66a

…passed) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Spec 1313] afx send: mailbox-first delivery (never force-inject) - #1330

[Spec 1313] afx send: mailbox-first delivery (never force-inject)#1330
mohidmakhdoomi wants to merge 144 commits into
mainfrom
builder/spir-1313

mohidmakhdoomi commented Aug 1, 2026 •

edited

Loading

Uh oh!

mohidmakhdoomi commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mohidmakhdoomi commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes (9 phases, single PR)

Post-gate hardening (folded into this PR)

Testing

Notes for reviewers

Spec / Plan / Review

Uh oh!

mohidmakhdoomi commented Aug 3, 2026

Architect Integration Review — Changes Requested

🔴 Must fix — dropped PTY write reported as delivered (silent loss)

🟡 Cleanup 1 — vestigial spec-1280 test guard

🟡 Cleanup 2 — stale SendBuffer comments

Not required now

After the fix

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mohidmakhdoomi commented Aug 1, 2026 •

edited

Loading

🔴 Must fix — dropped PTY write reported as `delivered` (silent loss)

🟡 Cleanup 2 — stale `SendBuffer` comments