Skip to content

fix(coding-agent): single-source the interactive queue state - #1705

Closed
snimu wants to merge 13 commits into
snimu/supervised-rename-authorityfrom
snimu/tui-queue-single-source
Closed

fix(coding-agent): single-source the interactive queue state#1705
snimu wants to merge 13 commits into
snimu/supervised-rename-authorityfrom
snimu/tui-queue-single-source

Conversation

@snimu

@snimu snimu commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

What was wrong

The interactive queue was stored three times: the core session's action store (authoritative), the TUI's connection snapshot, and a second TUI-local mirror that optimistically re-applied every mutation. Keeping the copies aligned required identity guards, a sync() pass, and a text-based indexOf fallback — and that fallback had a real bug: with two identical queued messages, editing the later one could silently edit the first.

The fix

  • The TUI now reads the queue from one place (the connection snapshot of the session's actions). The local mirror, optimistic patching, identity guards, sync(), and the indexOf fallback are deleted.
  • Mutations are validated server-side against the original (lane, index, text) tuple, so an edit can never be redirected to a different item with the same text.
  • After an applied move, the selection re-derives deterministically from the refreshed canonical queue (the moved item is at index±1) — no text retargeting, no sync machinery.
  • Queued mutations resolve their target when their turn in the serialized chain begins, so consecutive moves and move-then-edit chains stay correct.

Net −167 lines in the first pass; the review round added the deterministic selection re-derivation (+69/−15).

How it's verified

Reviewer independently simulated server semantics against the real prototype methods: consecutive moves, move-then-edit, rapid unawaited keypresses, duplicate-text edits, selection reset races, and external queue shrink — all correct or safely rejected. New stateful regression asserts a 2→1→0 move/edit chain lands with the right indices. tsgo/biome/focused suites green (86 tests); repo-wide check green. Two-model implement/review loop, approved on second pass.

Stacked on #1702 (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 interactive queue mutation and selection logic in the coding-agent TUI; correctness now depends on session action events staying aligned with mutation responses, though server-side tuple validation limits misapplication risk.

Overview
The interactive TUI no longer keeps a second queued-messages mirror or patches it optimistically after move/edit/delete. Queue reads and the pending-message UI now come from connectionState.sessionActions via getConnectionQueue(), and session_action_update drives display refresh and selection reconciliation.

Queued-message editing targets the selected (lane, index, text) tuple on the daemon instead of re-resolving position with indexOf, which fixes mis-edits when duplicate text appears in the queue. QueueSelection.sync() is replaced by refreshAt, which either reaffirms that exact tuple in the canonical queue or exits browse mode and restores the stashed draft.

After moves, selection is updated from the refreshed queue (with pendingQueueMove blocking conflicting refreshes) rather than swapping entries locally. Session rebind, resync, and switch paths drop refreshConnectionQueue failures and call updatePendingMessagesDisplay directly; tests are expanded for event-driven navigation, duplicate-text edits, and resync races.

Reviewed by Cursor Bugbot for commit 20c2482. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Single-source interactive queue state from connectionState in InteractiveMode

  • Removes the local connectionQueue mirror in InteractiveMode; queue reads now derive from connectionState.sessionActions via the new getConnectionQueue() helper
  • Replaces QueueSelection.sync() with refreshAt(), which verifies the exact lane/index/text tuple instead of retargeting by text, so duplicate-text queued items are no longer confused during move or edit
  • moveQueueSelection and applyQueueSelection now send mutations for the currently selected tuple and refresh selection against authoritative state; failures preserve the user's edit in the editor
  • Adds pendingQueueMove flag to block selection refresh during in-flight moves; refreshQueueSelectionFromState keeps browse position in sync on session_action_update and resync events
  • Risk: QueueSelection.sync() is removed — any out-of-tree callers still using it will break; in-tree tests and callers are migrated to refreshAt()

Macroscope summarized 20c2482.

Linear: ENG-5649

Comment thread packages/coding-agent/src/modes/interactive/interactive-mode.ts
Comment thread packages/coding-agent/src/modes/interactive/interactive-mode.ts Outdated
Comment thread packages/coding-agent/src/modes/interactive/queue-selection.ts
Comment thread packages/coding-agent/src/modes/interactive/interactive-mode.ts Outdated
Comment thread packages/coding-agent/src/modes/interactive/interactive-mode.ts
Comment thread packages/coding-agent/src/modes/interactive/interactive-mode.ts Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 384a702. Configure here.

Comment thread packages/coding-agent/src/modes/interactive/interactive-mode.ts
Comment thread packages/coding-agent/src/modes/interactive/interactive-mode.ts
@snimu

snimu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #1815: GitHub's stack lock prevents retargeting stacked PRs, so this PR was recreated as a plain PR against main (same branch, same content, all review threads here are resolved). Please review/merge #1815 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant