fix(web-shell): improve follow-up suggestion handling - #5996
Conversation
|
Thanks for the PR! Template looks good ✓ — all required sections present with bilingual description. On direction: this solves two real UX problems in web-shell — the send button looking disabled when a follow-up suggestion is already visible, and stale suggestions leaking across session switches. Both are straightforward quality-of-life fixes well within the web-shell's scope. No CHANGELOG reference expected here (web-shell follow-up UX is qwen-code–specific). On approach: the scope is tight — three files, ~77 additions, all directly serving the stated goals. Reusing the existing Moving on to code review and testing. 🔍 中文说明感谢贡献! 模板完整 ✓ — 所有必填章节齐全,含中英双语。 方向:解决了 web-shell 中两个真实的用户体验问题 —— 当 follow-up suggestion 已经可见时发送按钮仍显示为不可用,以及切换 session 时旧 suggestion 残留。两者都是合理的体验优化,在 web-shell 范围内。 方案:范围紧凑 —— 三个文件,约 77 行新增,全部服务于目标。复用已有的 进入代码审查和测试 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe diff is clean and well-targeted. Two changes, both serving the stated goals: Send-button follow-up support ( Session-switch cleanup ( No correctness bugs, no security concerns, no AGENTS.md violations. The TestingUnit tests ( TypeScript checks:
Real-scenario (tmux): Not applicable — this PR modifies web-shell (browser) follow-up suggestion UX. The send-button enable/disable state and session-switch suggestion cleanup run in a browser React app, not the terminal CLI. No tmux/CLI invocation can exercise these paths. The unit tests + typechecks above provide the verification for this change. 中文说明代码审查Diff 干净且目标明确。两处改动都服务于既定目标: 发送按钮 follow-up 支持( Session 切换清理( 无正确性 bug,无安全问题,无 AGENTS.md 违规。 测试单元测试( TypeScript 检查: 真实场景(tmux):不适用 —— 本 PR 修改的是 web-shell(浏览器)follow-up suggestion UX。发送按钮的启用/禁用状态和 session 切换 suggestion 清理运行在浏览器 React 应用中,非终端 CLI。没有 tmux/CLI 调用可以覆盖这些路径。上述单元测试 + 类型检查提供了本变更的验证覆盖。 — Qwen Code · qwen3.7-max |
|
All three stages came back clean, so here's where I land: This is a well-scoped fix that solves two real annoyances — the send button looking dead when a follow-up suggestion is right there, and stale suggestions bleeding across session switches. The implementation does exactly what it says: reuses the existing The one thing I'd note: the Approving. ✅ 中文说明三个阶段全部通过,结论如下: 这是一个范围明确的修复,解决了两个真实的体验问题 —— 发送按钮在 follow-up suggestion 可见时看起来不可用,以及旧 suggestion 在 session 切换时泄漏。实现方式与描述一致:复用已有的 一个小注: 批准。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
✅ Real-build verification (tmux + live daemon + Playwright on the actual web-shell SPA)I verified this PR end-to-end on a real build, as a merge reference. Rather than asserting against a re-implementation, I built the actual web-shell artifacts from the PR head, served them with a real Verdict: both behaviors work, and an A/B against the pre-PR build proves the PR is responsible for the change. No regressions observed. Setup
Part 1 — logic unit test + mutation
Part 2 — real UI, identical Playwright script, A/B by buildSame script, same daemon, same fake model — the only variable is the 2 implementation files (PR head vs its parent
Ground-truth from the fake's request log (what the daemon actually submitted to the model):
The 4 control rows (baseline / A1 / C1 / D) are green in both legs, so the harness isn't trivially passing or failing everything — only the PR-specific rows flip. This matches the diff precisely: Scope / not covered
Recommendation: looks good to merge — the headline UX fix and the session-scoping fix both reproduce on the real SPA, and the pre-PR A/B confirms they are this PR's doing. 中文说明✅ 真实构建验证(tmux + 真实 daemon + Playwright 驱动真实 web-shell SPA)作为合并参考,我对本 PR 做了端到端验证。没有用"复刻一份逻辑"来断言,而是用 PR head 构建出真实的 web-shell 产物,由真实的 结论:两个行为都成立;与 PR 之前版本的 A/B 对比证明这些变化确实由本 PR 引入;未发现回归。 环境
第 1 部分 — 逻辑单测 + 变异测试
第 2 部分 — 真实 UI,同一份 Playwright 脚本,按构建做 A/B脚本、daemon、伪模型完全相同,唯一变量是那 2 个实现文件(PR head vs 其父提交
伪端点请求日志(daemon 实际提交给模型的内容)作为铁证:
4 个对照项(baseline / A1 / C1 / D)在两条腿里都绿,说明测试装置不是"全过/全挂"——只有 PR 相关项发生翻转。这与 diff 完全吻合: 范围 / 未覆盖
建议:可以合并 —— 头部 UX 修复与 session 隔离修复都在真实 SPA 上复现,PR 前/后 A/B 也确认是本 PR 的功劳。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running.
Two low-confidence observations for human review (not posted as inline comments): (1) the sidechannelFollowupSuggestion?.sessionId === sessionId check at line 267 selects the undefined sidechannel value when both operands are undefined, instead of falling back to lastFollowupSuggestion — mitigated by transcript store being empty in that state; (2) the session-cleanup effect guard at line 298 misses the disconnect-then-reconnect-to-different-session path — mitigated by multiple safety nets.
— qwen3.7-max via Qwen Code /review
| // sidechannel value, but not the transcript store: the provider resets | ||
| // that store for the new session, and it may already contain the new | ||
| // session's own follow-up suggestion. | ||
| controller.clear(); |
There was a problem hiding this comment.
[Suggestion] The session-switch effect calls controller.clear() and clearSidechannelFollowupSuggestion() but skips clearStoreFollowupSuggestion(store), relying on the provider's deferred async store.reset(). Between this effect running and the store reset completing, the sibling push effect sees the old lastFollowupSuggestion and re-pushes it (since lastPushedPromptIdRef was just reset to undefined), causing the old suggestion to briefly flash in the new session.
Consider adding clearStoreFollowupSuggestion(store) here, matching what the existing clear and onAcceptFollowup callbacks already do. If the new session already has its own suggestion in the store, the push effect will pick it up on the next store notification.
| controller.clear(); | |
| controller.clear(); | |
| clearSidechannelFollowupSuggestion(); | |
| clearStoreFollowupSuggestion(store); | |
| lastPushedPromptIdRef.current = undefined; |
— qwen3.7-max via Qwen Code /review
| it('ignores suggestions that no longer match the editor text', () => { | ||
| expect(getFollowupCompletion('run', 'show me tests')).toBeNull(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] The three new tests cover getFollowupCompletion well (empty editor, matching prefix, non-matching). Two significant behavioral changes introduced by this PR have no test coverage:
- Session-switch cleanup effect (
useDaemonFollowupSuggestion.ts): no test verifies thatcontroller.clear(),clearSidechannelFollowupSuggestion(), and thelastPushedPromptIdRefreset happen when the session changes. - Send-button followup integration (
useComposerCore.tssubmitText): no test covers thetextOverride ?? followupCompletion ?? editorTextprecedence or theonAcceptFollowupRef.current?.('enter', { skipOnAccept: true })call when a followup is submitted via the send button.
— qwen3.7-max via Qwen Code /review
What this PR does
This PR improves web-shell follow-up suggestion handling in two places. First, when a follow-up suggestion is visible, the composer send button now treats the suggestion as sendable content. Clicking send submits the full suggestion when the editor is empty or when the editor text is a matching prefix of that suggestion, matching the existing Enter-key behavior. Second, follow-up suggestions are scoped more carefully across session switches so a suggestion from one session is not carried into another session.
The change also adds focused logic coverage for follow-up completion matching: empty input, matching prefixes, and non-matching text.
Why it's needed
Before this change, users had to press Tab to materialize a visible follow-up suggestion before the send button became usable. This made the send button appear disabled even though the UI was already showing a valid next prompt. In addition, switching from a session with a follow-up suggestion to another session could leave the previous suggestion visible if the next session did not have one.
The updated behavior lets users send the visible follow-up directly and clears only the local/global suggestion state that can leak between sessions, while preserving the new session's own transcript-store suggestion if it has one.
Reviewer Test Plan
How to verify
In web-shell, get a completed response that shows a follow-up suggestion. Confirm the send button is enabled even before pressing Tab, and clicking it sends the full follow-up suggestion. Then type a prefix that matches the suggestion and confirm the send button still submits the full suggestion. Type unrelated text and confirm the suggestion is not used. Switch from a session with a suggestion to a session without one and confirm the old suggestion disappears. Switch from a session without a suggestion to one that has its own follow-up and confirm the new suggestion still appears.
Local verification:
cd packages/web-shell && npx vitest run client/hooks/useComposerCore.test.ts;cd packages/webui && npm run typecheck;cd packages/web-shell && npx tsc --noEmit -p tsconfig.lib.json.Evidence (Before & After)
Before: the send button stayed disabled for a visible follow-up until the user accepted it with Tab, and session switches could retain a stale suggestion from the previous session.
After: the send button can submit the visible follow-up directly, matching Enter behavior, and session switches clear leaked local/global suggestion state without deleting a new session's own suggestion.
Tested on
Environment (optional)
Node 22 workspace, targeted web-shell hook test plus webui and web-shell TypeScript checks.
Risk & Scope
Linked Issues
N/A
中文说明
这个 PR 做了什么
这个 PR 优化了 web-shell follow-up suggestion 的两个行为。第一,当 follow-up suggestion 可见时,composer 的发送按钮会把它视为可发送内容;如果输入框为空,或者当前输入是 suggestion 的匹配前缀,点击发送会直接提交完整 suggestion,这和已有的 Enter 键行为保持一致。第二,切换 session 时更严格地隔离 follow-up suggestion,避免一个 session 的 suggestion 带到另一个 session。
同时补充了聚焦的逻辑测试,覆盖 follow-up completion 的空输入、前缀匹配和不匹配文本三种情况。
为什么需要
修改前,用户必须先按 Tab 将可见的 follow-up suggestion 填入输入框,发送按钮才可用;这会让按钮看起来不可点击,虽然 UI 已经展示了一个有效的下一条 prompt。此外,从有 follow-up 的 session 切到另一个 session 时,如果下一个 session 没有 suggestion,之前的 suggestion 可能残留。
新的行为允许用户直接发送当前可见的 follow-up,并且只清理可能跨 session 泄漏的本地/全局 suggestion 状态,同时保留新 session 自己 transcript store 中的 suggestion。
Reviewer Test Plan
如何验证
在 web-shell 中让一次回复完成并展示 follow-up suggestion。确认不按 Tab 时发送按钮也可用,点击后会发送完整 follow-up suggestion。然后输入 suggestion 的匹配前缀,确认发送按钮仍会提交完整 suggestion。输入不相关文本时,确认不会使用 suggestion。从有 suggestion 的 session 切换到没有 suggestion 的 session,确认旧 suggestion 消失。从没有 suggestion 的 session 切换到一个自身有 follow-up 的 session,确认新 suggestion 仍能展示。
本地验证:
cd packages/web-shell && npx vitest run client/hooks/useComposerCore.test.ts;cd packages/webui && npm run typecheck;cd packages/web-shell && npx tsc --noEmit -p tsconfig.lib.json。前后对比证据
Before:可见 follow-up 在用户按 Tab 接受之前,发送按钮仍是 disabled;切换 session 时可能残留上一个 session 的 suggestion。
After:发送按钮可以直接提交可见 follow-up,行为与 Enter 保持一致;session 切换会清理泄漏的本地/全局 suggestion 状态,同时不会删除新 session 自己的 suggestion。
Tested on
Environment (optional)
Node 22 workspace,运行了定向 web-shell hook 测试,以及 webui/web-shell TypeScript 检查。
风险和范围
Linked Issues
N/A