refactor(coding-agent): derive available connection models on read - #1826
refactor(coding-agent): derive available connection models on read#1826snimu wants to merge 124 commits into
Conversation
…onfig-cache-reset
…flow-patterns-export
…e-daemon-lookup-fake
…ot-dispose-timeout
…onfig-cache-reset
…flow-patterns-export
…e-daemon-lookup-fake
![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 63cbcd3. Configure here.
| lane[selected.index] === selected.text && | ||
| target >= 0 && | ||
| target < lane.length | ||
| ) { |
There was a problem hiding this comment.
Queue selection lost on reorder race
Medium Severity
Reordering a queued message can drop browse selection when the mutation response arrives before the matching session_action_update. moveQueueSelection awaits sessionEventQueue (already-queued events only), then refreshAt requires the new index to already be in connectionState. On a stale queue that check fails and resets the cursor, so the editor jumps back to the draft even though the move applied.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 63cbcd3. Configure here.
| this.queuedMessagesContainer.clear(); | ||
| this.connectionQueue = { steering: [], followUp: [] }; | ||
| this.pendingQueueEdit = undefined; | ||
| this.pendingQueueMove = false; |
There was a problem hiding this comment.
Session reset keeps old queue images
Medium Severity
resetCurrentSessionRenderState no longer clears the session queue before pruning pasted images. liveImageMarkerIds now reads connectionState.sessionActions, so on paths such as renderCurrentSessionState the previous session's queued [image #N] markers still count as live and those blobs are not dropped.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 63cbcd3. Configure here.
![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:2815
rebindCurrentSession and renderCurrentSessionState display and edit queued messages from a pre-subscription snapshot, so a session_action_update that arrives during either transition is missed and the UI retains stale queue state until another queue event or resync. Restore an authoritative queue read after subscribing (and after rendering a replacement snapshot) before updating the queued-message display.
🚀 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 2815:
`rebindCurrentSession` and `renderCurrentSessionState` display and edit queued messages from a pre-subscription snapshot, so a `session_action_update` that arrives during either transition is missed and the UI retains stale queue state until another queue event or resync. Restore an authoritative queue read after subscribing (and after rendering a replacement snapshot) before updating the queued-message display.
Evidence trail:
packages/coding-agent/src/modes/interactive/interactive-mode.ts:2891-2899, 5087-5124, 5327-5375, 6569-6584 at 63cbcd3489d2e6c857b0a63d076e23834bac3b3d; git_diff MERGE_BASE REVIEWED_COMMIT -- packages/coding-agent/src/modes/interactive/interactive-mode.ts
| private async _runScheduledPostCompactionContinue(settlement: PostCompactionContinuationSettlement): Promise<void> { | ||
| while (this._postCompactionContinuationScheduled && this._postCompactionContinuationSettlement === settlement) { | ||
| await this.agent.waitForIdle(); | ||
| await this.waitForRetry(); |
There was a problem hiding this comment.
🟠 High core/agent-session.ts:7658
Overflow recovery stalls permanently because _runScheduledPostCompactionContinue waits on waitForRetry() before calling agent.continue(), while that retry promise is only resolved after the retry starts and finishes. Remove this wait so the scheduled continuation can issue the retry.
await this.agent.waitForIdle();
- await this.waitForRetry();
await this._waitForRefineIdle();🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/agent-session.ts around line 7658:
Overflow recovery stalls permanently because `_runScheduledPostCompactionContinue` waits on `waitForRetry()` before calling `agent.continue()`, while that retry promise is only resolved after the retry starts and finishes. Remove this wait so the scheduled continuation can issue the retry.
Evidence trail:
packages/coding-agent/src/core/agent-session.ts:7655-7696 @ 63cbcd3489d2e6c857b0a63d076e23834bac3b3d; packages/coding-agent/src/core/agent-session.ts:10681-10686 @ 63cbcd3489d2e6c857b0a63d076e23834bac3b3d; packages/coding-agent/src/core/agent-session.ts:8408-8435 @ 63cbcd3489d2e6c857b0a63d076e23834bac3b3d; packages/coding-agent/src/core/agent-session.ts:8592-8601 @ 63cbcd3489d2e6c857b0a63d076e23834bac3b3d; packages/coding-agent/src/core/agent-session.ts:3667-3696 @ 63cbcd3489d2e6c857b0a63d076e23834bac3b3d; packages/coding-agent/src/core/agent-session.ts:10920-10924,10947-10954 @ 63cbcd3489d2e6c857b0a63d076e23834bac3b3d
| /** 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:434-450, 453-497, 897-910, 1002-1020, 1102-1117 at 63cbcd3489d2e6c857b0a63d076e23834bac3b3d
|
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
The TUI stored
connectionModels— a filtered copy of the model catalog x configured providers — with its own assignment, clearing, and versioned return paths. A pure function of two other fields, stored, and therefore synchronizable and driftable (audit: duplicate sources of truth, dup-truth.md finding 4).The fix
The stored copy and its sync paths are deleted; the available-model list derives on read with the identical predicate (
catalog.filter(m => providers.has(m.provider))). The catalog, provider set, and async-refresh generation guard remain — they are source data and stale-request protection, not copy-sync. +17/−22.How it's verified
Reviewer confirmed all 4 former field sites converted (zero references remain), predicate character-identical including the invalidate and stale-generation cases, derivation off any render hot path, and the retained machinery guards source data only. 174/174 focused; full CI-style suite identical failing set to stack base. Two-model implement/review loop, approved first pass.
Stacked on #1739 (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
Touches compaction continuation, cross-daemon messaging, RPC transport semantics, and supervised renames—core loop behavior with behavior changes if edge cases (cancellation during queued-work wait, long RPC calls) were wrong.
Overview
Interactive TUI stops mirroring the model catalog and message queue in local fields: available models and queued steering/follow-ups are derived from
connectionStateon read, and “new chat” hints use message count instead of a separatesessionHasMessagesflag. Queue browsing/editing reconciles selection against server state viaQueueSelection.refreshAtrather than optimistic local patches.Agent session rewrites post-compaction continuation to run immediately (no 100ms timer), waiting on idle/retry/refine, queued-work pauses, compaction, and session-input pumps before calling
agent.continue(). RLM child roster snapshots are centralized onAgentSession(duration, preview, tools, activity, recap) with retained children stored as{ session, run? }; daemon attach builds child snapshots by delegating togetRlmChildSnapshots()plus resident active-session ids.Daemon / RPC: remote agent messages connect to the supervisor once, send once, and do not resend if the socket dies after delivery; worker-mode renames apply supervisor-approved names without re-running availability checks. Legacy RLM registries without
sessionDirhydrate via sharedreadLegacyRlmSubagentRegistry(defaults dir from the session file). RpcClient drops fixed request timeouts and surfaces transport death viatransportError, failing pending waits promptly.Smaller cleanups: shared
looksLikeSessionPath, kernel dispose snapshot after interrupting/waiting on execution queue, removal of test-only config-cache hooks andgetOverflowPatterns, and telemetry no longer auto-off underNODE_ENV=test.Reviewed by Cursor Bugbot for commit d34ad9b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Derive
InteractiveModeconnection queue and models fromconnectionStateon readconnectionQueue,sessionHasMessages, andconnectionModelsfromInteractiveMode; queue contents, available models, and new-chat detection are now computed on demand fromconnectionStateviagetConnectionQueue(),getAvailableConnectionModels(), andconnectionState.messageCount.QueueSelection.syncwithrefreshAt(queue, lane, index, expectedText)— selection reconciliation is now strict to lane/index and expected text, no longer retargeting to duplicate text elsewhere.moveQueueSelection,editQueueSelection) now defer to server state and reconcile selection post-events viapendingQueueMovegating, preventing double-apply and races.RpcClientto tracktransportErrorand fail all pending operations viafailPendingOperations; removes per-request client-side timeouts (REFINE_REQUEST_TIMEOUT_MS) so long-running commands are no longer cut off by the client.AgentSessionwith async coordination (_waitForQueuedWorkResume,_runScheduledPostCompactionContinue) that waits on agent/refine/queued-work idle states and honors acontinueAfterSessionInputflag.ReplKernelManager.flushSnapshotForDisposeto interrupt active execution, wait up toSNAPSHOT_EXECUTION_TIMEOUT_MSfor settle, then capture a bounded snapshot instead of racingsnapshotStateagainst a separate dispose timeout.readLegacyRlmSubagentRegistryinrlm-ledger.tsfor tolerant parsing of legacyrlm-subagents.jsonlfiles; daemonbuildRlmChildSnapshotsnow sources snapshots fromroot.runtime.session.getRlmChildSnapshots()and overlays daemonactiveSessionId.clearApiKeyCachefrommodel-registry,clearConfigValueCachefromresolve-config-value,getOverflowPatternsfromoverflow, and theNODE_ENV === "test"telemetry opt-out branch inisTelemetryEnabled.RpcClient.sendno longer enforces per-request timeouts — consumers relying on client-side timeout for long operations must pass explicit timeouts towaitForIdle/promptAndWaitor implement higher-level timeout logic.QueueSelection.refreshAtdrops selection when the item is not at the exact lane/index with matching text, which may surprise callers expecting text-based retargeting.findActiveDaemonSessionSummaryForInteractiveStartupis no longer exported frommain.ts.Macroscope summarized d34ad9b.
Linear ticket: ENG-5660
(ticket linked above)
Supersedes #1741 (recreated as a plain PR against
main; GitHub's stack lock prevented retargeting the stacked PR).