feat(web-shell): friendlier Esc interruption + queued-prompt UX - #6025
Conversation
The auto-drain effect popped a queued prompt, called setQueuedPrompts, then submitted via setTimeout(0). Because the daemon flips streamingState asynchronously, the setState re-render could re-run the effect and pop a second prompt before the first registered as streaming — both submitted back-to-back and the first was lost. Arm an "awaiting turn start" gate synchronously at pop so the re-run is blocked until streamingState goes non-idle, released by a dedicated effect with a safety-net timer for a prompt that never streams (e.g. a queued slash command). Cleanup no longer cancels/re-queues the pending submit while the gate is armed.
|
Thanks for the PR! Template looks good ✓ On direction: this is a solid UX improvement — accidental Esc cancellation of a streaming turn is a real pain point, and the two-press confirmation pattern is well-established (editors, terminals). The queue-drain fix (race condition dropping queued prompts) is a genuine correctness issue. CHANGELOG has cancellation-related work in CLI/core but no prior web-shell Esc handling — this fills a gap. On approach: the scope feels right. Extracting Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 方向：这是一个扎实的 UX 改进——流式输出时误触 Esc 取消是真实痛点，两次确认模式在编辑器和终端中已有成熟先例。队列出队竞态修复也是真正的正确性问题。CHANGELOG 中有 CLI/core 的取消相关改动，但 web-shell 的 Esc 处理此前没有——本 PR 填补了这个空缺。 方案：范围合理。将 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe implementation is well-structured. Three things stand out as good decisions:
No correctness bugs or security issues found. One minor observation:
Test ResultsAll 564 tests pass across 40 test files — including the 24 new ones: TypeScript check: clean, no errors. Real-Scenario TestingThis PR targets the web-shell (browser app requiring vite dev server + daemon). The tmux-based CLI testing workflow doesn't apply here — Esc key interactions, countdown ring animations, and queue drain timing are browser-only behaviors that can't be meaningfully exercised from a terminal. The unit tests cover the decision logic; visual/interactive verification needs manual testing in the browser. 中文说明代码审查实现结构良好。三个值得肯定的设计决策：
未发现正确性 bug 或安全问题。一个小观察：
测试结果564 个测试全部通过，涵盖 40 个测试文件——包括 24 个新增测试： TypeScript 检查：无错误。 真实场景测试本 PR 针对 web-shell（需要 vite 开发服务器 + daemon 的浏览器应用）。基于 tmux 的 CLI 测试工作流不适用于此——Esc 键交互、倒计时环动画和队列出队时序是纯浏览器行为，无法在终端中有意义地验证。单测覆盖了决策逻辑；视觉/交互验证需要在浏览器中手动测试。 — Qwen Code · qwen3.7-max |
|
This is a well-executed PR. The motivation is clear (accidental Esc cancels are a real annoyance), the implementation is clean, and the test coverage is solid. Going back to my independent proposal: I would have kept the Esc logic and queue drain inline in App.tsx. This PR does better — extracting The only thing I can't verify programmatically is the browser-side experience (countdown ring animation, two-press timing feel, transcript marker appearance) — that needs manual testing. The unit tests cover the decision logic thoroughly, and all 564 existing + new tests pass. LGTM. Approving. ✅ 中文说明这是一个执行良好的 PR。动机清晰（误触 Esc 取消是真实的困扰），实现干净，测试覆盖充分。 回到我的独立方案：我会把 Esc 逻辑和队列出队保留在 App.tsx 内联。本 PR 做得更好——将 唯一无法程序化验证的是浏览器端体验（倒计时环动画、两次按键时序手感、取消标记外观）——这需要手动测试。单测充分覆盖了决策逻辑，全部 564 个新旧测试通过。 LGTM。批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Qwen Code review did not complete successfully. Qwen review aborted with an API error before posting comments. See workflow logs. |
✅ Maintainer local verification — built & exercised against a real browserI built this PR head ( How I verified
Real-browser E2E (the headline UX, in an actual Chromium against the built SPA)
Mutation A/B (tests are load-bearing, not vacuous)
Each mutation broke exactly its targeted assertions and nothing else; reverting → all green again. Notes on the change (read-through)
Optional / non-blocking cleanups
中文说明（点击展开）✅ 维护者本地验证 —— 已构建并在真实浏览器中跑通我在独立 worktree 里构建了该 PR 头（ 验证方式
真实浏览器 E2E（核心交互，在真实 Chromium 中对构建出的 SPA 验证）
变异 A/B 测试（测试是承重的，不是空过场）
每个变异都恰好只破坏其针对的断言、其它不受影响；还原后全部恢复绿色。 对改动的通读说明
可选 / 不阻塞的清理建议
|
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
Automated code review — no high-confidence critical findings in the changed code at this commit.
Reviewed the two-press Esc confirm refactor, decideEscapeIntent() pure function, canDrainQueue() gate, and QueuedPromptDisplay extraction. The queue drain race is addressed by arming awaitingTurnStartRef before setState triggers a re-render. Streaming takes priority over clearing text in the Esc intent logic. No critical issues found.
Generated by Claude Code
Behavior-preserving cleanups addressing review feedback on the Esc-interruption and queued-prompt changes: - Remove the now-dead queue.footer i18n key (EN + ZH) and the unreferenced .queuedHint CSS, orphaned when the Esc-clears-queue behavior was dropped. - Co-locate the queued-prompt styles in QueuedPromptDisplay.module.css instead of reaching into the parent App.module.css. - Make the Esc confirm-window constants the single source of truth: export them from escapeIntent.ts and drive the countdown-ring duration from one of them via a CSS custom property. - Nudge the queue-drain safety net with a dedicated tick counter instead of cloning queuedPrompts, so it no longer re-renders the composer for a no-op. - Drop a redundant !compact guard in StatusBar left over from flattening a ternary. - Document the pop/gate-arm ordering invariant in the drain effect.
| if (source === 'prompt_cancelled') { | ||
| return ( | ||
| <div className={styles.cancelled} role="status"> | ||
| <span>{t('turn.stopped')}</span> |
There was a problem hiding this comment.
[Critical] Every prompt_cancelled block now renders as You cancelled this request, but the bridge publishes the same event for peer/client cancellations and for reason: 'forward_failed'. In those paths this UI tells the current user they cancelled work they did not cancel, hiding transport failures or another client's stop. Since content is already the neutral localized cancellation text from transcriptBlocksToDaemonMessages, render that unless originator/reason is preserved and checked.
| <span>{t('turn.stopped')}</span> | |
| <span>{content}</span> |
— GPT-5 via Qwen Code /review
| // While the gate is armed the re-run is already blocked, so let the | ||
| // pending submit fire — don't cancel it or re-queue. Only when unarmed | ||
| // (a genuine dependency change before submit) restore the prompt. | ||
| if (!awaitingTurnStartRef.current) { |
There was a problem hiding this comment.
[Suggestion] While awaitingTurnStartRef.current is true, this cleanup intentionally skips clearing the pending setTimeout(0). That handles the benign rerender after popNextQueuedPrompt(), but it also lets the stale submit fire after real invalidations such as pendingApproval, interactionBlocked, or connected changing before the timeout executes. Those cases used to cancel/requeue the popped prompt; now the prompt can still call handleSubmit under a blocked/offline state and be lost. Please distinguish the benign queue-state rerender from real dependency invalidations, or revalidate against live refs before submitting and requeue on failure.
— GPT-5 via Qwen Code /review
| }, TURN_START_GATE_SAFETY_MS); | ||
|
|
||
| drainingQueueRef.current = true; | ||
| let sent = false; |
There was a problem hiding this comment.
[Critical] Silent prompt loss when handleSubmit throws during auto-drain.
The drain's setTimeout(0) callback wraps handleSubmit in try/finally with no catch. When the awaitingTurnStart gate is armed, the cleanup deliberately skips re-queuing — so if handleSubmit throws (e.g., connection drops between the canDrainQueue check and the submit), the prompt has already been popped via popNextQueuedPrompt() but is neither submitted nor re-queued. It vanishes silently with no error toast and no recovery path.
Before this PR, the cleanup always re-queued unsent prompts unconditionally. The new gate-armed path breaks that safety net.
| let sent = false; | |
| const timer = setTimeout(() => { | |
| drainSubmitTimerRef.current = null; | |
| sent = true; | |
| try { | |
| handleSubmit(nextPrompt.text, nextPrompt.images); | |
| nextPrompt.onComplete?.(); | |
| } catch (err) { | |
| queuedPromptsRef.current = [nextPrompt, ...queuedPromptsRef.current]; | |
| setQueuedPrompts(queuedPromptsRef.current); | |
| } finally { | |
| drainingQueueRef.current = false; | |
| } | |
| }, 0); |
— qwen3.7-max via Qwen Code /review
| queuedPromptsRef.current = []; | ||
| setQueuedPrompts([]); | ||
| drainingQueueRef.current = false; | ||
| awaitingTurnStartRef.current = false; |
There was a problem hiding this comment.
[Suggestion] Session switch does not reset Esc armed state.
This effect clears queue, drain, and timer state on session switch — but does not call resetEscapeState(). If the user arms cancel (first Esc) on session A while it's streaming, then switches to session B which is also streaming, the escStreamingBoundary boolean stays true and the boundary effect doesn't re-fire. The armed state (cancelArmed, escArmedActionRef, escapeTimerRef) persists into the new session for up to 2 seconds.
During that window, a single Esc in session B hits decideEscapeIntent with armed: 'cancel' and returns { kind: 'cancel' } — bypassing the two-press confirmation that this PR specifically introduces to prevent accidental cancellation.
| awaitingTurnStartRef.current = false; | |
| awaitingTurnStartRef.current = false; | |
| if (awaitingTurnStartTimerRef.current) { | |
| clearTimeout(awaitingTurnStartTimerRef.current); | |
| awaitingTurnStartTimerRef.current = null; | |
| } | |
| resetEscapeState(); |
Adding resetEscapeState here (and to the dep array — it's a stable useCallback with [] deps) ensures the two-press safety is never silently downgraded across sessions.
— qwen3.7-max via Qwen Code /review
| if (e.defaultPrevented || e.isComposing) return; | ||
| const live = escLiveRef.current; | ||
|
|
||
| if (e.key !== 'Escape') { |
There was a problem hiding this comment.
[Suggestion] Modifier-only key presses (Shift, Ctrl, Alt, Meta) disarm the two-press confirm.
Any keydown where e.key !== 'Escape' triggers resetEscapeState(). Modifier-only presses have e.key values like 'Shift', 'Control', 'Alt', 'Meta' — all of which enter this branch and disarm the pending cancel/clear. A user who presses Shift while reaching for the second Esc loses the armed state and must start over.
| if (e.key !== 'Escape') { | |
| if (e.key !== 'Escape') { | |
| if (escArmedActionRef.current !== null && e.key.length === 1) { | |
| resetEscapeState(); | |
| } |
Filtering on e.key.length === 1 (or checking !['Shift', 'Control', 'Alt', 'Meta'].includes(e.key)) lets the user shift their grip between the two presses without losing the armed state.
— qwen3.7-max via Qwen Code /review
4 participants
What this PR does
While the web-shell is streaming a turn, pressing Esc now arms a two-press confirmation instead of cancelling immediately: the send button becomes a stop control with a countdown ring and a "Press Esc again to stop" hint, and a second Esc within the window confirms the cancel. When nothing is streaming and the composer has text, the same two-press gesture clears the input instead. A cancelled turn now leaves a clear "You cancelled this request" row in the transcript, so it is obvious why the turn ended. Finally, cancelling a turn that has messages queued behind it no longer strands them — once the cancelled turn settles, the next queued message is sent automatically, and the queue keeps flowing one turn at a time.
Why it's needed
Before this change a single Esc cancelled a streaming turn with no confirmation and no visual feedback, which made accidental interruptions easy and left the user unsure whether anything had happened. Queued messages were also abandoned when the active turn was cancelled, forcing the user to retype and resend them. Making interruption a deliberate, legible two-press gesture and keeping the queue draining after a cancel removes both of those rough edges. The change also fixes a pre-existing race in which a burst of synchronous re-renders could pop two queued prompts in the same tick and silently drop one.
Reviewer Test Plan
How to verify
Start a turn and let it stream, then press Esc once: the send button should turn into a stop control with a countdown ring and a "Press Esc again to stop" hint, and the turn should keep running. Press Esc a second time within the window and the turn cancels, leaving a "You cancelled this request" row in the transcript. Repeat the first Esc but wait out the window without a second press — the affordance should disarm on its own and the turn should continue uninterrupted.
With no turn streaming, type some text into the composer and press Esc twice: the first press shows the clear hint, the second clears the input.
Queue several messages while a turn is running, then cancel that turn: the next queued message should send automatically once the cancelled turn settles, with the remaining queue preserved and advancing one turn at a time. While a drained message is still pending, switch sessions or navigate away and confirm nothing fires into the wrong or torn-down session.
Evidence (Before & After)
Before
After
Tested on
Environment (optional)
Local web-shell dev server (vite) against a running daemon (
cli serve).Risk & Scope
Linked Issues
中文说明
这个 PR 做了什么
在 web-shell 流式输出某个回合时，按 Esc 不再立即取消，而是变成两次确认：发送按钮会变成一个带倒计时环的停止控件，并显示"再按一次 Esc 停止"的提示，在窗口期内按下第二次 Esc 才确认取消。当没有任何回合在流式输出、而输入框里有文字时，同样的两次手势改为清空输入。被取消的回合现在会在对话记录里留下一行清晰的"你已取消请求"，让人一眼看出回合为何结束。最后，取消一个后面还排着消息的回合不再会丢弃这些消息——被取消的回合结束后，下一条排队消息会自动发送，队列继续逐回合推进。
为什么需要
在此改动之前，单次 Esc 会在没有任何确认、也没有任何视觉反馈的情况下取消流式回合，这让误触很容易发生，用户也不确定到底发生了什么。排队消息还会在当前回合被取消时被丢弃，迫使用户重新输入并发送。把中断变成一个刻意、可读的两次手势，并在取消后让队列继续流转，消除了这两处粗糙体验。该改动同时修复了一个既有竞态：一连串同步重渲染可能在同一个时钟周期内弹出两条排队消息并悄悄丢掉其中一条。
评审验证计划
如何验证
开始一个回合并让它流式输出，然后按一次 Esc：发送按钮应变成带倒计时环的停止控件并显示"再按一次 Esc 停止"提示，回合应继续运行。在窗口期内按第二次 Esc，回合取消，对话记录里留下一行"你已取消请求"。重复第一次 Esc，但不按第二次、等待窗口期结束——提示应自行解除，回合不受影响继续。
在没有回合流式输出时，在输入框里输入一些文字并按两次 Esc：第一次显示清空提示，第二次清空输入。
在一个回合运行时排入若干消息，然后取消该回合：被取消的回合结束后，下一条排队消息应自动发送，其余队列保留并逐回合推进。当某条已出队消息仍在等待发送时，切换会话或离开页面，确认不会有消息被发送到错误的或已销毁的会话里。
证据（前后对比）
测试平台
运行环境（可选）
本地 web-shell 开发服务器（vite），对接运行中的 daemon（
cli serve）。风险与范围