Skip to content

fix(web-shell): show skill slash commands (e.g. /review) before first prompt - #6153

Merged
wenshao merged 3 commits into
QwenLM:mainfrom
wenshao:fix/web-shell-skill-completion-before-first-prompt
Jul 2, 2026
Merged

fix(web-shell): show skill slash commands (e.g. /review) before first prompt#6153
wenshao merged 3 commits into
QwenLM:mainfrom
wenshao:fix/web-shell-skill-completion-before-first-prompt

Conversation

@wenshao

@wenshao wenshao commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Restores skill-backed slash-command autocompletion in the Web Shell composer before the first prompt is sent. Typing /rev now suggests /review (and every other skill) immediately on a fresh session, instead of only after the first message.

The deferred connect path now fetches the session-less /workspace/skills status alongside /workspace/providers and seeds connection.commands / connection.skills from it. The full session-scoped supported-commands snapshot — which additionally carries custom, MCP-prompt and workflow commands — still replaces this list once the first prompt creates a session, so nothing is lost.

Why it's needed

Since session creation was deferred until the first prompt (#6066), the deferred connect path reports connected but only fetches workspace providers; it never populates the slash-command list. Before sending a message the composer therefore falls back to the hardcoded local command list (getLocalCommands), which contains only the ~32 built-ins and omits every skill. As a result /rev did not autocomplete to /review on a freshly opened Web Shell, and no skill was completable until after the first prompt. The completion matching itself was fine ("review".includes("rev")); the command list was simply empty of skills.

/workspace/skills is answered by the daemon from Config's SkillManager without a live session (same class as the /workspace/providers call the deferred path already makes) and includes bundled skills such as review, so it is the natural session-less source for these commands.

Reviewer Test Plan

How to verify

  1. Start qwen serve --web and open the Web Shell on a fresh session (do not send any message yet).
  2. Type /rev in the composer.
    • Before: no /review suggestion appears (only the hardcoded built-ins complete, e.g. /hel/help).
    • After: /review (and other skills) appear in the completion menu.
  3. Send any prompt, then type /rev again — /review completes in both builds (the session snapshot path was already working; this PR fixes only the pre-first-prompt state).

Automated coverage:

  • packages/webui/src/daemon/session/mappers.test.ts — new mapWorkspaceSkills unit tests (undefined → empty; skills → skill slash commands with descriptions/argument hints).
  • packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx — new test asserting the deferred connect path populates connection.commands/skills with a bundled review skill and still creates no session.
$ npx vitest run   # in packages/webui
Test Files  17 passed (17)
     Tests  242 passed (242)

Lint/format: eslint and prettier --check clean on all four changed files.

Evidence (Before & After)

Behavior is exercised by the new provider test: on the deferred (no-session) connect it now yields connection.commands = [{ name: 'review', … }] and connection.skills = ['review'], whereas before neither was set. No standalone TUI/screenshot capture — this is a data-flow fix verified by unit tests.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Unit tests + lint run on macOS. The change is platform-agnostic TypeScript in the webui data layer; CI covers Windows/Linux.

Environment (optional)

npx vitest run in packages/webui. A full local tsc --build was not run because this checkout has pre-existing dependency staleness unrelated to this change (an older installed simple-git lacking the unsafe.allowUnsafeHooksPath option used by packages/core); a clean npm ci build in CI is unaffected.

Risk & Scope

  • Main risk or tradeoff: The pre-first-prompt list now includes skills but still not custom .qwen/commands, MCP prompts or saved workflows — those require the session-scoped snapshot and continue to appear after the first prompt. This is a strict improvement over the current behavior, not a full restoration of every command type before first prompt.
  • Not validated / out of scope: No real-UI/Playwright capture; verified via unit tests. /workspace/skills fetch failures degrade gracefully (warn + no skills), matching the existing /workspace/providers handling.
  • Breaking changes / migration notes: None. mapWorkspaceSkills is additive; the deferred setConnection only sets commands/skills when non-empty and they are overwritten by the session snapshot after the first prompt.

Linked Issues

中文说明

这个 PR 做了什么

恢复 Web Shell 输入框在「首次发送消息之前」对技能类斜杠命令的自动补全。现在全新会话下输入 /rev 会立即补出 /review(以及其它所有技能),而不再是只有发过一条消息之后才行。

延迟连接路径现在会在拉取 /workspace/providers 的同时,一并拉取无需 session 的 /workspace/skills 状态,并据此填充 connection.commands / connection.skills。首次 prompt 创建 session 后,完整的、按 session 的 supported-commands 快照(额外包含自定义命令、MCP prompt、工作流命令)仍会替换这份列表,不会丢失任何东西。

为什么需要

自从 #6066 把 session 创建推迟到首次 prompt 之后,延迟连接路径虽然会报告 connected,但只拉取了 workspace providers,从未填充斜杠命令列表。于是在发消息之前,输入框退回到写死的本地命令列表(getLocalCommands),它只含约 32 个内置命令、不含任何技能。结果就是全新打开的 Web Shell 里 /rev 补不出 /review,且首次 prompt 之前任何技能都补不出来。补全匹配逻辑本身没问题("review".includes("rev")),只是命令列表里根本没有技能。

/workspace/skills 由 daemon 从 ConfigSkillManager 回答,无需活动 session(与延迟路径已经在调用的 /workspace/providers 属于同一类),并且包含 review 等 bundled 技能,因此是这些命令天然的、无 session 的数据来源。

复核测试计划

如何验证

  1. 启动 qwen serve --web,在全新会话打开 Web Shell(先不要发任何消息)。
  2. 在输入框输入 /rev
    • 修复前:不出现 /review 建议(只有写死的内置命令能补,如 /hel/help)。
    • 修复后:/review(及其它技能)出现在补全菜单里。
  3. 随便发一条 prompt,再输入 /rev —— 两个版本都能补出 /review(session 快照路径本来就正常;本 PR 只修首次 prompt 之前的状态)。

自动化覆盖:

  • packages/webui/src/daemon/session/mappers.test.ts —— 新增 mapWorkspaceSkills 单测(undefined → 空;技能 → 带描述/参数提示的技能斜杠命令)。
  • packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx —— 新增测试:延迟连接路径用 bundled review 技能填充 connection.commands/skills,且依然不创建 session。
$ npx vitest run   # 在 packages/webui 下
Test Files  17 passed (17)
     Tests  242 passed (242)

Lint/格式:四个改动文件的 eslintprettier --check 均通过。

证据(前后对比)

行为由新增的 provider 测试覆盖:延迟(无 session)连接现在产出 connection.commands = [{ name: 'review', … }]connection.skills = ['review'],而修复前两者都未设置。没有单独的 TUI/截图 —— 这是数据流层面的修复,用单测验证。

测试平台

系统 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

单测 + lint 在 macOS 运行。改动是 webui 数据层的平台无关 TypeScript;Windows/Linux 由 CI 覆盖。

运行环境(可选)

packages/webui 下运行 npx vitest run。本地没有跑完整的 tsc --build:该 checkout 存在与本改动无关的依赖陈旧问题(本地安装的 simple-git 版本偏旧,缺少 packages/core 用到的 unsafe.allowUnsafeHooksPath 选项);CI 里干净的 npm ci 构建不受影响。

风险与范围

  • 主要风险/取舍:首次 prompt 之前的列表现在包含技能,但仍不含自定义 .qwen/commands、MCP prompt、已保存工作流 —— 这些需要按 session 的快照,会在首次 prompt 之后出现。这是相对当前行为的严格改进,而非在首次 prompt 前完整恢复所有命令类型。
  • 未验证 / 范围外:没有真实 UI / Playwright 截图,靠单测验证。/workspace/skills 拉取失败时优雅降级(warn + 无技能),与既有 /workspace/providers 的处理一致。
  • 破坏性变更 / 迁移说明:无。mapWorkspaceSkills 是纯新增;延迟连接的 setConnection 仅在非空时设置 commands/skills,且首次 prompt 后会被 session 快照覆盖。

关联 Issue

无跟踪 issue;通过 Web Shell 输入框行为反馈发现,是 #6066 引入的回归。

wenshao and others added 2 commits July 2, 2026 10:37
… prompt

Since session creation is deferred until the first prompt (QwenLM#6066), the
deferred connect path reported 'connected' but only fetched workspace
providers — it never populated the slash-command list. Before sending a
message the composer therefore fell back to the hardcoded local command
list, which omits skills, so '/rev' would not autocomplete '/review'.

Fetch the session-less /workspace/skills status alongside providers in the
deferred connect path and seed connection.commands/skills from it, so
skill-backed slash commands autocomplete immediately. The full
session-scoped supported-commands snapshot (which also carries custom,
MCP-prompt and workflow commands) still replaces this once the first
prompt creates a session.
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required sections present.

On direction: this fixes a real, narrow regression from #6066 (deferred session creation). Before that change, skill slash commands like /review were available on connect; after it, they silently vanished until the first prompt. Clear user-facing impact, clear root cause. Well within scope.

On approach: the scope is tight — 4 files, +218/-7, all in the webui data layer. The fix reuses the existing session-less /workspace/skills endpoint (already called in the non-deferred connect path at workspace/actions.ts) and mirrors it into the deferred path. No scope creep, no drive-by refactors. This is exactly the minimal change the problem calls for.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

方向:修复 #6066(延迟 session 创建)引入的真实、窄范围回归。改动前技能斜杠命令(如 /review)在连接时就可用,改动后直到首次 prompt 才出现。用户影响明确,根因清晰,完全在合理范围内。

方案:范围紧凑——4 个文件,+218/-7,全在 webui 数据层。复用了已有的无 session /workspace/skills 端点(非延迟连接路径 workspace/actions.ts 已在调用),镜像到延迟路径中。无范围蔓延,无顺手重构。这正是该问题所需的最小改动。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Reviewed the diff against an independent proposal. Given the problem (skill commands missing in deferred connect), I'd have done the same thing: call workspaceSkills() alongside workspaceProviders(), map the result into DaemonCommandInfo[], and seed the connection state. The PR matches this exactly.

Reuse: client.workspaceSkills() is already used in the non-deferred path (workspace/actions.ts:147). The new mapWorkspaceSkills mapper follows the same shape and conventions as mapSupportedCommands — same return type, same satisfies DaemonAvailableCommand pattern, same conditional-spread style. No duplicated logic worth extracting.

Correctness: The Promise.allSettled pattern is right — skills failing must not block providers (and vice versa). The conditional spread (...(deferredSkillCommands.length > 0 ? { commands: ... } : {})) correctly avoids overwriting an existing commands array with an empty one. The session-scoped snapshot still replaces this on first prompt, so no stale data risk.

No blockers. No convention violations.

Tests & Typecheck

Test Files  2 passed (2)
     Tests  139 passed (139)
  Duration  4.86s
tsc --noEmit -p packages/webui/tsconfig.json
(clean — no output)
eslint (4 changed files)
(clean — no output)

CI: Ubuntu tests pass ✅, precheck pass ✅.

Real-Scenario Testing

N/A — this is a data-flow fix in the webui layer. The autocomplete behavior lives in the browser-based Web Shell composer, not in the CLI TUI. The behavior is fully covered by the new unit tests (deferred connect populates connection.commands with skill entries, skills failure doesn't block connect). No tmux test applicable.

中文说明

代码审查

基于独立提案审查 diff。给定问题(延迟连接路径缺少技能命令),我会做同样的事:在 workspaceProviders() 旁边调用 workspaceSkills(),将结果映射为 DaemonCommandInfo[],注入连接状态。PR 完全吻合这个方案。

复用: client.workspaceSkills() 已在非延迟路径(workspace/actions.ts:147)使用。新的 mapWorkspaceSkillsmapSupportedCommands 保持一致的返回类型、satisfies DaemonAvailableCommand 模式和条件展开风格。无需抽取的重复逻辑。

正确性: Promise.allSettled 模式正确——技能失败不应阻塞 providers(反之亦然)。条件展开正确避免了用空数组覆盖已有的 commands。首次 prompt 后 session 快照仍会替换,无过期数据风险。

无阻塞问题,无约定违反。

测试 & 类型检查

全部 139 个测试通过,tsc 和 eslint 均干净。CI Ubuntu 测试通过 ✅。

真实场景测试

不适用——这是 webui 数据层修复。自动补全行为在浏览器 Web Shell 中,非 CLI TUI。新单测已完全覆盖。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

This PR does exactly what it says — fixes a narrow regression where deferred session creation (#6066) left skill slash commands missing before the first prompt. The approach is the obvious one: reuse the session-less /workspace/skills endpoint that's already called in the non-deferred path.

Looking at the whole picture:

  • The independent proposal matches the PR's approach exactly. I wouldn't have done it differently.
  • Every change in the diff is needed for the stated goal. No drive-by refactors, no scope creep.
  • The code is straightforward — a new mapper function, a widened Promise.allSettled, and conditional state seeding. Nothing trying too hard.
  • Tests are thorough: mapper unit tests (undefined → empty, skills → commands), provider integration tests (deferred connect populates skills, skills failure doesn't block connect).
  • If I had to maintain this in six months, I'd thank the author — it's well-scoped and easy to understand.

Approving. ✅

中文说明

本 PR 完全实现了其目标——修复 #6066 延迟 session 创建导致首次 prompt 前技能斜杠命令缺失的窄范围回归。方案显而易见:复用非延迟路径已在调用的无 session /workspace/skills 端点。

整体评估:

  • 独立提案与 PR 方案完全一致。不会用不同方式做。
  • diff 中每个改动都是目标所需,无顺手重构、无范围蔓延。
  • 代码简洁直接——新 mapper 函数、拓宽的 Promise.allSettled、条件状态注入。没有过度设计。
  • 测试充分:mapper 单测、provider 集成测试。
  • 六个月后维护时会感谢作者——范围清晰、易于理解。

批准 ✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No review findings. Downgraded from Approve to Comment: self-PR; CI still running.

— qwen3.7-max via Qwen Code /review

Comment thread packages/webui/src/daemon/session/mappers.ts
Comment thread packages/webui/src/daemon/session/DaemonSessionProvider.tsx
Add a parallel test to the deferred-connect warn coverage: when
client.workspaceSkills() rejects, the connection still reports
'connected' (skills are non-blocking) and the failure is logged via
console.warn, mirroring the existing workspaceProviders-failure test.

Addresses review feedback on QwenLM#6153.

@ytahdn ytahdn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Clean regression fix — deferred connect now fetches /workspace/skills in parallel with /workspace/providers, seeding skill-backed slash commands for autocomplete before the first prompt. CI workflow --method GET fixes are legitimate bugfixes (prevented unintended POST on gh api calls with -F). Build passes, 139 tests pass. No blocking issues found.

— qwen3.7-max via Qwen Code /review

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Downgraded from Approve to Comment: CI still running.

— qwen3.7-max via Qwen Code /review

status: 'connected',
workspaceCwd: '/mock-workspace',
});
expect(connection).not.toHaveProperty('commands');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The "warns when deferred workspace skills fail" test asserts not.toHaveProperty('commands') but does not symmetrically assert not.toHaveProperty('skills'). Both properties derive from the same mapWorkspaceSkills(undefined) call — adding the missing assertion guards against a future regression that accidentally sets skills when the fetch fails.

Suggested change
expect(connection).not.toHaveProperty('commands');
expect(connection).not.toHaveProperty('commands');
expect(connection).not.toHaveProperty('skills');

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@ytahdn
ytahdn added this pull request to the merge queue Jul 2, 2026
@wenshao
wenshao removed this pull request from the merge queue due to the queue being cleared Jul 2, 2026
@wenshao
wenshao merged commit f3b3b99 into QwenLM:main Jul 2, 2026
70 checks passed
pull Bot pushed a commit to edisplay/qwen-code that referenced this pull request Jul 2, 2026
… unavailable (QwenLM#6169)

* fix(serve): keep skill slash commands available after the ACP child is reaped

`GET /workspace/skills` is answered exclusively by the ACP child — the
daemon has no local SkillManager. `requestWorkspaceStatus` only checks for
an already-live channel (`liveChannelInfo()`, never `ensureChannel()`), so
when no child is running it returns the idle placeholder
(`initialized: false`, empty `skills`).

That is the norm before the first session, and — crucially — again after
the child is reaped on session close, which happens immediately by default
(`--channel-idle-timeout-ms` defaults to 0 = immediate kill). Unlike
`/workspace/providers`, skills have no daemon-local status provider to fall
back on. So once a user has created and closed a session, every subsequent
pre-first-prompt `/workspace/skills` query returns empty, the Web Shell's
slash-command list falls back to the hardcoded built-ins (which omit
skills), and `/rev` stops autocompleting `/review`.

Retain the last skills status a live child produced and replay it while no
channel is live, so skill-backed slash commands keep autocompleting; the
next live query refreshes the cache. `initialized` cleanly separates a real
child answer (always `true`) from the idle placeholder (always `false`).

Completes QwenLM#6153, which wired the Web Shell to fetch `/workspace/skills` in
the deferred-connect path but could not surface skills the daemon was
unable to answer without a live child.

* fix(serve): enumerate workspace skills locally when the ACP child is unavailable

The cache from the previous commit keeps the last child answer alive across
a reap, but it never warms when the child never answers at all — most
visibly under `npm run dev`, where the on-demand-transpiled child's
`initialize` handshake routinely exceeds the 10s preheat budget, so preheat
times out and no channel ever comes up. `/workspace/skills` then stays empty
until the first prompt, dropping `/review` and every other skill from the
Web Shell's pre-first-prompt autocomplete even though the skills exist on
disk (typing `/review` in full still runs it, since submitting spawns a
session — hence "not in the list, but usable").

Add a daemon-local skills provider that enumerates skills straight from the
filesystem via SkillManager (a lightweight Config shim — no child, no MCP
init), mirroring the existing daemon-local providers-status provider. The
facade falls back to it only after both a live child answer and the cached
last answer are unavailable, so the live child stays authoritative (and
keeps extension-provided skills) while a never-preheated child still yields
the on-disk skills — `/review` included.

* fix(serve): fall back to cached/local skills when the child query throws mid-flight

Addresses review feedback on QwenLM#6169: the channel can die after
`liveChannelInfo()` returns a valid channel but before the RPC completes, so
`queryWorkspaceStatus` rejects. Previously that exception propagated even
though the cache or the daemon-local provider could still answer. Wrap the
query in try/catch (logging via writeStderrLine, matching
getWorkspaceEnvStatus / getWorkspacePreflightStatus) and treat a mid-flight
failure as "no live child", so the request degrades to the cached last answer
or daemon-local enumeration instead of failing.

* refactor(serve): address review feedback on daemon-local skills provider

- Extract the SkillConfig → ServeWorkspaceSkillStatus mapping into a shared
  workspace-skills-mapping module used by both the ACP child's
  buildWorkspaceSkillsStatus and the daemon-local provider, so the two skill
  listings can't drift; cover it (including the disable-model-invocation
  branch) with a unit test.
- Memoize the SkillManager per workspace so repeat queries reuse its in-memory
  cache instead of re-scanning every skill level on each call.
- Honor the safe-mode env (isSafeModeEnv, as Config does) instead of hardcoding
  isSafeMode to false; keep bareMode off (the daemon never runs `--bare`).
- Log daemon-local enumeration failures via writeStderrLine, matching the rest
  of the workspace-service error handling.

* test(serve): cover daemon-local skills error path; guard the facade provider call

- Test the previously-uncovered `buildWorkspaceSkillsStatus` catch branch:
  when enumeration fails it returns `{ initialized: false, skills: [],
  errors: [{ kind: 'skills', status: 'error', error }] }` and logs to stderr.
  Also cover the per-workspace SkillManager memoization.
- Wrap the facade's `workspaceSkillsStatusProvider` call in try/catch so a
  throwing injected provider degrades to the idle placeholder instead of
  failing the request (matching getWorkspaceEnvStatus / getWorkspacePreflightStatus),
  with a facade test for the throw path.
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request Jul 5, 2026
QwenLM#6319)

* fix(web-shell): keep skill slash commands after starting a new session

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.

* fix(web-shell): treat a fulfilled empty command snapshot as authoritative

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.

* test(web-shell): cover supported-commands fetch failure after a clear

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.
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.

3 participants