fix(gateway,tui): queue-mode busy guard + queued-turn cancel contract#89078
fix(gateway,tui): queue-mode busy guard + queued-turn cancel contract#89078SebTardif wants to merge 0 commit into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 4, 2026, 8:20 PM ET / 00:20 UTC. Summary Reproducibility: yes. Current main still blocks non-local active-run TUI sends in both sendMessage and canSubmitTuiChatMessage before Gateway queue handling can apply. Review metrics: 3 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Refresh to a clean merge result, keep the Gateway-owned queued-turn cancellation design, renew exact-head proof or maintainer override, and clear security/dependency review before landing. Do we have a high-confidence way to reproduce the issue? Yes. Current main still blocks non-local active-run TUI sends in both sendMessage and canSubmitTuiChatMessage before Gateway queue handling can apply. Is this the best way to solve the issue? Mostly yes. Keeping TUI thin and moving queued-turn cancellation into Gateway is the right owner boundary, but this exact branch still needs clean merge, exact-head proof, and security/dependency clearance. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 4ec7842be0af. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (9 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
2091d97 to
cbc7fa5
Compare
cbc7fa5 to
0c98adc
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
18f883d to
79b1507
Compare
|
Still actively maintained. Rebased onto latest The fix remains necessary on current main: ClawSweeper last rated this silver shellfish and asked for live TUI+Gateway mid-run proof for non-steer modes. Existing proof covers unit tests, build, compiled-dist greps, and gateway startup. Live mid-run TUI proof still needs a long-running provider turn with credentials; happy to add that if a maintainer wants it prioritized. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
This assigned pull request has been automatically marked as stale after being open for 27 days. |
d673c3b to
0dc6764
Compare
CI failure classification:
|
| Check | Result |
|---|---|
| PR touch set | TUI + session queueMode plumbing only (src/tui/*, src/gateway/session-utils*.ts) |
| Failed scenario | Telegram/Crabline native command session targeting (/stop active session), gateway QA path |
| Overlap | No shared code path with TUI editor busy-guard / queueMode submit gate |
Current head 0dc6764 |
QA Smoke CI = SUCCESS, full check set green after rebase onto main |
Follow-ups already landed on this PR (unrelated to that timeout)
CI later failed on tui-command-handlers assertions after merge-with-main introduced addBlockedChatSubmitNotice (coalesceConsecutive: true). That was a real test drift, fixed by rebasing and updating expectations. Not related to native-command-session-target.
steipete
left a comment
There was a problem hiding this comment.
The reported TUI queue-mode bug is valid, but this PR needs a Gateway-owned queued-turn cancellation contract before it is safe to land.
Once TUI forwards a prompt during an active run, Gateway can enqueue it and then terminalize the original chat.send before the queued turn starts. The TUI loses that run as an abort target, so Esc or /stop can report completion while the queued prompt still executes.
Two maintainer-side implementations were tested and rejected:
- Retaining the per-send controller across queue wait breaks terminal dedupe, collect batching, reconnect projection, embedded-run start/timeout ownership, and summarize-overflow behavior.
- Clearing the whole session queue bypasses per-requester and hidden-run authorization, and clearing after active abort races queue drain promotion.
The right pre-merge state is one canonical Gateway queue lifecycle: owner-aware queued identity/cancellation, cleanup before active-run signaling, selected-global-agent scoping, and coherent terminal/dedupe/projection/timeout behavior across followup, collect, and summarize. TUI should still forward active-run prompts to Gateway rather than project queue policy locally.
Verified useful pieces in the maintainer rewrite: real PTY TUI-to-Gateway follow-up delivery, repeated identical TUI prompts not coalesced, transcript slot ordering, and preservation of the older active run/status when queue admission fails. Those pieces are not being pushed while the cancellation contract remains unresolved.
Addressing CHANGES_REQUESTED (queued-turn cancellation contract)Implemented a Gateway-owned cancel identity for turns admitted to followup/collect after What landed
Code map
Tests
Happy to add a live PTY mid-run followup + Esc proof recording if you want that as the final merge gate. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Dependency graph guard clearedThis PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh
|
|
/allow-dependencies-change false positive from native prep push: verified zero dependency files differ from mainline base 7a0188c; remote tree 0466533396c49168397ea452e912f6828ceb7d8a matches the rebased local prep tree. |
|
Thanks @SebTardif — this PR supplied the starting point for the TUI queue-mode fix. During maintainer rebase recovery the fork branch stopped accepting edits, so I recreated the reviewed tree as #100123 and preserved your contribution with The replacement expanded the fix across queued-turn admission, authorization, cancellation, restart cleanup, overflow/collect lifecycle, mixed-version TUI stop, and real PTY proof. It landed in 1b8d837. For future PRs, keeping GitHub’s “Allow edits by maintainers” enabled helps avoid the replacement path when a rebase is needed. Thanks again for the report and implementation direction. |
Fixes #89059
Fixes #90012
What Problem This Solves
Fixes an issue where TUI users sending a prompt during an active run would have the prompt blocked before Gateway queue handling could apply. It also fixes queued prompts becoming unabortable after the original
chat.sendclient run finalized, which could let Esc or/stopreport completion while queued work still executed.Why This Change Was Made
Gateway and the auto-reply queue now own one queued-turn lifecycle across admission, collect-mode aggregation, transcript persistence, cancellation, dispatch, and terminal state. TUI remains a thin client: it forwards mid-run prompts, blocks only while admission is pending, and uses session-scoped abort for Esc and
/stopeven after a queued client run has left local UI state.Queued cancellation remains requester-authorized by device or connection identity. Collect mode keeps cancellation owners in separate batches, retains queue-level cancellation after admission, and retires sibling client identities only when the aggregate has atomically acquired the reply lane.
User Impact
TUI users can send follow-up and collect-mode prompts while an agent is running, intentionally repeat prompt text, cancel queued work reliably, and continue chatting after the queue drains. Failed post-admission bookkeeping no longer produces contradictory accepted-then-error terminal events, and queued transcript/media/context data stays attached to the collected turn.
Evidence
blacksmith-testbox; leasetbx_01kwq9sqcc96ax8hcsevfr44fj(amber-barnacle).0466533396c49168397ea452e912f6828ceb7d8a; remote heade307d2534636a42acb4329c3a2e8afca56385393.pnpm build: passed.pnpm check:changed: passed core/test typechecks, all lint shards, dependency and patch guards, storage/runtime sidecar checks, import-cycle check, webhook ordering, and pairing guards.