Keep wait live after interrupt-with-follow-up - #858
Conversation
TheGreatAxios
left a comment
There was a problem hiding this comment.
Critic · Comment
send_input(interrupt:true) still acks interrupted and keeps wait live until the follow-up report surfaces. Edge paths can still disagree or drop salvage.
Findings
src/subagent/agent-fleet.ts:260—completeAfterInterruptstill deletesforceInterrupted. After this change onlyclose_agent/interrupt_agentset that overlay, so a follow-up reply can undo a laterclose_agentoverlay and freeze wait asdone. Trigger: send_input interrupt, then close_agent, then the follow-up resolves beforecloseOnestamps shutdown.src/subagent/lifecycle-tools.ts:385—onFollowupReplyis the only clearer forfollowupLive. A rejected follow-up never calls it (session-store.ts:822-828), so spawn settlement keeps swallowing original salvage.src/subagent/lifecycle-tools.ts:404—markQueuedhas no matchingclearQueuedwhen the follow-up begins running.projectedStatusthen prefersqueuedover liverunning, so list status and lifecycle disagree.docs/ARCHITECTURE.md:231— still sayssend_inputwithinterrupt:truesets the mailbox interrupt overlay so wait unblocks. That is the old contract this PR inverted.
Notes
interrupt_agent/close_agentstill callfleetRecords.interrupt()at the call sites.- Happy-path tests in
agent-fleet.test.tscover wait-before-send and post-send timeout → done. - Stale comment at
src/subagent/lifecycle-tools.ts:311still claims the overlay flip is the same as send_input interrupt.
TheGreatAxios
left a comment
There was a problem hiding this comment.
Primary · Comment
send_input(interrupt:true) keeps wait live until the follow-up settles on the happy path, but wait/list can still disagree and close/reject paths can drop the overlay or salvage. Cannot request-changes on our own PR; these should-fix items land in follow-up commits on this branch.
Findings
src/subagent/agent-fleet.ts:260—completeAfterInterruptstill deletesforceInterrupted, so a follow-up reply can undo a laterclose_agentoverlay and freeze wait asdoneinstead ofinterrupted.src/subagent/lifecycle-tools.ts:385— follow-up rejection never clearsfollowupLive, so spawn settlement swallows original salvage.src/subagent/lifecycle-tools.ts:404—markQueuedhas no matchingclearQueuedwhen the follow-up begins running, so list can showqueuedwhile lifecycle isrunning.docs/ARCHITECTURE.md:231— still describes the old interrupt-overlay contract this PR inverted.
Notes
- Happy path holds: tool ack stays
interrupted; wait/list/resume agree while the follow-up is running; follow-up report surfaces asdone. - CI on this PR is green (prettier, eslint, typecheck, build-and-test).
TheGreatAxios
left a comment
There was a problem hiding this comment.
Critic · Comment
Follow-up commits close the prior overlay/salvage/queued holes. One remaining should-fix: an uncollected close overlay still loses to a successful follow-up completed stamp.
Findings
src/subagent/agent-fleet.ts:330—projectedStatusreturns terminal session status beforeforceInterrupted. Trigger: no in-flight wait; send_input interrupt; close_agent starts; follow-upcomplete()s while close is still held; later wait/list showdonenotinterrupted. The new close-window test does not catch this because in-flight wait freezes interrupted first.docs/ARCHITECTURE.md:231— leftover “does not write a mailbox overlay” in the same bullet the follow-up edited.
Notes
- Prior items 1–3 are closed with tests. interrupt_agent comment matches. New ARCHITECTURE sentence matches; the old sentence in the same bullet does not.
TheGreatAxios
left a comment
There was a problem hiding this comment.
Primary · Comment
Follow-up commits on this branch close the review findings. Happy path still leaves wait live until the follow-up settles as done; close/interrupt overlays stay interrupted even if a follow-up later stamps the session completed.
Findings
None remaining from the prior should-fix list.
Notes
projectedStatusconsultsforceInterruptedbefore terminal session status.- Rejected follow-up clears
followupLiveso salvage can attach. - Queued overlay clears when the follow-up is admitted.
deferred<void>in the new tests is nowdeferred<undefined>so eslint--max-warningsdoes not fail CI.bun test src/subagent/agent-fleet.test.ts: 75 pass.
An interrupt overlay freezes wait as interrupted, so a queued follow-up never surfaces even after it completes. Mark the follow-up lane instead and leave wait running until that turn settles.
A close or interrupt wait overlay must stay interrupted until collected, even if a followup later stamps the session completed. send_input interrupt does not set that overlay, so a happy-path followup wait still collects done.
Main asserted the wait mailbox flipped to interrupted as soon as send_input interrupt:true queued a followup. Wait now stays live until that followup settles; the leftover interrupted stamp remains on the session.
dae5b48 to
58f9bab
Compare
Summary
send_input(interrupt: true)leaves wait live until the queued follow-up settles, instead of freezing the mailbox as interruptedVerification
bun test src/subagent/agent-fleet.test.ts: 75 pass, 0 failbunx eslinton the touched subagent files: no errors (pre-existing warnings only)deferred<void>in a new test (fixed on the latest push)Fixes CL-7331