feat(daemon): support @extension mentions - #6008
Conversation
|
Thanks for the PR, @callmeYe! Template looks good ✓ On direction: this is a clean convergence play — #5849 brought On approach: the scope feels right. Extracting the shared extension mention helpers into One minor observation: Moving on to code review. 🔍 中文说明感谢贡献,@callmeYe! 模板完整 ✓ 方向:这是一个清晰的对齐改动 — #5849 给 CLI 带来了 方案:范围合理。将共享的 extension mention helper 抽到 一个小观察: 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Code ReviewThe refactoring is clean and well-motivated. Extracting shared extension mention helpers into The daemon ACP resolution in The WebShell No critical blockers or AGENTS.md violations found. Unit TestsAll targeted tests pass:
Tmux Smoke TestBefore (main branch)After (this PR)Both before and after handle the 中文说明代码审查重构整洁且动机明确。将共享的 extension mention helper 抽取到 daemon ACP 解析逻辑( WebShell 无关键阻塞问题或 AGENTS.md 违规。 单元测试所有目标测试通过:
Tmux 冒烟测试前后对比: — Qwen Code · qwen3.7-max |
|
This PR does exactly what it says — closes the gap between CLI and daemon WebShell extension mention behavior. The shared module extraction is the minimum viable DRY: one module, two consumers, no over-abstraction. The security guards (path traversal prevention, budget caps) are properly preserved through the refactor rather than reimplemented. The WebShell completion is well-designed with three distinct modes and graceful error fallback. Tests are comprehensive — 288 tests pass across the affected areas, including new tests for daemon context injection, deduplication, budget exhaustion, and path traversal protection. The before/after tmux smoke test confirms no CLI regression from the import restructuring. The new daemon features (ACP resolution, WebShell autocomplete) can't be exercised in a headless tmux session but are well-covered by unit tests. Clean PR — ships a focused consistency improvement without scope creep. 中文说明这个 PR 完全兑现了承诺 — 补齐了 CLI 和 daemon WebShell 之间 extension mention 行为的差距。共享模块抽取是最小可行的 DRY:一个模块、两个消费者、没有过度抽象。安全守卫(路径穿越防护、budget cap)在重构中被正确保留而非重新实现。WebShell 补全设计良好,有三种不同模式和优雅的错误降级。测试全面 — 受影响区域共 288 个测试通过,包括 daemon context 注入、去重、budget 耗尽和路径穿越防护的新测试。 前后 tmux 冒烟测试确认 import 重构没有引入 CLI 回归。新的 daemon 功能(ACP 解析、WebShell 自动补全)无法在无头 tmux 中交互测试,但已被单元测试充分覆盖。 干净的 PR — 交付了一个聚焦的一致性改进,没有范围蔓延。 — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
✅ Maintainer local verification — real build + PR test plan + real-daemon
|
| Suite | Result |
|---|---|
web-shell client/completions/atCompletion.test.ts |
4 passed |
cli acp-integration/session/Session.test.ts |
203 passed |
cli extension-mention-ref.test.ts + atCommandProcessor.test.ts |
81 passed |
GitHub CI Test (ubuntu-latest, Node 22.x) is green (that leg runs lint/prettier/typecheck + tests); BLOCKED is review-gating, not a failing check.
2. Real-daemon @ext injection E2E (the differentiated evidence)
Installed a real active extension (broadcast-helper: a description, an MCP-server capability, and a CONTEXT.md carrying a unique nonce) under QWEN_HOME/extensions, booted a real qwen serve daemon pointed at a fake OpenAI endpoint that captures the request body, and drove the /acp Streamable-HTTP flow (initialize → session/new → session/prompt). Each test used a fresh session so the captured model request carried no prior-turn history.
The mention-injection signal is the --- Extension: … (untrusted third-party content) --- marker block (only the mention path emits it). An active extension's context file is also loaded into every turn anyway (baseline), so the nonce count goes 1 → 2 when the mention additionally reads the file.
| Test (fresh session) | Model request (captured) | Result |
|---|---|---|
@ext:broadcast-helper |
marker block 1, capability line - MCP Servers: broadcaster, description nonce, context-file nonce 2 (baseline + mention read), visible @ext:broadcast-helper preserved |
✅ injected, prompt text untouched |
@ext:broadcast-helper ×2 + @ext:BROADCAST-HELPER |
marker block 1 | ✅ deduped, case-insensitive |
@ext:nonexistent-xyz |
marker 0, context-file nonce 1 (always-on baseline only), visible text preserved | ✅ unknown → plain text, no injection, no failure |
GET /workspace/extensions also returns the new optional description field (additive — the acp-bridge/status + sdk-types change), confirming the autocomplete-detail path end-to-end.
3. Mutation testing — the tests genuinely guard the behavior
Reverted each guard individually and confirmed the matching Session.test.ts case flips to red:
| Reverted guard | Test that failed |
|---|---|
matchExtensionByRef → always undefined |
injects active extension context for @ext mentions |
both dedup layers (per-name Map + resolvedExtensionNames) |
dedupes repeated extension mentions … (--- Extension: Browser length 2, expected 1) |
isSubpath context-file sandbox (security) |
caps … and skips files outside the extension (the out-of-extension secret got injected) |
4. Code review notes
- Shared helper
extension-mention.tsis security-conscious.sanitizeDisplayTextstrips terminal control + BiDi/zero-width chars from extension names/descriptions;buildExtensionMentionContextreads context files only afterfs.realpath+isSubpath(realExtPath, realPath)(symlink-escape-safe path-traversal guard), skips unreadable/empty files, and bounds output to 200 KB total / 50 KB per file with… (truncated). - Daemon resolution preserves the visible prompt.
Session.tscollects@ext:refs from text parts (deduped by lowercase name), resolves them againstgetActiveExtensions(), and appends the extension context as separateParts — the user's original text parts (with@ext:<name>) are untouched. Unknown/inactive names are debug-logged and skipped. - Web-shell completion (
atCompletion.ts):@ext:enters extension-only mode (no file glob), filters active extensions by name/displayName, sorts prefix-matches first, caps at 50, inserts the canonical@ext:<name>. Covered by the 4 unit tests + the live extension-status endpoint that feeds it.
Notes / observations (non-blocking)
- For an active extension, its context file is loaded into every turn regardless of the mention; the
@ext:mention adds the capability/marker block and re-includes the file. So seeing extension context without a mention is the normal always-on behavior, not the mention path. - Web-shell
@completion was verified via its unit tests + the daemon extension-status endpoint (which I confirmed live), not a browser/Playwright UI run — its completion source is pure, fully-unit-tested logic.
🇨🇳 中文版本(完整对应)
✅ 维护者本地验证 —— 真实构建 + PR 测试计划 + 真实 daemon @ext 注入 E2E + 变异测试
在干净的 npm ci worktree(落后 main 1 个 commit,mergeable,按现状测试)上验证 PR head 720630334。除复现 PR 测试计划外,我还用一个真实 active extension + fake 模型端点驱动了真实 qwen serve daemon,抓取模型实际收到的请求,端到端证明 @ext:<name> 解析;再对注入、去重、路径穿越守卫做变异测试。结论:功能在真实 daemon 上有效且有防护(含一个与安全相关的 context-file 沙箱),建议合并。
环境: macOS(darwin-arm64),Node v22.22.2。
1. PR 测试计划(真实构建)—— 全绿
| 套件 | 结果 |
|---|---|
web-shell client/completions/atCompletion.test.ts |
4 通过 |
cli acp-integration/session/Session.test.ts |
203 通过 |
cli extension-mention-ref.test.ts + atCommandProcessor.test.ts |
81 通过 |
GitHub CI Test (ubuntu-latest, Node 22.x) 绿(该 leg 跑 lint/prettier/typecheck + 测试);BLOCKED 是 review 门控而非失败的 check。
2. 真实 daemon @ext 注入 E2E(差异化证据)
在 QWEN_HOME/extensions 下安装一个真实 active extension(broadcast-helper:带 description、一个 MCP-server capability、一个带唯一 nonce 的 CONTEXT.md),启动真实 qwen serve daemon 指向一个抓请求体的 fake OpenAI 端点,走完整 /acp Streamable-HTTP 流程(initialize → session/new → session/prompt)。每个测试用全新 session,使抓到的模型请求不含上一轮历史。
mention 注入的信号是 --- Extension: … (untrusted third-party content) --- marker 块(只有 mention 路径会发它)。active extension 的 context 文件本来就常驻注入每一轮(baseline),所以 mention 额外读取该文件时 nonce 计数从 1 → 2。
| 测试(全新 session) | 抓到的模型请求 | 结果 |
|---|---|---|
@ext:broadcast-helper |
marker 块 1、capability 行 - MCP Servers: broadcaster、description nonce、context 文件 nonce 2(baseline + mention 读取)、可见 @ext:broadcast-helper 保留 |
✅ 已注入,prompt 文本不变 |
@ext:broadcast-helper ×2 + @ext:BROADCAST-HELPER |
marker 块 1 | ✅ 已去重,大小写不敏感 |
@ext:nonexistent-xyz |
marker 0、context 文件 nonce 1(仅常驻 baseline)、可见文本保留 | ✅ 未知 → 普通文本,不注入、不报错 |
GET /workspace/extensions 也返回新的可选 description 字段(增量——acp-bridge/status + sdk-types 改动),端到端确认了自动补全详情路径。
3. 变异测试 —— 测试确实守得住行为
逐一回退每个守卫,确认对应 Session.test.ts 用例翻红:
| 回退的守卫 | 失败的测试 |
|---|---|
matchExtensionByRef → 恒 undefined |
injects active extension context for @ext mentions |
两层去重(按名 Map + resolvedExtensionNames) |
dedupes repeated extension mentions …(--- Extension: Browser 长度 2,应为 1) |
isSubpath context 文件沙箱(安全) |
caps … and skips files outside the extension(extension 目录外的 secret 被注入) |
4. 代码审查记录
- 共享 helper
extension-mention.ts重安全。sanitizeDisplayText从 extension 名称/描述里剥离终端控制 + BiDi/零宽字符;buildExtensionMentionContext仅在fs.realpath+isSubpath(realExtPath, realPath)(防 symlink 逃逸的路径穿越守卫)后才读 context 文件,跳过不可读/空文件,并把输出限制在总量 200 KB / 单文件 50 KB(… (truncated))。 - daemon 解析保留可见 prompt。
Session.ts从 text part 收集@ext:引用(按小写名去重),对getActiveExtensions()解析,并把 extension context 作为独立Part追加——用户原始 text part(含@ext:<name>)原样不动。未知/未激活名称 debug 记录并跳过。 - web-shell 补全(
atCompletion.ts):@ext:进入 extension-only 模式(无 file glob),按 name/displayName 过滤 active extension,前缀匹配优先排序,上限 50,插入规范的@ext:<name>。由 4 个单测 + 喂给它的实时 extension-status 端点覆盖。
备注 / 观察(不阻塞合并)
- 对于 active extension,无论是否 mention,其 context 文件都会注入每一轮;
@ext:mention 额外加上 capability/marker 块并再次纳入该文件。所以无 mention 也能看到 extension context,是正常的常驻行为,并非 mention 路径。 - web-shell
@补全通过其单测 + daemon extension-status 端点(我已实时确认)验证,未跑浏览器/Playwright UI——其补全 source 是纯逻辑、已被单测完整覆盖。
|
|
||
| // Add initial query text first | ||
| processedQueryParts.push({ text: initialQueryText }); | ||
| processedQueryParts.push(...extensionParts); |
There was a problem hiding this comment.
[Suggestion] The combined code path where a prompt contains both @file references (producing fileData parts) and @ext: mentions is untested. All three new Session tests use text-only prompts with @ext: but no file attachments. This processedQueryParts.push(...extensionParts) line (and the one at line 5136 in the else branch) are only reached when atPathCommandParts.length > 0 or embeddedContext.length > 0 — that fallthrough is never exercised.
A bug in the combined path (e.g., extension parts inserted in wrong position, or duplicated) would go undetected. Consider adding a test that sends a prompt with both a resource_link block (producing a fileData part) and a text block containing @ext:browser, then asserting both the file content and the extension context block appear in the sent message.
— qwen3.7-max via Qwen Code /review
| let realExtPath: string; | ||
| try { | ||
| realPath = await fs.realpath(contextFilePath); | ||
| realExtPath = await fs.realpath(extension.path); |
There was a problem hiding this comment.
[Suggestion] fs.realpath(extension.path) is computed inside the .map() loop over contextFiles, so the same syscall is issued once per context file even though the value never changes.
| realExtPath = await fs.realpath(extension.path); | |
| realPath = await fs.realpath(contextFilePath); |
Hoist this call above the .map():
let realExtPath: string;
try {
realExtPath = await fs.realpath(extension.path);
} catch {
return { text: contextText, remainingBudget };
}
const fileReads = await Promise.allSettled(
extension.contextFiles.map(async (contextFilePath) => {
// ... use realExtPath directly
}),
);— qwen3.7-max via Qwen Code /review
| ); | ||
| return null; | ||
| } | ||
| return fs.readFile(realPath, { |
There was a problem hiding this comment.
[Suggestion] fs.readFile(realPath, { encoding: 'utf-8' }) reads any file without a content-type check. If an extension accidentally lists a binary file (image, .wasm, compiled asset) as a context file, Node reads it into a string with U+FFFD substitutions and the garbled result is injected into the model prompt, consuming budget.
Consider a lightweight binary-content check — e.g. reject files containing a null byte in the first few KB:
const content = await fs.readFile(realPath, { encoding: 'utf-8', signal: options.signal });
if (content.includes('\0')) {
options.onDebugMessage?.(`Skipping binary context file: ${contextFilePath}`);
return null;
}— qwen3.7-max via Qwen Code /review
| const lowerQuery = query.toLowerCase(); | ||
| return status.extensions | ||
| .filter((ext) => ext.isActive) | ||
| .map((ext) => ({ |
There was a problem hiding this comment.
[Suggestion] displayName and description go through sanitizeDisplayText, but ext.name passes through raw into both the completion label (rendered in the CodeMirror dropdown as @ext:${ext.name}) and the apply text inserted into the editor. An extension whose name contains ANSI escapes or Unicode bidi overrides would render/inject them unsanitized — inconsistent with the CLI path.
The apply value must stay canonical for daemon @ext:<name> resolution, so simply sanitizing it isn't an option. Instead, filter out extensions whose name fails sanitization (they can't be safely displayed anyway):
.filter((ext) => sanitizeDisplayText(ext.name) != null && ext.isActive)
.map((ext) => ({
...ext,
displayName: sanitizeDisplayText(ext.displayName ?? '') ?? undefined,
description: sanitizeDisplayText(ext.description ?? '') ?? undefined,
name: sanitizeDisplayText(ext.name) ?? ext.name,
}))— qwen3.7-max via Qwen Code /review
| ); | ||
| } | ||
|
|
||
| const AT_TOKEN_RE = /@([^\s,;!?()[\]{}]+)/g; |
There was a problem hiding this comment.
[Suggestion] AT_TOKEN_RE = /@([^\s,;!?()[\]{}]+)/g does not exclude a trailing period, so @ext:browser. at the end of a sentence is captured as ext:browser. (including the dot), which parseExtensionRef returns as {name: 'browser.'} — no extension matches and the mention silently fails to resolve.
The CLI path (atCommandProcessor.ts) explicitly terminates @path tokens when . is followed by whitespace or end-of-string, so daemon users get different behavior than CLI users for sentence-ending mentions. Tightening the regex to exclude trailing ., : and similar punctuation would make the two paths consistent.
— qwen3.7-max via Qwen Code /review
| } | ||
|
|
||
| return { text: contextText, remainingBudget }; | ||
| } |
There was a problem hiding this comment.
[Suggestion] The new 179-line shared module extension-mention.ts has no dedicated unit test file. buildExtensionMentionContext (the core async function — file I/O, symlink resolution, path-traversal guard, per-file cap, aggregate budget, abort signal) is only exercised indirectly through Session.test.ts. A regression in budget accounting, abort propagation, or the remainingBudget return value would only surface through the integration tests.
Consider adding packages/cli/src/utils/extension-mention.test.ts covering: no context files, file within budget, file over per-file cap, aggregate budget exhaustion across multiple files, unreadable file (debug callback invoked), file outside extension path via symlink, abort signal propagation, and the getExtensionDisplayName / sanitizeDisplayText exports.
— qwen3.7-max via Qwen Code /review
What this PR does
This PR brings daemon WebShell extension mentions in line with the existing CLI behavior. Active extensions now appear in WebShell
@autocomplete, selecting one inserts a canonical@ext:<name>reference, and daemon ACP prompt handling resolves those references into additional extension context for the model while keeping the user's visible prompt text unchanged.The change also includes a shared extension mention helper so CLI and daemon prompt paths use the same parsing, sanitization, capability formatting, and bounded context-file handling. Daemon extension status now carries an optional description field for better autocomplete detail without breaking older clients.
Why it's needed
Before this change, daemon WebShell users could type or insert extension-looking text, but the daemon prompt path did not treat
@ext:<name>as an actual extension mention. That meant extension capabilities and allowed context files were not injected into the model turn. This makes WebShell daemon sessions behave consistently with CLI extension mentions and avoids a surprising gap between selecting a mention and the model actually receiving extension context.Reviewer Test Plan
How to verify
In WebShell, type
@and confirm active extensions appear before workspace file suggestions, type a partial name such as@broand confirm extension and file matches are filtered, then type@ext:and confirm only extension suggestions remain. Selecting an extension should insert@ext:<extension.name>.In a daemon ACP session with an active extension, send a prompt containing
@ext:<name>and confirm the visible prompt text is preserved while the model request also receives the selected extension's capabilities and allowed context-file content. Repeating the same mention should inject it only once, and unknown or inactive extension names should remain plain text without failing the prompt.Local commands run:
cd packages/web-shell && npx vitest run client/completions/atCompletion.test.ts— 1 file passed, 4 tests passed.cd packages/cli && npx vitest run src/acp-integration/session/Session.test.ts— 1 file passed, 203 tests passed.cd packages/cli && npx vitest run src/ui/hooks/extension-mention-ref.test.ts src/ui/hooks/atCommandProcessor.test.ts— 2 files passed, 81 tests passed.npm run build && npm run typecheck— passed. Build output still reports existing vscode companion curly-rule warnings, but no errors, and typecheck completed successfully.Evidence (Before & After)
Before: WebShell daemon
@completion did not expose active extension mention options as real daemon-resolved extension references, and daemon prompt resolution did not inject extension context for@ext:<name>.After: WebShell completion includes active extension suggestions,
@ext:is extension-only, selection inserts a canonical daemon-resolvable reference, and daemon ACP prompt resolution appends sanitized extension context while preserving the user's original prompt text.Tested on
Environment (optional)
Node.js workspace install in a fresh worktree from
origin/main; targeted Vitest commands plus repository build and typecheck.Risk & Scope
Linked Issues
Related: #5849
中文说明
What this PR does
这个 PR 让 daemon WebShell 的 extension mention 行为对齐现有 CLI 行为。当前激活的 extension 会出现在 WebShell
@自动补全中,选择后插入规范的@ext:<name>引用,daemon ACP prompt 处理会把这些引用解析为额外的 extension context 注入给模型,同时保持用户可见的 prompt 文本不变。本次改动还抽取了共享的 extension mention helper,让 CLI 和 daemon prompt 路径共用相同的解析、清洗、能力格式化和有边界的 context file 处理。daemon extension status 现在会携带可选的 description 字段,用于更好的自动补全详情展示,并且不破坏旧客户端。
Why it's needed
在这个改动之前,daemon WebShell 用户可以输入或插入看起来像 extension 的文本,但 daemon prompt 路径不会把
@ext:<name>当成真正的 extension mention。结果是 extension capabilities 和允许的 context files 不会注入模型请求。这会让选择 mention 和模型实际收到 extension context 之间出现不一致。本 PR 让 WebShell daemon session 与 CLI extension mention 行为一致。Reviewer Test Plan
How to verify
在 WebShell 中输入
@,确认激活的 extensions 显示在 workspace file suggestions 前面;输入类似@bro的部分名称,确认 extension 和 file 都按输入过滤;再输入@ext:,确认只显示 extension suggestions。选择 extension 后应插入@ext:<extension.name>。在启用了某个 active extension 的 daemon ACP session 中,发送包含
@ext:<name>的 prompt,确认用户可见文本保持不变,同时模型请求额外收到该 extension 的 capabilities 和允许的 context-file 内容。重复 mention 同一个 extension 时只注入一次,未知或未激活 extension 名称保持普通文本且不会导致 prompt 失败。本地执行的命令:
cd packages/web-shell && npx vitest run client/completions/atCompletion.test.ts— 1 个文件通过,4 个测试通过。cd packages/cli && npx vitest run src/acp-integration/session/Session.test.ts— 1 个文件通过,203 个测试通过。cd packages/cli && npx vitest run src/ui/hooks/extension-mention-ref.test.ts src/ui/hooks/atCommandProcessor.test.ts— 2 个文件通过,81 个测试通过。npm run build && npm run typecheck— 通过。build 输出中仍有既有 vscode companion curly-rule warnings,但没有 error,typecheck 成功完成。Evidence (Before & After)
Before:WebShell daemon 的
@completion 不会把 active extension options 作为 daemon 可解析的真实 extension reference 暴露出来,daemon prompt resolution 也不会为@ext:<name>注入 extension context。After:WebShell completion 包含 active extension suggestions,
@ext:进入 extension-only 模式,选择后插入规范的 daemon 可解析 reference,daemon ACP prompt resolution 会追加清洗后的 extension context,同时保留用户原始 prompt 文本。Tested on
Environment (optional)
基于
origin/main的新 worktree 中安装 Node.js workspace 依赖后执行 targeted Vitest、仓库 build 和 typecheck。Risk & Scope
Linked Issues
Related: #5849