fix(web-shell): keep skill slash commands after starting a new session - #6319
Conversation
Starting a new session (the sidebar button, quick action, and the /new, /reset and /clear commands all route through clearSession) ran getConnectionAfterSessionClear, which deleted connection.commands and connection.skills. Nothing repopulated them: the SSE loop returns early on manualSessionClear and the deferred skill-fetch path never re-runs, so before the new session's first prompt the composer fell back to the hardcoded local command list. That list omits skills, so typing "/rev" and pressing Tab would not complete "/review". Preserve the workspace-scoped commands and skills across a clear (skills, custom, MCP-prompt and workflow slash commands all live at the workspace/config level, not the session), and only drop the session-scoped supportedCommands and context snapshots. This keeps skill-backed slash commands autocompleting in the new deferred session before its first prompt — the same guarantee QwenLM#6153 added for the initial deferred connect — while still forcing the next session to refetch fresh metadata. The next session's available_commands_update refreshes the list once it lands.
|
Re-run — incorporating the author's follow-up commit (6fa5c7f) and response. Template looks good ✓ Problem: observed bug, same class as #6153. Direction: aligned — direct continuation of the deferred-connect fix (#6153), closing the gap on the clear path. Web shell slash-command autocomplete is core UX. Approach: minimal and focused. Three targeted code changes (clear path, event handler, session attach). The On the deferred-connect Moving on to code review. 🔍 中文说明重新运行——结合作者的后续提交 (6fa5c7f) 和回复。 模板完整 ✓ 问题:已观测到的 bug,与 #6153 同类。 方向:对齐——#6153 deferred-connect 修复的延续,补上 clear 路径的缺口。Web shell 斜杠命令自动补全是核心 UX。 方案:极简且聚焦。三处定向代码改动。事件处理器和 session attach 中 关于 deferred-connect 路径保留 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
|
Re-run — incorporating commit 6fa5c7f (rejection-path test). Code ReviewThe diff is surgical and consistent across all three code paths.
Session attach (DaemonSessionProvider.tsx): Same pattern — Rejection-path test (6fa5c7f): The new Deferred-connect path (~line 470): Left intentionally at No correctness bugs, no security concerns, no convention violations. TestingUnit tests run in worktree (3 commits, including 6fa5c7f):
New/updated tests locking the fix:
CI on GitHub: all 7 active checks green (ubuntu-latest Test ✓, Classify PR ✓, 4 review-pr checks ✓). Web Shell E2EThis bug requires browser-based web shell interaction (slash menu + Tab autocomplete) to reproduce visually. The provider-level integration test covers the same behavioral guarantee programmatically — it drives the real 中文说明重新运行——结合提交 6fa5c7f(拒绝路径测试)。 代码审查Diff 精准且在三个代码路径间保持一致。
Session attach (DaemonSessionProvider.tsx): 拒绝路径测试 (6fa5c7f): 新测试覆盖 Deferred-connect 路径: 刻意保留 未发现正确性 bug、安全问题或规范违反。 测试worktree 中单元测试(3 个提交,含 6fa5c7f):
CI:全部 7 个活跃检查绿色。 Web Shell E2E该 bug 需要浏览器内 web shell 交互才能视觉复现。Provider 级集成测试以编程方式覆盖了相同行为保障,跨三层五个测试锁住行为,足够。 — Qwen Code · qwen3.7-max |
|
Re-run — after the author's follow-up commit and response to review feedback. This is a clean, well-targeted fix. Two deleted The author's follow-up commit (6fa5c7f) adds the rejection-path test I flagged as missing in the prior round — 160 tests pass (up from 159), typecheck clean, CI all green. The test suite covers: commands survive a clear, session-scoped fields are dropped, undefined-before-clear is handled gracefully, an authoritative empty update clears stale commands, a failed fetch preserves the list, and the full provider flow drives real No concerns remain. Approving. 中文说明重新运行——在作者后续提交和回复审查反馈之后。 这是一个干净、目标明确的修复。删除两行 作者的后续提交 (6fa5c7f) 补充了前次审查指出的拒绝路径测试。作者对 deferred-connect 160 测试通过(较上次增加 1 个),typecheck 通过,CI 全绿。测试覆盖:命令在 clear 后存活、session 级字段被丢弃、undefined-before-clear 被优雅处理、权威空更新清除陈旧命令、失败抓取保留列表、完整 provider 流程驱动真实 无剩余顾虑。批准合并。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Downgraded from Approve to Comment: CI still running.
— qwen3.7-max via Qwen Code /review
…tive Address review feedback on the new-session command fix. Preserving commands across a clear means a later refresh must be able to clear them again when the workspace command list genuinely shrinks to empty, otherwise the preserved entries would keep autocompleting forever. Both refresh paths previously kept the previous list on an empty result (`commands.length > 0 ? commands : current.commands`): - The streamed available_commands_update handler now assigns the mapped commands directly, matching how skills were already handled — the daemon snapshot is authoritative. - The post-attach supported-commands assignment now falls back to the preserved list only when the fetch was skipped or failed (supportedCommands === undefined), not when it returned an empty list. Add tests: an available_commands_update that empties the list clears stale commands; a fulfilled-empty supported-commands fetch after a clear drops the preserved commands; and getConnectionAfterSessionClear is exercised with the commands/skills fields already absent.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Two additional suggestions that don't map to a specific diff line:
Missing test for supportedCommands() rejection path — The supportedCommands !== undefined ternary (line 851) was introduced specifically to handle the case where the supportedCommands() fetch rejects, but no test exercises that rejection path. Both new provider tests use mocks that resolve successfully. A regression reverting to length > 0 would pass all existing tests while silently dropping preserved commands when the fetch fails in production.
Deferred-connect bootstrap uses same length > 0 pattern (~line 470-475, unchanged) — The deferred-connect path still uses deferredSkillCommands.length > 0 / deferredSkills.length > 0 guards to decide whether to overwrite commands/skills. Now that commands survive clear, a fulfilled-but-empty workspaceSkills() result would leave stale commands from the previous session in the deferred state — the same bug class this PR fixes in the other two code paths. Consider keying on the fetch outcome: skillsResult.status === 'fulfilled' rather than deferredSkills.length > 0.
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Add error-path coverage for the post-attach command assignment: when the new session's supportedCommands() rejects, supportedCommands stays undefined and the commands preserved across the clear must survive rather than being wiped. Complements the fulfilled-empty test, which locks that a successful empty snapshot is instead treated as authoritative.
|
Thanks for the two follow-ups. One implemented, one I'd push back on: 1. Missing test for the 2. Deferred-connect 中文两条后续建议,一条已实现,一条我想反驳: 1. 缺 2. 延迟连接的 |
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.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
doudouOUC
left a comment
There was a problem hiding this comment.
LGTM. All review comments have been properly addressed:
- Staleness gap (actions.ts) — Fixed by making
available_commands_updateunconditionally authoritative and usingsupportedCommands !== undefinedas the fallback guard. - Missing undefined-before-clear test (actions.test.ts) — Added, confirms delete calls are harmless no-ops.
- models length pattern (DaemonSessionProvider.tsx) — Correctly scoped out; different lifecycle semantics, no staleness risk.
The workspace-vs-session scoping is well-defined, the three code paths are consistent, and the test coverage locks the behavior against regressions.
ytahdn
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
What this PR does
Preserves the workspace-scoped slash-command list when the user starts a new chat, so skill-backed commands keep autocompleting in the new session before its first prompt. Starting a new session now only discards the session-scoped snapshots (the supported-commands and context payloads that carry the cleared session id), while keeping the derived
commands/skillsthat the composer's slash menu reads.Why it's needed
Every "new session" entry point in the web shell — the sidebar button, the composer quick action, and the
/new,/resetand/clearcommands — routes throughclearSession(), which callsgetConnectionAfterSessionClear. That helper deletedconnection.commandsandconnection.skills. Nothing repopulated them: after the old session's SSE stream closes the connection loop returns early onmanualSessionClear, and the deferred skill-fetch path never re-runs. So in the newly created (second) session, before the first prompt, the composer fell back to the hardcoded local command list, which omits skills. Typing/revand pressing Tab therefore would not complete/review.This is the same class of gap that #6153 fixed for the initial deferred connect (before the very first prompt), except it was triggered on the clear path and left unfixed. Skills, custom commands, MCP-prompt commands and workflow commands are all workspace/config level, not session level, so they stay valid across a clear within the same workspace and should not be dropped.
Reviewer Test Plan
How to verify
qwen serve --web, open the web shell, and let the workspace expose a bundled skill such as/review./revand press Tab — it completes/review./new)./revand press Tab./review, so Tab can't complete it./review, same as the first session.Automated coverage:
packages/webui→npx vitest run src/daemon/session/— 177 tests pass, including:actions.test.ts›getConnectionAfterSessionClear›clears session fields …(assertscommands/skillssurvive, session-scoped fields are dropped).DaemonSessionProvider.test.tsx›keeps workspace skill slash commands after clearing so /review still autocompletes— drives the realclearSessionthrough the provider and assertsconnection.commandsstill containsreview.delete next.commands/delete next.skillsmakes both new tests fail, confirming they lock the behavior.npx tsc --noEmit,eslintandprettier --checkon the changed files all pass.Evidence (Before & After)
The user-visible effect is the slash-menu / Tab completion described above. It is captured behaviorally by the provider-level integration test (real
clearSession): before the change the assertionconnection.commandscontainsreviewfails after a clear; after the change it passes.Tested on
Environment (optional)
Local unit/integration tests via
vitestinpackages/webui.Risk & Scope
available_commands_update, and the session-scopedsupportedCommands/contextare still dropped so metadata is refetched (no stale reuse).Linked Issues
N/A
中文说明
这个 PR 做了什么
在用户新建会话时,保留工作区级的斜杠命令列表,使得技能类命令在新会话首条 prompt 之前仍能自动补全。新建会话现在只丢弃 session 级的快照(带着被清理 sessionId 的 supported-commands 和 context),而保留输入框斜杠面板读取的
commands/skills。为什么需要
Web Shell 里所有"新建会话"入口——侧边栏按钮、输入框快捷动作,以及
/new、/reset、/clear命令——都汇入clearSession(),它会调用getConnectionAfterSessionClear。该函数删除了connection.commands和connection.skills,而且没人回填:旧会话 SSE 流关闭后,连接循环在manualSessionClear处提前返回,抓取技能的 deferred 分支也不会再跑。于是在新建的(第 2 个)会话里、首条 prompt 之前,输入框退化成写死的本地命令列表(不含技能)。此时输入/rev按 Tab 补不出/review。这与 #6153 修复的"初始 deferred 连接(首条 prompt 之前)丢技能命令"是同一类问题,只是触发点在 clear 路径上,之前没修到。技能、自定义命令、MCP prompt、工作流命令都是工作区/配置级、而非会话级,在同一工作区里跨会话依然有效,因此不应被删除。
审阅者测试计划
如何验证
qwen serve --web打开 web shell,工作区暴露一个内置技能如/review。/rev按 Tab —— 补全为/review。/new)。/rev按 Tab。/review,Tab 补不出。/review,与第 1 个会话一致。自动化覆盖:
packages/webui→npx vitest run src/daemon/session/—— 177 测试通过,含:actions.test.ts›getConnectionAfterSessionClear›clears session fields …(断言commands/skills保留、session 级字段被删)。DaemonSessionProvider.test.tsx›keeps workspace skill slash commands after clearing so /review still autocompletes—— 通过 provider 驱动真实clearSession,断言清理后connection.commands仍含review。delete next.commands/delete next.skills加回去,两个新测试都变红,确认测试锁住了行为。npx tsc --noEmit、eslint、prettier --check全部通过。证据(前 / 后)
用户可见效果即上文的斜杠面板 / Tab 补全,由 provider 级集成测试(真实
clearSession)行为化地捕获:修复前"清理后connection.commands含review"断言失败,修复后通过。测试平台
环境(可选)
在
packages/webui用vitest跑本地单测/集成测试。风险与范围
available_commands_update刷新,且 session 级的supportedCommands/context仍被丢弃以强制重取(不会复用陈旧数据)。关联 Issue
N/A