fix(coding-agent): serialize post-compaction continuation on run settlement - #1817
fix(coding-agent): serialize post-compaction continuation on run settlement#1817snimu wants to merge 54 commits into
Conversation
…-blanket-timeouts
…d-rename-authority
…ot-dispose-timeout
…ot-dispose-timeout
…-blanket-timeouts
…d-rename-authority
…ot-dispose-timeout # Conflicts: # packages/coding-agent/src/core/kernel/index.ts # packages/coding-agent/test/kernel-abort.test.ts
…ot-dispose-timeout
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| } | ||
| this.subscribeToAgent(); | ||
| await Promise.all([this.refreshConnectionQueue(), this.refreshHeartbeatCatalog().catch(() => undefined)]); | ||
| this.updatePendingMessagesDisplay(); |
There was a problem hiding this comment.
🟡 Medium interactive/interactive-mode.ts:2802
rebindCurrentSession can leave the queue UI stale when another client queues a message before subscribeToAgent() is installed, because the earlier getState() snapshot is never refreshed and the missed queue event is not replayed. Restore an authoritative getQueue() reconciliation after subscription so those messages become visible and editable.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/modes/interactive/interactive-mode.ts around line 2802:
`rebindCurrentSession` can leave the queue UI stale when another client queues a message before `subscribeToAgent()` is installed, because the earlier `getState()` snapshot is never refreshed and the missed queue event is not replayed. Restore an authoritative `getQueue()` reconciliation after subscription so those messages become visible and editable.
Evidence trail:
packages/coding-agent/src/modes/interactive/interactive-mode.ts:2512-2532, 2785-2804, 5074-5086, 7389-7415 @ ed169d3f5f6385b8a1ca522159f01542c268988c
packages/coding-agent/src/modes/agent-connection/in-process-agent-connection.ts:126-131, 140-142, 208-213 @ ed169d3f5f6385b8a1ca522159f01542c268988c
packages/coding-agent/src/modes/agent-connection/daemon-agent-connection.ts:382-386, 393-400, 1615-1621 @ ed169d3f5f6385b8a1ca522159f01542c268988c
| this._postCompactionContinuationScheduled = false; | ||
| continuation = this.agent.continue(); | ||
| } |
There was a problem hiding this comment.
🟠 High core/agent-session.ts:7685
Messages from agent.continue() are silently lost (never persisted, never delivered to observers) when a concurrent refine() disconnects the agent listener between the _waitForRefineIdle() call at the top of the loop and the agent.continue() call inside the commit fence. The first _waitForRefineIdle() passes, but by the time continuation is invoked, _applyRefine has set _refineInFlight and called _disconnectFromAgent(), so the continuation's events have no subscriber.
A second _waitForRefineIdle() check is needed inside the commit fence, just before calling agent.continue(), to ensure no refine application started in the gap.
} else {
this._postCompactionContinuationScheduled = false;
+ await this._waitForRefineIdle();
continuation = this.agent.continue();
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/agent-session.ts around lines 7685-7687:
Messages from `agent.continue()` are silently lost (never persisted, never delivered to observers) when a concurrent `refine()` disconnects the agent listener between the `_waitForRefineIdle()` call at the top of the loop and the `agent.continue()` call inside the commit fence. The first `_waitForRefineIdle()` passes, but by the time `continuation` is invoked, `_applyRefine` has set `_refineInFlight` and called `_disconnectFromAgent()`, so the continuation's events have no subscriber.
A second `_waitForRefineIdle()` check is needed inside the commit fence, just before calling `agent.continue()`, to ensure no refine application started in the gap.
Evidence trail:
packages/coding-agent/src/core/agent-session.ts:7646-7689 @ ed169d3f5f6385b8a1ca522159f01542c268988c
packages/coding-agent/src/core/agent-session.ts:8030-8068 @ ed169d3f5f6385b8a1ca522159f01542c268988c
packages/coding-agent/src/core/agent-session.ts:8199-8215,8303-8310 @ ed169d3f5f6385b8a1ca522159f01542c268988c
packages/coding-agent/src/core/agent-session.ts:3817-3852 @ ed169d3f5f6385b8a1ca522159f01542c268988c
git diff MERGE_BASE REVIEWED_COMMIT -- packages/coding-agent/src/core/agent-session.ts
| if (sessionGeneration !== this.sessionEventGeneration) return; | ||
| await this.sessionEventQueue; | ||
| if (sessionGeneration !== this.sessionEventGeneration) return; | ||
| this.refreshQueueSelectionAt( |
There was a problem hiding this comment.
🟡 Medium interactive/interactive-mode.ts:7034
A successful mutateQueuedMessage() reorder can exit queue-browse mode and restore the saved draft when its session_action_update arrives after the RPC response. refreshQueueSelectionAt() reads the stale connectionState snapshot at the new index, fails the text check, and calls reset(); the delayed event then cannot restore the selection, so subsequent reorders or edits of the moved item are lost. Synchronize with the authoritative post-mutation queue (or apply a guarded optimistic update) before refreshing the selection.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/modes/interactive/interactive-mode.ts around line 7034:
A successful `mutateQueuedMessage()` reorder can exit queue-browse mode and restore the saved draft when its `session_action_update` arrives after the RPC response. `refreshQueueSelectionAt()` reads the stale `connectionState` snapshot at the new index, fails the text check, and calls `reset()`; the delayed event then cannot restore the selection, so subsequent reorders or edits of the moved item are lost. Synchronize with the authoritative post-mutation queue (or apply a guarded optimistic update) before refreshing the selection.
Evidence trail:
packages/coding-agent/src/modes/interactive/interactive-mode.ts:2640-2644, 2658-2660, 7013-7044 at ed169d3f5f6385b8a1ca522159f01542c268988c; packages/coding-agent/src/modes/interactive/queue-selection.ts:63-82 at ed169d3f5f6385b8a1ca522159f01542c268988c; git diff MERGE_BASE REVIEWED_COMMIT -- packages/coding-agent/src/modes/interactive/interactive-mode.ts
| /** Best-effort final snapshot before a graceful dispose, bounded by a timeout. */ | ||
| private async flushSnapshotForDispose(): Promise<void> { | ||
| if (!this.options.snapshot || !this.isRunning) return; | ||
| const pendingExecutions = this.executionQueue; |
There was a problem hiding this comment.
🟠 High kernel/repl-manager.ts:1104
dispose() and shutdown({ snapshot: true }) can wait indefinitely instead of honoring SNAPSHOT_EXECUTION_TIMEOUT_MS. flushSnapshotForDispose() captures executionQueue before waiting, so a concurrent execute() can enqueue a non-terminating request afterward; captureSnapshot() queues behind it, and its timeout does not start until that request finishes. Reserve the queue before awaiting it (or otherwise block new executions) so the final snapshot cannot be overtaken.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/kernel/repl-manager.ts around line 1104:
`dispose()` and `shutdown({ snapshot: true })` can wait indefinitely instead of honoring `SNAPSHOT_EXECUTION_TIMEOUT_MS`. `flushSnapshotForDispose()` captures `executionQueue` before waiting, so a concurrent `execute()` can enqueue a non-terminating request afterward; `captureSnapshot()` queues behind it, and its timeout does not start until that request finishes. Reserve the queue before awaiting it (or otherwise block new executions) so the final snapshot cannot be overtaken.
Evidence trail:
packages/coding-agent/src/core/kernel/repl-manager.ts:454-497, 897-910, 1002-1020, 1102-1126 at ed169d3f5f6385b8a1ca522159f01542c268988c
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ed169d3. Configure here.
| if (this._postCompactionContinuationSettlement === settlement) { | ||
| this._settlePostCompactionContinue(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Continuation identity reused across runners
High Severity
_schedulePostCompactionContinue reuses an unsettled settlement whenever _postCompactionContinuationScheduled is already false, and that flag is cleared before agent.continue() finishes. A later schedule (including auto-compaction during that same run) therefore starts a second runner on the same settlement, so identity checks cannot tell the stale runner from the new one. The first runner can still forget continuation messages or reject the shared settlement, which drops the follow-up turn or fails headless idle waiters.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit ed169d3. Configure here.
| if ( | ||
| this.connectionQueue === queueBefore && | ||
| lane[selected.index] === selected.text && | ||
| target >= 0 && |
There was a problem hiding this comment.
Queue selection races mutation events
Medium Severity
After a queue reorder, selection is refreshed from getConnectionQueue() as soon as mutateQueuedMessage returns. pendingQueueMove suppresses event-driven retargeting until that refresh, and refreshAt drops the selection unless the exact post-move tuple is already in the snapshot. If session_action_update arrives after the RPC response, browse mode exits and chained moves or edits no longer address the item that was just moved.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit ed169d3. Configure here.
|
Superseded: the chained stack was restructured into independent PRs (byte-identical combined tree). A fresh standalone PR for this change follows on the same branch name. |
1 participant
What was wrong
After an automatic compaction, the session decided whether it could continue by re-checking every 100ms on a recursive timer. That timer raced actually-starting runs: it could fire between "check" and "act", collide with a run that had just started, and it woke the process ten times a second during long work for no reason.
The fix
How it's verified
Focused queue/compaction suites green; the goal-continuation-quiescence suite (PR #1610) passes 7/7 untouched — same semantics. Full CI-style suite compared against the stack base: identical failure names (pre-existing environment noise only). Reviewer independently traced the fence/settlement flow, verified run-ownership closure, and re-ran the noisy suite at base in a throwaway worktree to confirm zero delta. Two-model implement/review loop.
Stacked on #1708 (test the whole stack at the leaf; merge base-first).
Note: intentionally no Linear ticket for this cleanup stack, so that check stays red.
Note
Medium Risk
Changes core post-compaction scheduling and RPC/daemon messaging paths where races or duplicate delivery could affect session continuity; supervised rename skips a local validation step that must stay correct.
Overview
This PR bundles several reliability fixes in the coding agent: post-compaction continuation, RPC/daemon client behavior, TUI queue editing, kernel teardown, and supervised session renames.
Post-compaction continuation drops the 100ms polling timer. Continuation now runs immediately and waits on real readiness (agent idle, retries, auto-refine, queued-work pauses, compaction in flight, session commit fence). A
continueAfterSessionInputflag on the continuation settlement controls whether an empty resume should wait for session-owned input before callingagent.continue(). Cancelled or replaced continuations are identity-guarded so stale runners cannot settle or reject newer waiters.Interactive TUI stops mirroring the message queue in a separate
connectionQueue; queued steering/follow-ups come fromconnectionState.sessionActions. Queue browse/edit/move mutations use the selected lane/index/text directly (viarefreshAt) instead of re-resolving by text or optimistically patching a local mirror.RPC client removes blanket 30s command timeouts (including refine) and default idle/event-collection timeouts; optional timeouts remain. Transport failures (process error, stdout close, stop) fail all pending requests and event waiters.
promptnow validates the RPC response.Daemon: remote
send_messageretries only until the supervisor connection is established—one send per call; disconnect after send surfaces an error without resend. Worker/supervised rename commands apply supervisor-approved names viaapplyStateSessionNamewithout re-running local availability checks.Python kernel graceful dispose interrupts in-flight work, waits briefly on the execution queue, then captures a final snapshot or skips teardown if hung—
SNAPSHOT_DISPOSE_TIMEOUT_MSis removed.Reviewed by Cursor Bugbot for commit ed169d3. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Serialize post-compaction continuation on run settlement in
AgentSession_runScheduledPostCompactionContinuenow loops while scheduled, waiting for agent/refine idle and queued-work resume before callingagent.continue()continueAfterSessionInputflag toPostCompactionContinuationSettlementso continuation can be deferred until pending session input completes; the flag persists across compaction abort/reconnect cycles in agent-session.tssend,refine, andwaitForIdlenow pend until response or transport failure, withfailPendingOperationsrejecting all in-flight waits on process error or stdout closeconnectionQueuemirror inInteractiveModewith reads fromconnectionState.sessionActions;QueueSelection.syncis replaced byrefreshAtwhich requires exact lane/index/text match or resets selectionReplKernelManager.flushSnapshotForDisposeto interrupt active executions and wait for the execution queue to settle before capturing the final snapshotsend()andrefine()no longer time out automatically — callers that previously relied on per-request timeouts will pend indefinitely until a response or transport error;QueueSelectionnow drops selection instead of retargeting to matching text elsewhere in the queue📊 Macroscope summarized ed169d3. 13 files reviewed, 5 issues evaluated, 0 issues filtered, 4 comments posted
🗂️ Filtered Issues
Linear: ENG-5651
Supersedes #1716 (recreated as a plain PR against
main; GitHub's stack lock prevented retargeting the stacked PR).