Repository navigation
afx send: add --interrupt-after <seconds> (hold, then force-deliver after a bounded wait) #1481
Description
Activity
- added a commit that references this issue
on Aug 18, 2026 Known corner to fold into this work (documented in the PR #1492 review, deliberately NOT built standalone — owner call 2026-09-01):
boundedinsubmitToSessionis computed once at enqueue (contended && ceiling && !behindOperator), so with a long delivery in flight and one operator queued (ceiling armed), a second operator arriving meanwhile gets an unbounded wait — it sits through the delivery's entire paced write plus the first operator, where pre-#1365 its interrupt landed immediately. Needs a >2s delivery plus two operator gestures inside its window, so it is rare, and the write-edge size cap (new issue, refs #1564/#1521) shrinks worst-case delivery duration to a couple of seconds, which mostly evaporates the window. If/when --interrupt-after is built: re-evaluate the bound when the operator ahead drains (or race against all non-operator predecessors) rather than deciding once at enqueue.Reacted by Mohid MakhdoomiArchitect preflight — 2026-09-06 (HEAD
08ec15122)#1483 remains the sequencing authority. #1481 is its only unticked item; no completed/consolidated issue is being reopened. Hard prerequisites are satisfied: #1476 / PR #1485 and #1365 / PR #1492 are merged, and both builder tips are ancestors of this HEAD.
Protocol: strict PIR. Approach review is necessary for the timeout/write-edge semantics, followed by running-terminal evidence before a PR. First deliverable is a committed plan, then STOP for explicit human plan-approval. This comment does not approve any gate.
Current-code verification
- The feature remains absent: CLI
packages/codev/src/agent-farm/cli.ts:452-488exposes immediate--interruptand--delay, but no--interrupt-after; no implementation ofinterruptAfterexists in packages. db/schema.ts:273anddb/mailbox.ts:148-150:not_beforeis a delivery eligibility lower bound, not an interrupt deadline. Setting it to the new deadline would defeat “try gated delivery immediately.” Keep eligibility and timeout policy distinct in the design.db/index.ts:169,177calls/exports the realrunGlobalMigrations;db/migrations.ts:29is currently v18. Any new schema step must use and test that real runner, including fresh/upgrade convergence.servers/tower-routes.ts:1744-1751,1812-1863: delayed interrupt fires Ctrl+C only, its body remains gated; the nudge is in-memory and lost across restart while the message survives.:2145-2180: immediate interrupt is the explicit claim-first body bypass. The issue's phrase “force-deliver” and its suggested delayed-interrupt reuse are not interchangeable: the plan must state exactly whether the deadline guarantees an interrupt attempt or an ungated body write, what happens if the prompt stays busy, and the restart/offline/cancellation tradeoffs, for human decision.servers/session-submit.ts:44-112,350-435: convergence is implemented; per-agent → per-terminal is the settled lock order. The Sep 1 issue comment's corner is still present:boundedis computed once at enqueue (:369-371) while another operator is ahead. Evaluate and address the corner without allowing operator-on-operator interleaving or blocking unrelated agents' drain/alarms.db/mailbox.ts:161-183,373-387: both escalation age and owner starvation aggregation currently depend on creation/eligibility time. A future interrupt deadline needs explicit alarm treatment; do not hide ordinary starving mail for the same recipient or corrupt notice-clearing membership.
Dependencies and required evidence
#1477's seeded-registry wiring harness is already built/reviewed in PR #1625, protocol complete but not merged. Its reference files are under
/home/user/code/codev_root/codev/.builders/air-1477/packages/codev/src/agent-farm/__tests__/air-1477-{owner-escalation-wiring,cleanup-dismiss-invocation}.test.ts. Read by absolute path; do not modify that worktree or silently absorb its PR. Plan an explicit reuse/landing strategy. #1473 / PR #1634 is also parked/unmerged; account for later integration without silently assuming its input tracking is in main.Require deterministic race/clock/real-runner tests plus an isolated live Tower + real PTY/CLI path for: clean-before-deadline (no later Ctrl+C); busy through deadline; dismiss/supersede/deliver while timeout waits; session replacement/offline/restart; two queued operators behind a long delivery; predeadline starvation suppression without suppressing other mail; no duplicate body delivery; unchanged existing flag behavior and invalid flag combinations. Do not restart/stop the live Tower on 4100.
Contributor constraint (load-bearing)
We are not cluesmith/codev maintainers. Every PR needs a maintainer/reviewer approval before merge; the maintainer will merge. Do not merge a PR, close any issue, or clean up any worktree. Explicit human approval is required at each porch gate; a gate notification is not approval. Keep long findings on the issue/PR and send short status messages to the spawning architect.
- The feature remains absent: CLI
On it! Working on this with the PIR protocol (plan + dev-approval gates before PR).
- added a commit that references this issue
on Sep 6, 2026 Architect plan review — human decision required
Reviewed committed plan
21bed9b14by absolute worktree path and independently checked its root-cause claims against HEAD. Porch is waiting at plan-approval; this is not an approval.Problem: There is still no send mode that prefers clean delivery but interrupts after a patience budget.
Root cause: The current CLI/API separates force-now from gated/delayed eligibility.
not_beforeprevents early delivery; the delayed-interrupt path only sends Ctrl+C, whereas immediate interrupt claims the row and writes its body without the gate. Those are materially different contracts, not interchangeable helpers. The queued-operator ceiling issue also remains insession-submit.ts:369-371.Proposed fix: The builder recommends a separate durable deadline, one serialized Ctrl+C attempt after it, then gated body delivery. I favor retaining the gate, but this is a deliberate deviation from the issue's literal force-delivery promise: if the screen stays unverifiable/busy, the message remains held. The plan also preserves timeout intent across restart/offline time, which can interrupt a later turn, fixes the queued-operator bound, and suppresses premature alarms per row rather than per recipient. Both the weaker delivery guarantee and durable late-interruption behavior need explicit human acceptance. If literal force-body delivery is required, revise the ownership/race design before approval.
Testing: Planned, not executed: real-runner migration convergence; deterministic timeout/cancellation/restart/operator/alarm races; isolated live Tower + PTY + worktree CLI evidence, including a real supported agent. No implementation or PR yet. Scope is high risk (state, terminal serialization, cross-package API); final integration review requires parallel 3-way CMAP and actual running-path verification.
Committed plan. #1483's checkbox remains unticked. No merge, issue closure, or worktree cleanup authorized.
Human-directed plan revision — mimic
--interruptafter the patience budgetThe human clarified the intended contract and instructed: “Ok have the builder revise the plan.” This authorizes a plan revision only, not plan-approval or implementation. Supersedes the proposed Ctrl+C-only/gated-body contract in plan
21bed9b14and my initial preference for that alternative.Required behavior
For
afx send <target> <message> --interrupt-after 8:- Behave like ordinary
afx sendinitially: persist once, attempt gated delivery immediately, and continue normal gated retries during the eight-second patience window. - If the same message is still undelivered when the deadline is reached, transition to existing immediate
afx send --interruptsemantics: Ctrl+C, the existing fixed settle, then ungated message writing and Enter (unless no-enter). Do not keep the body gated after this transition. - Delivery, dismissal, or supersession before the force-write transition must cancel escalation. Recheck at the actual write/ownership boundary, not just when a timeout callback starts. Explicitly handle a normal write already in flight and cancellation while waiting on locks; never create a second mailbox row/body send just to invoke interruption.
Required plan changes
- Reuse/refactor the existing interrupt writer and terminal submission primitive; avoid a parallel implementation of Ctrl+C/settle/body/Enter behavior. Keep the settled lock order and the queued-operator corner fix, without parking the global drainer or unrelated agents behind a terminal wait.
- Spell out row ownership/claim timing, in-flight delivery races, claim-before-write crash/loss tradeoffs, degraded-write warnings, and the audit outcome. Do not claim guaranteed receipt or exactly-once PTY effects. The budget bounds when escalation is initiated, not scheduler/lock latency or acknowledgment by an agent; schedule the deadline promptly, not with an unexplained extra polling delay.
- Revise persistence/schema, alarm membership, diagnostics, and tests to support the force-body contract, removing the obsolete one-shot-nudge model wherever inappropriate. Preserve immediate delivery eligibility before the deadline.
- Explicitly propose behavior for no live/writable recipient at the deadline, session replacement, and Tower restart. The human has not independently approved the old plan's indefinite durable late-interruption policy; don't silently inherit it as settled. Minimize surprise and flag any remaining product choice for the revised plan gate.
- Require tests for clean-before-deadline/no later Ctrl+C, busy/unverifiable gate at deadline still gets the interrupt body path, cancellation and in-flight delivery races with no second body, two operators behind a long delivery, offline/restart/session replacement, predeadline alarm suppression without hiding unrelated mail, unchanged ordinary/immediate/delayed flags, plus isolated live Tower/real PTY/worktree-CLI evidence.
Revise the plan and any accompanying requirements snapshot/thread notes, commit/push the revision, and return the SHA + artifact path + remaining decisions. Stay at the existing plan-approval gate. No implementation, gate approval, PR merge, issue closure, or worktree cleanup; maintainer/reviewer approval and maintainer merge remain required.
- Behave like ordinary
- added a commit that references this issue
on Sep 6, 2026 Architect review of revised plan
697c15a16— comment #5493338483 coveredReviewed the pushed artifact by absolute worktree path before requesting human review. Re-fetched comment #5493338483 and verified the underlying defect in current
session-submit.ts:366-397,412-433. This is a plan review, not gate approval.Problem: Implement bounded-patience send without preserving the queued-operator latency defect flagged for this issue.
Root cause:
boundedis fixed when an operator is enqueued. An earlier operator makes operator 2 choose the unbounded branch; its awaited combined predecessor tail still includes the original long delivery even after operator 1's actual write finishes. DecrementingpendingOperatorslater cannot change the already-selected branch.Fix / conformance mapping: Revised plan §3, lines 62–70, retains the fix in the shared submission primitive, not a timeout-only wrapper:
Comment requirement / invariant Revised plan Do not permanently decide the wait from the enqueue-time operator count Separate operator-write completion from the combined predecessor tail After the preceding operator drains, do not remain stuck behind a delivery whose budget expired Require prior operators to finish AND either combined predecessors to finish or the delivery ceiling to expire; budget is measured from submission enqueue, not reset after operator completion Never bypass another queued/in-flight operator Await preceding operators' actual write completion, including scheduled body/Enter completion Keep unfinished delivery work represented for later writers Preserve the combined tail until all relevant predecessor/current work completes The force-body revision (§2) also shares the immediate Ctrl+C/100 ms settle/body/Enter writer and this primitive. It does not just invoke today's unchanged
--interruptand inherit the corner.Testing: Test Plan item 6 explicitly covers a >2-second delivery with two operators: operator 2 proceeds after operator 1 and its own expired ceiling, before the long delivery finishes, never overlapping operator 1; third-writer ordering and unrelated-agent progress are included. §3 additionally requires failure/no-op/interspersed-delivery cases. Implementation acceptance will require this regression to discriminate against the old behavior, plus unit and isolated real-Tower/PTY evidence. These tests are planned, not yet implemented or run.
Other review points: The revised contract now uses the original still-held row for ungated force-body escalation, with write-edge cancellation/ownership checks, prompt deadline scheduling, partial-write disarming to avoid a second forced body, and explicit claim-before-write loss/degradation diagnostics. No implementation defect is claimed fixed by this document.
Still requires human decision: Proposed lifecycle policy cancels force authority across Tower restart (including future deadlines), an unavailable/unwritable target at the deadline, or session replacement while queued. The body remains ordinary held mail; no surprise catch-up interrupt occurs later. This is explicitly proposed, not previously approved. A possible partial normal body write also disarms the timeout force rather than injecting a second copy.
Reviewed plan at immutable revision. The plan folds in the comment at design/test-plan level and is ready for human review. Porch stays at plan-approval; no implementation, merge, closure, or cleanup authorized.
Claude consultation — revised plan
697c15a16Human-requested
consult -m claudecompleted successfully usingclaude-opus-5(484.1 s). Verdict: COMMENT, not a gate approval. Claude independently confirms §3 genuinely incorporates comment #5493338483. The architecture is supported; it recommends plan refinements before approval. No implementation/tests were run by this consultation.Architect verification / disposition (read before the raw review)
I checked the findings against current source; reviewer claims below are evidence, not automatically accepted facts.
- F1 — accept an accounting clarification/test, not the claimed demonstrated duplicate. After introducing the operator-completion AND condition, degradation must reflect actually proceeding ahead of unfinished predecessor work at the write edge, not merely an elapsed ceiling while waiting for an operator. HOWEVER, the review's numeric example has combined predecessors done at 3.4 s before O2's 3.5 s ceiling, so a proper race would already select completion. It does not establish its stated O2-caused duplicate; the delivery's bypass watcher may already have closed. Require a valid ordering trace/test and accurate
wroteBytes/degradation accounting; do not repeat that example as a reproduced defect. - F2 — make existing requirements explicit. §3 already requires error isolation and cleanup/reset hooks; spell out rejection-neutral operator completion tails, identity-guarded eviction of the new per-session structure, no-op completion, and tests. Current combined-tail handling is at
session-submit.ts:359-362,444-449. - F3 — substantive timeout-contract concern. Current
mailbox-delivery.ts:765-772re-holds dropped/preempted normal writes, and ordinary delivery may retry. Permanently cancelling force for ANY earlier uncertain write (even long before the deadline) can silently remove the user's requested escalation. Same-row ownership through DB outcome is essential; lifetime-long disarming is a separate policy, not automatically required by that ownership invariant. Revisit/narrow this rule, state the retained partial-write/duplicate risks and visible outcome, and test a predeadline partial failure followed by recovery and a still-held row at deadline. Do not silently add new product semantics or claim exactly-once PTY effects. - F4 — confirmed integration obligations. Current normal delivery uses
ports.broadcastandports.onHeldStateChange(mailbox-delivery.ts:811-812), bound inmailbox-wiring.ts:306-307. Plan must explicitly update the activity/feed/overview/held-count surfaces for force transitions and skips. Reuse existing wiring exactly once; do not blindly emit both helper and underlying notifications as separate events. - F5 — qualify the size claim. Main already enforces
MAX_MESSAGE_BYTES = 48 * 1024(message-format.ts:120,tower-routes.ts:1912), so this is NOT an arbitrarily large/1-MiB message path. A byte cap still permits many short lines and long paced writes (message-write.ts:25-28,96-109), so unattended force-duration risk is worth documenting/testing without inventing a missing prerequisite. - F6/F7/F8/F10 — useful precision. Distinguish send patience from submission wait ceiling; acknowledge the unbounded wait behind another operator; include no-op submissions in operator ordering; preserve delivery try-lock/decline semantics; state that timed force can overtake older held rows while those rows still alarm.
- F9 — define the outcome vocabulary. Specify whether
degradedimplies completed-but-unverified and how degraded+failed is preserved/reported. A documented precedence or a separate field are alternatives; no extra schema field is mandated by this review. - Validator detail — confirmed. Existing
validateDelaySecondsrequiresNumber.isInteger(delayed-send.ts:95-104); the proposed fractional timeout requires its own validation (possibly sharing the upper bound), not unchanged reuse of that function.
The proposed restart/offline/replacement behavior remains a human product decision, not a correctness finding or an approved policy. The consultation does not authorize rearming/catch-up windows, recipient restrictions, or another force policy. Plan-approval remains pending. Maintainer-only merge workflow remains in effect.
Full Claude review (raw advisory output; qualifications above apply)
I'll verify the plan against actual source before assessing. Starting with the core files.Now the delivery path, message writer, and mailbox DB.Now the interrupt paths in tower-routes and the drainer tick.I've verified the plan's claims against the actual source in the builder worktree. Here is my independent review.
Claude Plan Review — PIR #1481, revision
697c15a16Verdict: COMMENT (advisory only — this is not gate approval, and I have not touched porch, files, or worktrees.)
The architecture is sound and, unusually, its central claims survive contact with the code. The three load-bearing decisions — (a) share the immediate-interrupt writer rather than reimplement it, (b) arbitrate a second body on the same row with per-row write ownership rather than with
status = held, (c) fixbehindOperatorby tracking predecessor completion instead of deciding at enqueue — are each necessary and each correctly motivated. I found no reason to reject the approach. I found six things that should be written into the plan before it goes to the gate, one of which (F1) is a concrete new-defect risk the plan would otherwise ship.Plan citation audit — all verified accurate, which matters given the hot-tier lesson about summaries vs. ground truth:
Plan claim Verified cli.ts:452-488has no bounded-patience option✅ cli.ts:446-495db/mailbox.ts:148-161separates eligibility from escalation age✅ findHeldForAgent:140-151,ESCALATION_START_SQL:161tower-routes.ts:1812-1863delayed interrupt is in-memory, terminal-bound, Ctrl+C-only, gated body✅ :1812-1865 tower-routes.ts:2145-2225immediate interrupt claims the row before an ungated body✅ :2148-2232 ( markMailboxDelivered:2170 precedessubmitToSession:2193)session-submit.ts:350-435computes bounded waiting once at enqueue✅ :366-371 ( behindOperator:370,bounded:371)mailbox-delivery.ts:1012-1065awaits agents sequentially before alarms✅ tick:1013-1066db/mailbox.ts:373-387supplies both owner-alarm age and clearing membership✅ findStarvingAgents:373-388, consumed bynoticeOverdue:1135-1168v18 is current; runGlobalMigrationsis the real runner✅ migrations.ts:559-566,db/index.ts:169;spec-1313-migration.test.tsis the patternNamed test files exist ✅ all six
1. Maintainer comment #5493338483 — conformance assessment
§3 conforms in substance. The comment asks for exactly one thing: "re-evaluate the bound when the operator ahead drains (or race against all non-operator predecessors) rather than deciding once at enqueue." §3 adopts both halves — an operator-only chain (so O2 waits on O1's own write, never O1's delivery-containing tail) AND a ceiling raced against non-operator predecessors, joined by
AND(operatorsDone, OR(combinedDone, ceilingExpired)).I traced the requested scenarios against that rule:
Scenario Result under §3 D=10s delivery; O1 @t=1 (ceiling→t=3, writes degraded, done t=3.2); O2 @t=1.5 (ceiling→t=3.5) O2 runs t=3.5. Today it runs at t=10. Corner closed. Same, but O1's write is slow and finishes t=6 O2 runs t=6 — operator-vs-operator serialization preserved (the hard AND). O3 arrives while O2 is merely queued O3 chains on O2's current, which exists at enqueue → cannot bypass. Preservesspec-1365-serializer-convergence.test.ts:359.Ceiling expires before vs after the prior operator drains Both orderings collapse to max(operatorsDone, min(combinedDone, ceiling)). No bypass either way.Mixed kinds Correct, but see F8 — deliveries never queue ( trySubmitToSession:479declines on contention), so "combined predecessors" is at most {one active delivery} ∪ {operators}. The plan should say so; it constrains the implementation.Failure See F2 — not yet specified. No-op writes See F7 — ambiguous. But conformance is only in substance, not in the two places where the implementation can go wrong (F1, F2), and the plan understates why this coupling is mandatory rather than merely tidy. The maintainer called the corner "rare" because it needs two operator gestures inside a >2s delivery window.
--interrupt-afterremoves the rarity: it manufactures operator submissions unattended, on timers, potentially several per terminal (--allbroadcast, multiple senders). The corner stops being a two-humans-typing-fast coincidence. That argument belongs in §3's motivation — it is the strongest justification for folding the fix in here rather than deferring it again.
2. Findings — correctness (ordered by priority)
F1 — §3:
bypassedmust be decided by who won the race, not by whether the ceiling fired. A latched flag manufactures spuriouspreemptedholds.Plan: §3, "Run only once both conditions hold: preceding operators finished, and combined predecessors finished OR the delivery ceiling expired."
Code:session-submit.ts:379-406(bypassed→unserializedWritesbump), consumed atmessage-write.ts:213andmailbox-delivery.ts:766-772.Today
bypassedand "I proceeded unserialized" are the same event, so latching is safe. Under §3 they come apart, because the ceiling can expire while the hard AND is still holding the submission back.Failing timeline:
t=0Delivery D (row R1) begins a paced write, completingt=3.4.t=1.0Operator O1 enqueues; ceiling expirest=3.0; writes degraded; its write finishest=3.2.t=1.5Operator O2 (our timed force, row R2) enqueues; ceiling expirest=3.5.t=3.4D's write completes — combined predecessors are now settled.t=3.5O2's gate opens. It was fully serialized against D: it wrote nothing before D finished.
If
bypassedwas latched att=3.5because "my ceiling expired", O2 bumpsunserializedWrites(:404-406). D'swatchBypasses(message-write.ts:185, 213) then reportsraced() === true→preempted→mailbox-delivery.ts:766-772holds row R1 and re-delivers the whole body later. A duplicate charged for a race that provably did not happen — the exact failure mode thewroteBytesno-op guard (session-submit.ts:196-202) was added to prevent, arriving through a new door.Correction to write into §3:
bypassedis true only if the combined-predecessor tail was still unsettled at the instant the write callback ran. Implement as a genuinePromise.racewhose winner is recorded, or sample a settled-flag on the combined tail synchronously inside the write callback. Add the assertion to the test list: an operator whose ceiling expired but which nonetheless ran after its predecessors completed must not bumpunserializedWriteCount, and the concurrent delivery must reportwritten, notpreempted.F2 — §3: the operator-only chain must be settled-wrapped and self-evicting. Neither is stated.
Plan: §3, "Keep an operator-only completion chain separate from the combined submission tail… Preserve error isolation."
Code:session-submit.ts:359-362(the existing settled-wrap and why),:444-449(self-eviction ofchains),:297-301(evictBypassCountIfIdle).Two obligations the existing
chainsdischarges and a new parallel structure must discharge independently:- Poisoning. If O1's
write()throws, itscurrentrejects. If the operator chain stores that raw promise, every subsequent operator on that terminal awaits a rejected promise — either an unhandled rejection (whichtower-server's handler turns intoexit(1), permailbox-delivery.ts:1036-1040) or a permanently wedged operator path on that terminal.:359-362exists verbatim for this reason on the combined chain; the operator chain needs the same treatment and the plan should say so. - Leak.
chainsself-deletes on drain (:444-449) andunserializedWritesself-evicts (:297-301) — the codebase treats per-session map growth as a defect class (the comment at:237cites VSCode: bound the mailbox escalation-toastseenSet (dedupe by mailboxId with eviction) #1472). A second per-session map must state its eviction rule. Extendspec-1365-serializer-convergence.test.ts:224("leaves no chain behind once mixed traffic settles") to cover it.
F3 — §2a: the
partial-normal-writedisarm is justified in principle but not accurately bounded. As written it silently turns the flag into a no-op in cases where it demonstrably prevents nothing.Plan: §2a, "If a normal write reports
dropped,preempted, or throws… skip force for this row … The gated outcome handler records this skip for every armed row with an uncertain partial write, even if its timer has not fired yet."The plan's own rationale is internally inconsistent, and it admits as much two sentences later ("Existing ordinary retry semantics can themselves repeat partial/unconfirmed effects"). Concretely:
droppedandpreemptedboth leave the rowheld(mailbox-delivery.ts:765,:766-772), and the drainer will re-write the entire body on the next clean gate pass with no attempt cap. So refusing to force does not avoid a second copy of the body — the gate will produce one anyway. It only removes the escalation the user explicitly paid for.Failing timeline:
t=0afx send --interrupt-after 8. Row armed, deadlinet=8.t=1Gated attempt reaches its write edge; the shellper socket dies mid-pace →dropped(message-write.ts:212). Row re-heldno-live-pty.t=2Socket recovers. Screen busy for the next hour.t=8Deadline. Force permanently disarmed by a seven-second-old partial write, against a composer that has since been fully repainted.- The row now behaves as an ordinary send forever, and the CLI already returned
held, so nothing tells the user their escalation was cancelled except an inbox field.
The exception that is justified is narrow and different: the force must not write a second body while arbitrating against a write that it itself declined ownership to, or one that finished moments ago on the same composer. That is a same-row, same-window rule, not a permanent property of the row.
Recommended correction: bound the skip by state, not by row lifetime. Something like: the force is disarmed only if a byte-attempting write to this row was in flight or completed within the arbitration window (e.g. the ownership continuation that the force actually registered), and re-arms if a subsequent gated pass observes the row still
heldwith a fresh render-gate verdict. Whatever rule is chosen, the plan must state it as a rule, must state the user-visible consequence, and must make the disarm visible in the CLI/inbox rather than only ininterrupt_outcome.F4 — §2: the force path's broadcast, indicator and activity-feed obligations are unstated.
Plan: §2 specifies the DB claim and the write; §2b specifies warnings. Neither says what else must fire.
Code: the two existing paths that move a row out ofheldboth do more than update SQL:mailbox-delivery.ts:807-813—markDelivered→ports.broadcast(broadcastForRow(...))→ports.onHeldStateChange()→ log.tower-routes.ts:2208-2215— the immediate interrupt'sbroadcastMessage({type:'message', …})into the activity feed.
A forced row leaves the held set through neither. Without explicit obligations the shipped behavior is: held-count badge and
mailboxEscalatedattention bit go stale until the nextonHeldStateChangefrom an unrelated event; the forced message never appears in the Tower message feed or SSE stream; dashboards disagree withafx inbox. Add to §2: the force edge must emit the delivery broadcast,onHeldStateChange(), the activity-feedbroadcastMessage, and theoverview-changednotification, and the test plan must assert events, not just DB state (the plan already says "count/notice events" in Test Plan item 4 — make it an explicit requirement in §2).F5 — §2/Risks: an unattended ungated force of an arbitrarily large body is a new risk class the plan does not name.
Code:
session-submit.ts:213-216— "a paced write's duration is(lines−1)×10+80ms and a request body is capped only byparseJsonBody's 1 MiB, so a 48 KB--fileof short lines is ~8 minutes on the wire."--interrupthas this exposure today with a human watching.--interrupt-after --filemoves it to a timer: a Ctrl+C followed by an eight-minute ungated paced write that holds the per-terminal lock for its whole duration, during which every other delivery to that terminal is declined and any operator either waits or degrades. The maintainer's comment explicitly points at the write-edge size cap (refs #1564/#1521) as what shrinks this window. The plan mentions neither.Recommended: name the interaction in Risks & Alternatives, and pick one — document loudly, refuse
--interrupt-afterabove a body-size threshold, or state the dependency on the size-cap issue. This is a product call, but the plan should not be silent on it.F6 — terminology: "budget" means two different things, and the honest worst case is not stated.
§2 uses budget for the user's patience deadline ("The budget bounds initiation of escalation"). §3 uses budget for the per-submission operator ceiling ("measure the delivery budget from enqueue"). They are different clocks with different origins (send time vs.
submitToSessionentry) and different purposes (when to escalate vs. when to write unserialized). Name them distinctly in plan and code (patienceDeadlineAtvs.waitCeilingMs).Related and more important for the contract: state the real worst case for initiation. It is
deadline + (wait behind any queued operators, which is unbounded by design) + (up to OPERATOR_SUBMIT_WAIT_CEILING_MS behind a delivery). §2's "not event-loop/lock latency" gestures at this; the unbounded operator-vs-operator component deserves an explicit sentence, and belongs in the CLI help text too.F7 — §3: "preceding operators' actual writes" is ambiguous for no-op writers.
The delayed
^Clegitimately writes nothing (tower-routes.ts:1832-1834,wroteBytes: () => firedat:1849). Does a no-op operator count as a predecessor that must complete? It must — serialization is about turns, not bytes;wroteBytesexists solely for bypass accounting (session-submit.ts:196-202). Reword to "preceding operators' own submission completion (including a submission that legitimately wrote nothing)" so an implementer cannot read "actual writes" as "only byte-producing writes."F8 — §3: state that deliveries never queue.
trySubmitToSession:479returnsfalseon contention. So the queue behind an operator is {at most one in-flight delivery} ∪ {operators}. Saying this converts "combined predecessors" from a vague set into a two-element structure and removes a whole class of implementation over-engineering.F9 — §1:
interrupt_outcomeconflates a degradation flag with a terminal outcome.degradedandwritten-unverifiedare not alternatives — a force can be both ceiling-bypassing and completed-unverified, and §2b asks for both facts. One enum column can only record one. Either add a separateinterrupt_degradedflag/column or define a documented precedence and accept the information loss explicitly.F10 — unstated semantic: the force delivers out of order.
deliverAgentMailis strictly oldest-first (mailbox-delivery.ts:664,held[0]). A timed force claims its own row regardless of older held mail, so the agent can receive message N before messages 1..N−1. Immediate--interruptalready behaves this way, so this is consistent precedent rather than a defect — but it is now happening unattended, and it interacts with §4's alarm rules (older ordinary rows keep alarming, which the plan correctly preserves). Say it out loud in §1 or §4 and cover it in a test.
3. Product decisions — reasonable choices, not correctness gaps
These are the lifecycle policies the human has explicitly not approved. My read: none is a correctness defect; all are defensible; two are worth reconsidering.
Policy Assessment Runtime-only force authority; restart disarms even future deadlines Defensible and precedented — delayed-send.ts:24-33makes exactly this trade for the delayed^C("shutdown drops the ^C nudge, never the message"). Consistency with an existing documented boundary is worth more than durability here. I'd take it.Startup disarm sweep writes skipped-restartFine under the single-Tower/single-global.db invariant. Requires the stated ordering (sweep before mailbox writers start) so it cannot race a live send; the plan says this. Offline/unwritable at deadline → skip permanently, never retry on a later session The weakest of the set. A session that is momentarily unresolvable — a respawn, an afx spawn --resume— permanently cancels the escalation for a row that stays held and deliverable. Consider a bounded re-attempt window (e.g. retry while the row is still held and within one escalation period) as an alternative worth putting in front of the human. Not blocking.Replacement while waiting → skip; replacement before deadline → target current session Correct and clearly reasoned. Take it. Wall-clock honesty (relative timers, recheck at fire, rearm on backward jump, no early wake on forward jump) Accurate about Node timer behavior. Worth adding a cheap backstop: have the drainer tick notice overdue armed rows, so a dropped/overslept timer still forces. Costs little and removes a silent-failure mode. --all+--interrupt-afterNot discussed. N recipients means N unattended Ctrl+C's at one instant. Worth a deliberate decision (allow / warn / refuse) rather than falling out of "each broadcast recipient receives its own deadline". Architect targets / self-send Not discussed. An unattended ^Cinto an architect session is higher-consequence than into a builder. Worth one sentence either way.
4. Schema, contracts, tests and evidence (question 5)
Sufficient, with the additions below.
- Migration. v19 is correct (v18 at
migrations.ts:559-566),runGlobalMigrationsis the real runner (db/index.ts:169), andspec-1313-migration.test.tsis exactly the fresh/upgrade/idempotency pattern to copy. No index is needed for three nullable columns on a table already bounded by the 30-day prune. Good. - Cross-package.
types/src/api.ts+sdk/src/tower-client.tswith no server↔client import is the right shape and respects the Introduce packages/codev-sdk: client SDK for Tower (server/client dependency isolation) #1189 boundary tests. - Validation. Reusing the
--delaybound/validation semantics (delayed-send.ts:95-106) is right, but note thatvalidateDelaySecondsrequiresNumber.isIntegerwhile §1 wants fractions accepted — so this is a new validator sharing the ceiling constant, not a reuse of the function. Make that explicit so nobody "reuses" it and silently rejects--interrupt-after 0.5. - Evidence. The child-Tower e2e pattern (
send-integration.e2e.test.tsexists) with isolatedCODEV_AGENT_FARM_DIR, a non-4100 port, and worktree-built CLI is the correct harness, and the plan correctly forbids touching the live Tower or existing worktrees. The separation of "fixture PTY proofs" from "real agent CLI observations" is the right discipline and should be held to. - Exactly-once. The plan correctly refuses to claim it (§2b). Keep that wording; do not let it soften during implementation.
Required tests that distinguish today's behavior from the planned behavior
The
spec-1365file already uses a CONTROL-test idiom (:126,:152) — reuse it, because these need to demonstrate the current bug, not just assert the fix.- CONTROL + fix, the maintainer corner. 3 s delivery in flight; O1 enqueues (ceiling armed); 100 ms later O2 enqueues. Today: O2's write timestamp lands after the delivery completes (~3 s) — the unbounded wait. Planned: O2 writes at ≈
max(O1 done, its own 2 s ceiling), strictly before the delivery completes, and strictly after O1's write. Assert recorded write order, not just timing. - Three operators. Preserve
spec-1365-serializer-convergence.test.ts:359verbatim under the new rule — a third operator still cannot bypass a merely-queued second one. - F1 regression. Ceiling expires while the hard AND still holds; predecessors then settle before the write runs → assert
unserializedWriteCountis unchanged and the concurrent delivery reportswritten(notpreempted, not re-held). - F2 regression. An operator whose write throws must not block the next operator on that terminal, and must not produce an unhandled rejection;
pendingSubmissionSessions()and the new operator map both return to empty. - CONTROL + fix, the double-body. Deadline fires while a gated paced write of the same row is mid-pace. Without ownership: two bodies on the PTY and a
markDeliveredthat returns false — assert this in the CONTROL to prove the hazard is real. With ownership: exactly one body, exactly oneheld → deliveredtransition, force records a cancel. - The microtask window. Gated write has completed but
deliverAgentMailhas not yet reachedmarkDelivered(mailbox-delivery.ts:807) — drive it with a port that inserts an await. The force must decline (ownership still held), not observestatus = heldand claim. This is the specific race that makes "release ownership only after committing the delivery" load-bearing. - Force-first. Force claims and writes; a subsequent gated precheck must refuse via
row-resolved(mailbox-delivery.ts:705-707) with zero bytes. - Unverifiable gate. No profile / classifier-stuck through the deadline → still
^C+ 100 ms settle + ungated body + Enter;--no-enteromits Enter. This is the human-selected contract and needs a direct test. - Events, not just rows. Force claim emits delivery broadcast +
onHeldStateChange+ activity-feed message +overview-changed(F4). - Alarms. Armed pre-deadline row suppressed from
findStarvingAgentsmembership while an ordinary held row for the same agent still alarms; suppression lifts on skip/restart; notice clearing and re-arming across the deadline; and — the one the plan already caught — the JSageMsatmailbox-delivery.ts:1086must use the same start expression as the SQL.
5. What I looked for and did not find
Stating these so the absence of a finding is informative rather than ambiguous:
- No lock cycle. The plan's "per-terminal + synchronous try-ownership only, never the per-agent serializer, never await a row/agent lock while holding the terminal lock" preserves the
arch-criticalinvariant.PtySession.write()emits no'submit'(session-submit.ts:82-85), so no re-entrancy from inside the lock. - No global-drainer stall. The coordinator's timers are independent of
MailboxDrainer.tick's sequential agent loop (:1033-1053), and the plan explicitly forbids adding operator-lock waits to it. Correct — a blocking wait there would stall every other agent's mail plus escalation and prune. - Per-row ownership is not speculative refactoring. I verified the concrete double-body path it closes:
markDeliveredruns only atmailbox-delivery.ts:807, after the paced write and after the terminal lock has been released, so a force that consultsstatusalone will readheldfor a row whose body is already fully on the wire. Nothing cheaper than ownership closes that. - The shared-writer extraction is safe.
tower-routes.ts:2196-2197is two statements (session.write('\x03')thenwriteMessageToSession(session, msg, noEnter, 100)); extracting them preserves the timing contract exactly, since the 100 ms offset and the returned completion offset are both properties ofwriteMessageToSession(message-write.ts:74-113). - Not a defect: the
--interrupt/--interrupt-aftercontract divergence on unwritable targets (503 vs. persist-and-hold). Deliberate and stated.
Bottom line for the gate. The design is right and the maintainer comment is genuinely folded in, not merely referenced. Before this is worth approving I would want F1, F2 and F3 written into the plan text — F1 because it introduces a new spurious-duplicate path if implemented naively, F2 because it silently reintroduces two defect classes the codebase has already paid for, and F3 because as written it can cancel the user's escalation for reasons that prevent nothing. F4–F10 are smaller and could be handled in review, though F4 is the kind of integration gap that ships stale UI if it is not written down. Everything under §3 above is a product call for the human, not a blocker.
- F1 — accept an accounting clarification/test, not the claimed demonstrated duplicate. After introducing the operator-completion AND condition, degradation must reflect actually proceeding ahead of unfinished predecessor work at the write edge, not merely an elapsed ceiling while waiting for an operator. HOWEVER, the review's numeric example has combined predecessors done at 3.4 s before O2's 3.5 s ceiling, so a proper race would already select completion. It does not establish its stated O2-caused duplicate; the delivery's bypass watcher may already have closed. Require a valid ordering trace/test and accurate
- added a commit that references this issue
on Sep 6, 2026 Architect verification of Claude follow-up — plan
e6d2799afReviewed the committed/pushed diff from
697c15a16by absolute worktree path, checked the relevant current write/outcome/broadcast code, and confirmed porch remains at plan-approval. Only plan/requirements/thread documents changed. No approval or implementation is implied. This is architect verification of the Claude dispositions, not a second Claude consultation.Problem: The plan needed to preserve the requested timeout after uncertain normal-write failures and make the shared serializer/accounting/integration requirements explicit.
Root cause: Current dropped/preempted normal writes remain held and retryable (
mailbox-delivery.ts:765-772), while successful writes become terminal before echo verification (:807). The former plan incorrectly treated any historical possible-partial write as permanent cancellation of escalation, even when ordinary retry was still allowed. Separately, the shared serializer's enqueue-time ceiling decision is the confirmed #5493338483 defect; a replacement must preserve submission ordering and truthful interference accounting.Fix — verified in the revised plan:
- §2a keeps force armed after uncertain partial failure; it waits for active same-row ownership AND DB outcome, cancels after successful/terminal delivery, and permits one sequential forced retry only for a still-held row. A persistent prior-partial flag and row-specific warnings disclose possible duplicate effects. No fresh gate-clean requirement sneaks into the forced retry.
- §3 still satisfies comment #5493338483. It now explicitly covers rejection-neutral operator tails, no-op submission ordering, identity-guarded map eviction, declined delivery contention, distinct patience/submission clocks, and actual-write-edge rather than latched-timer degradation accounting.
- The F1 trace is corrected: O2 does not add a bypass after D and O1 have completed; the test does NOT falsely require D to be unpreempted when O1 already bypassed it. Companion traces test genuine remaining-delivery contention.
- §2b names the existing event fanout exactly once and specifies a lossless outcome vocabulary for degraded success/failure, independently of prior partial history.
- Dedicated fractional timeout validation leaves integer-only
--delayvalidation unchanged. Existing 48-KiB cap and potential long short-line writes are described accurately; no fabricated size-cap dependency.
Testing: The plan adds the predeadline-partial-failure/recovery/busy-at-deadline regression, the ownership→DB microtask window, valid F1 traces, no-op/rejection/eviction cases, exact event fanout, and newer-force/older-held ordering. Existing maintainer-corner and isolated live-Tower/PTY evidence requirements remain. These are planned tests, not executed results; no implementation exists yet.
Human decisions still required:
- No catch-up force after Tower restart (even if the deadline was future), an unavailable/unwritable target at deadline, or replacement while queued. Keep the durable body for ordinary gated delivery instead.
- After a possibly partial normal write, allow one sequential forced retry if still held, with warnings and explicit possible duplicate effects, rather than cancelling the user's escalation forever. Successful normal delivery never gets a forced retry.
I find the verified Claude plan refinements addressed and the maintainer requirement preserved. Plan at reviewed immutable revision. Ready for human plan review with the two policies above; no merge, issue closure, checkbox completion, or cleanup authorized.
1 remaining item
- added 15 commits that reference this issue
on Sep 6, 2026
What
Add an
afx send --interrupt-after <seconds>flag: attempt normal gated delivery, hold (as today) while the recipient's prompt is busy, and if it's still undelivered after<seconds>, escalate to an interrupt delivery (Ctrl+C the turn, then deliver) — a bounded, opt-in timeout that force-delivers only after patience runs out.This sits between the two existing shapes:
--interrupt— force now (urgent: deliver immediately, claim-first).--delay <s>— wait<s>, then gated-deliver, never force.--interrupt-after <s>(new) — try gated delivery immediately, keep holding, then force after<s>if still held.Guidance to document: use
--interrupt-afterfor time-sensitive messages (they should land soon, but a clean prompt is preferred if one appears in time); use--interruptfor urgent messages that must land right now.Why
Today the only escape from an indefinite hold is
--interrupt, which force-injects immediately even when the recipient might reach a clean prompt a few seconds later. For time-sensitive-but-not-urgent mail (a cron nudge, a "wrap up soon"), the operator wants "prefer a clean delivery, but don't wait forever." There's no way to express that bounded-patience semantic now — it's all-or-nothing between force-now and wait-forever.Notes
not_before/eligibility columns and escalation-age clock from the 1313 durable---delayround, plus the delayed-interrupt reshape (fire Ctrl+C at due time, body delivers through the gated drainer after the turn).--interrupt-afteris essentially "hold now, arm an interrupt atnow + s."<s>to find a clean prompt. Keep the immediate-gated path the default; document the force semantics on the flag.--interrupt-afterto route its forced write through the same serialized path).--interrupt-afterrow is "will self-resolve," so it shouldn't trip the held-mail starvation owner-notice before its deadline.