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

[mohidmakhdoomi]

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)

1. Mailbox store — durable mailbox table (migration v15, agent-addressed rows); kills silent loss.
2. Render-empty gate — render-gate.ts classifies a headless-terminal screen (via @xterm/headless); claude/codex profiles. Idle-gate cost well under the ≤ ~50ms budget.
3. 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).
4. 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).
5. Fast delivery triggers — submit + output-quiescence triggers via a single handleUserInput chokepoint on PtySession; held mail delivers near-immediately once the line clears.
6. Cron rerouting — the most-unguarded writer joins the one gated path; per-task supersede; honest run-log outcomes.
7. afx inbox + broadcasts + escalation — CLI (list/show/dismiss, workspace-scoped), overview-changed held-count SSE, mailbox-escalation visibility event (never delivery), liveness telemetry.
8. Dashboard + VSCode indicators — count-only held indicators + attention state on escalation (read-only; dismissal stays CLI-only).
9. Docs + skeleton mirror — send vocab, afx inbox, mailbox config; skeleton mirrored; the mailbox-first invariant added to the arch-critical.md hot tier.

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 across several additional CMAP rounds:

• Silent-loss on a dropped PTY write — a shellper socket that dies during the paced text→…→Enter sequence (Shellper reconnect error is swallowed: terminal becomes a silent zombie (no input/output, 'Message sent' logged for dropped frames) until next Tower restart #1198) was invisible to the t=0 writable precheck, so the row was marked delivered off the paced-write timer. The dropped-write result is now threaded end-to-end as a boolean; a dropped write holds the row (no-live-pty) — never a false delivery.
• 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 correctness — the classifier had only ever been validated against a synthesized claude-idle fixture, so real-output false-busy bugs shipped latent. Resolved by capturing real gzipped terminal rings as fixtures and moving classification onto a persistent bounded SessionScreen mirror per session, fed incrementally at PtySession's output chokepoint: each classify is O(viewport) rather than O(ring), the [PIR #1205] ring cap can no longer tear the classifier's input, and the whole-render #1047 OOM residual is closed. A per-session ringToken verdict memo keeps repeated checks cheap.
• Ghost-cursor exemption — claude paints a dim suggested-command "ghost" into an idle composer whose first cell doubles as the inverse block cursor; the classifier now exempts exactly that cell (inverse + non-dim, at the headless cursor, with a dim non-empty tail). Requires positive ghost evidence, so a real one-char draft can never be false-cleaned.

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 real fixtures (claude/codex/agy: idle/draft/menu/picker/trust/wrapper/ghost), write serialization, dropped-write hold (sync first write + delayed Enter/multiline), inbox routes (incl. 405 method guard), cron supersede, escalation-is-visibility-only, restart-safe identity, and production-path render tests that push real >2 MiB captures through a real RingBuffer and a real SessionScreen (ring path torn/BUSY, mirror path CLEAN).
• Playwright: dashboard held-count indicator (4/4 real chromium, incl. live update).
• Last verified green: packages/codev 4551 passing / 48 skipped / 0 failing; VSCode 667; dashboard 328; send-integration.e2e 7/7.
• 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 >1 MiB rings classify clean; idle-with-ghost→delivered; 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; the pr gate is the backstop.
• CLAUDE.md / AGENTS.md are untouched — those two prompt surfaces are owned by the open Spec 1280, so 1313's send-outcomes docs live only in the afx reference (agent-farm.md) + skeleton mirror + arch-critical.md. Both files are byte-identical to origin/main and to each other; 1313 makes no net change to them.
• Adopt/reconnect torn-seed (#1361, fast-follow) — a long-lived alt-screen session whose replay exceeds the 1 MiB reconnect cap can seed its gate mirror born-torn on a Tower restart and hold mail until the next repaint/viewer. This is pre-existing (the reconnect cap fed the old delivery path the same capped bytes) and fail-safe (the row stays persisted, visible in afx inbox, and escalates — delayed delivery, never loss or misdelivery), and self-heals on any repaint. Tracked separately so this PR stays scoped.

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

[mohidmakhdoomi]

Architect Integration Review — PR #1330 (Spec 1313, mailbox-first delivery)

Risk tier: HIGH (~13.3k additions / ~2.2k deletions, 100 files, core Tower/agent-farm/DB-migration/terminal surfaces) → full 3-way CMAP + independent architect verification against source.

3-way CMAP verdicts

Model	Verdict	Confidence
Gemini	APPROVE	HIGH — no blocking issues
Claude	APPROVE	HIGH — no blocking issues
Codex	REQUEST_CHANGES	HIGH — one blocker: the deferred #1361 adopt/reconnect liveness gap

The split is entirely on one point: the adopt/reconnect "born-torn" gate-mirror gap (#1361). Everything else is unanimous praise for a correctness-by-construction refactor.

The contested point (#1361), adjudicated against source

Codex: an adopted/reconnected shellper session seeds SessionScreen from capRingSeed()'s 1 MiB tail (tower-terminals.ts:777/1036), so a long-lived alt-screen frame >1 MiB can be born torn → classified busy → mail held indefinitely until a repaint/viewer nudge; "would not merge with #1361 outstanding."

Architect verification (pty-session.ts:227–230): attachShellper feeds the gate mirror the identical capped bytes the ring already receives — pushData(replay) (pre-existing) and feedGateScreen(replay) (round-2) use the same replay string. Therefore this gap is:

• Pre-existing, not a regression — the reconnect cap predates Spec 1313; the old path fed the gate off the same capped ring. Round-2 fixes the live-ring tear (common case) and worsens nothing (strict Pareto improvement).
• Fail-safe — the row stays durably persisted, visible in afx inbox, and escalates. It is delayed delivery, not loss — the antithesis of the silent-loss/corruption this spec exists to kill.
• Self-healing — any repaint / viewer attach / output byte clears it (a Tower restart is normally followed by dashboard/VSCode viewers reconnecting).
• Narrow — requires >1 MiB single newline-free alt-screen frame + Tower reconnect + sustained idle + no viewer + no output, simultaneously.
• Tracked — Render gate: adopt/reconnect mirror seed capped at 1 MiB → idle adopted alt-screen agent can hold mail until next repaint #1361, with two concrete fixes (uncapped ≤8 MiB mirror seed, or repaint-nudge on adopt).

Architect adjudication: non-blocking for this PR. Claude concurs ("fail-safe, pre-existing, track don't block"); Codex's concern is legitimate and #1361 should be a required fast-follow, but a pre-existing, fail-safe, self-healing, tracked edge should not block a large corruption-eliminating refactor.

Independent architect verification (all confirmed against source)

• ✅ Silent-loss fix (the prior integration round's blocker): correct — writeMessagePaced(): Promise<boolean> records any dropped write across the paced text→…→Enter sequence; deliverAgentMail defaults written=false, holds no-live-pty on drop before markDelivered, and invalidates the verdict memo in a rejection-safe finally.
• ✅ Migrations v15/v16: additive + idempotent — CREATE TABLE/INDEX IF NOT EXISTS; v16 ADD COLUMN command is PRAGMA table_info-gated. No destructive changes.
• ✅ Branch state: fully contains origin/main (0 behind, MERGEABLE) — the review's "un-rebased / T16-red / 285-behind" caveat is stale/moot.
• ✅ spec-1280 collision neutralized: CLAUDE.md/AGENTS.md are byte-identical to origin/main (0 net change) and to each other; 1313 touches neither prompt surface.
• ✅ Hot-tier displacement clean: the Spec 987 governance-tier fact and the forge-concept fact displaced from arch-critical.md both survive in cold arch.md (§Governance Docs, §Forge Concept Commands). No doc regression.
• ✅ API changes additive; SSE workspace-scoped; escalation is visibility-only (never a force path).

Decision

Held at the pr gate pending the human's merge call (2-1 split on a HIGH-confidence blocker + the committed→integrated gate is human-only). Architect recommendation: APPROVE for merge with #1361 as a required fast-follow.

---

Architect integration review

[waleedkadous]

Architect Integration Review

This is an impressive piece of systems work — the render-empty gate with empirically-captured per-harness profiles (including the agy color-keyed placeholder rule and the ghost-suggestion fixture), the transactional cron supersede, and the no-force-path principle are all the right architecture. Ran a 2-way CMAP (codex REQUEST_CHANGES / claude APPROVE, both HIGH) with the maintainer's concerns injected; where the lanes contradicted each other I verified against the branch myself. I'd like to see three changes before merge:

1. --delay's lower bound is lost on the hold path (verified: mailbox-store.ts/mailbox-wiring.ts carry no notBefore concept, and the NOT_FOUND→registry hold at tower-routes.ts:1653 excludes only escape && interrupt, not delayed sends). A --delay message to a target without a live/writable session is persisted delay-less and can drain immediately — breaking the /arch-save clear→re-init sequencing that --delay exists for. Suggested fix: persist not_before on the row and have the drainer respect it — which as a bonus makes --delay durable across Tower restarts, stronger than the [Spec 1307] /arch-save: packaged save→clear→re-init for architect context refresh #1335 in-memory timer it replaces.
2. Close the shutdown window inside the submission lock. The timer path's delivery-time generation re-check is good (claude's lane verified it), but codex identified the residual: the due callback checks liveness before entering submitToSession without a re-check inside the lock, and marks the row delivered ahead of the queued write — a shutdown during lock-wait can write after the drop decision, and a failed write stays marked delivered. Re-check the generation inside the lock, mark delivered only after the write settles, and cover this seam with a route-level test (the existing test calls the predicate directly).
3. Give the residue-starvation case a reachable alarm. Both lanes converge here, and it's the maintainer's top concern: one accidental visible character on an autonomous builder's line holds all mail (including cron) indefinitely — busy streaks are excluded from liveness telemetry by design, and escalation reaches only VSCode/dashboard, which headless flows don't watch. Minimum viable: surface heldCount/mailboxEscalated in afx status, and route a non-injecting escalation notice to the spawning architect's mailbox after a held-age threshold. (The remedy afx interrupt exists; the missing piece is the alarm that tells anyone to use it.)

Take-or-file follow-ups: route mailbox-wiring.ts:211's writeMessage through submitToSession so mailbox delivery can't interleave an interrupt (converges the two serializers the docs currently over-claim about); have afx cleanup dismiss a removed agent's held rows (today they pin heldCount/escalation forever); fix the stale "typing-aware send buffer" reference at agent-farm.md:528; and reconsider displacing the git add -A hot-tier line instead of the Spec 987 tier-routing meta-rule.

The cron supersede analysis came back fully clean — bounded to one held row per (workspace, task), verified transactional. And thank you for the honest T16-era course-corrections on this branch; the review record on it is exemplary.