feat(extensions): support Agent Plugins v1 - #8834
Conversation
E2E Test ReportTested the bundled CLI on macOS with fresh isolated
|
|
Gate re-run at @wenshao's request — the head is unchanged from the previous pass ( Template: complete ✓ Problem: an observed gap, not a theoretical one: an unmodified Agent Plugins v1 package fails both Direction: aligned. Native loading (no conversion, no file rewriting) is the right call for a standards-based format, and the capability boundary (direct-child Skills + stdio/Streamable HTTP MCP, nothing else) matches Codex's portable core. No auth/sandbox/public-contract escalation triggers. Size: core paths are touched ( Approach: scope remains disciplined. Everything outside the two fix commits was assessed as load-bearing in the first pass, and the fix delta maps 1:1 to the Critical findings from the Risk: Moving on to code review. 🔍 中文说明应 @wenshao 请求再次执行 Gate —— head 与上一轮相同（ **模板：**完整 ✓ **问题：**已观测到的缺口而非理论问题：未改动的 Agent Plugins v1 包目前在 **方向：**对齐。对标准化格式而言原生加载（不转换、不改写文件）是正确选择；能力边界（直接子目录 Skills + stdio/Streamable HTTP MCP，其余不支持）与 Codex 的 portable core 一致。未触及 auth/sandbox/公共契约等升级项。 规模：触及核心路径（ **方案：**范围依旧克制。两个 fix commit 之外的部分在首轮已评估为必要组成，fix delta 与两轮 **风险：** 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review (final pass at
|
| Finding | Fix verified at this head |
|---|---|
R1-2 + R2-1 — bundled commands/ becoming live, unconsented slash commands |
FileCommandLoader excludes extensions keyed on both format === 'agent-plugins-v1' and originSource === 'AgentPlugins'; regression test pins zero commands loaded |
| R1-3 — symlinked source root silently copying nothing | copyExtension realpaths the source when skipSymlinks is set; symlinks inside the package are dropped via lstat filter |
R1-4 — startup hang on a non-regular root plugin.json |
getAgentPluginSchemaStatus requires statSync(...).isFile() before reading |
R1-5 — cwd: "${PLUGIN_DATA}/<subdir>" accepted but never startable |
loader mkdirs contained stdio cwd targets at load time; launch-time validateAgentPluginStdioRuntimePaths re-checks containment |
R1-6 + R2-3 — root plugin.json pre-empting an explicit marketplace selection |
with a pluginName selection, root detection is bypassed and any carried plugin.json is removed from the converted directory |
R2-2 — unvalidated link-metadata source consumed by fingerprint/load paths |
both call sites require a non-empty string source before honoring type: 'link' |
Follow-ups the author committed to (from the maintainer E2E, explicitly non-blocking): document or warn when a package carries both plugin.json and qwen-extension.json (Agent Plugins precedence is silent today), and guard the empty MCP servers: section in extensions list output.
Flow — the install path, for orientation (unchanged):
sequenceDiagram
participant P1 as User
participant P2 as Install flow
participant P3 as Format detection
participant P4 as Plugin loaders
participant P5 as Consent prompt
participant P6 as Extension store
P1->>P2: install or link a package
P2->>P3: probe root manifest schema
P3-->>P2: AgentPlugins detected
P2->>P4: load plugin.json, skills, mcp.json
P4-->>P2: contained config and MCP entries
P2->>P5: show servers and skills
P5-->>P1: approve or abort
P1->>P2: approve
P2->>P6: copy without symlinks, persist PLUGIN_DATA by id
Files changed (30 of 33 shown)
| File | What changed |
|---|---|
docs/design/agent-plugins-v1-native-support.md |
Design doc - native format rationale, capability boundary, security policies |
docs/users/extension/_meta.ts |
Adds the agent-plugins page to docs navigation |
docs/users/extension/agent-plugins.md |
User docs - install and link commands, support matrix, PLUGIN_ROOT and PLUGIN_DATA |
docs/users/extension/extension-releasing.md |
Releasing docs now accept plugin.json as a root manifest |
docs/users/extension/introduction.md |
Ecosystem intro and storage layout mention Agent Plugins |
packages/acp-bridge/src/status.ts |
Serve origin union gains AgentPlugins |
packages/cli/src/commands/extensions/consent.ts |
Agent Plugins skip the third-party compatibility warning but keep the safety text |
packages/cli/src/commands/extensions/utils.ts |
Adds the Origin row label to extension details |
packages/cli/src/services/FileCommandLoader.ts |
Excludes Agent Plugins from bundled slash-command loading, keyed on format and originSource |
packages/cli/src/ui/components/extensions/views/PluginDetailView.tsx |
Renders the Origin row when the sidecar records an originSource |
packages/core/src/config/config.ts |
ExtensionOriginSource union gains AgentPlugins |
packages/core/src/extension/agent-plugins-v1/index.ts |
Module barrel for the new format |
packages/core/src/extension/agent-plugins-v1/manifest.ts |
plugin.json schema gating plus manifest validation, with a regular-file guard |
packages/core/src/extension/agent-plugins-v1/mcp.ts |
mcp.json loader - stdio and Streamable HTTP only, single-pass variable expansion, URL and header safety, PLUGIN_DATA cwd creation |
packages/core/src/extension/agent-plugins-v1/paths.ts |
Path containment helpers - realpath resolution and lexical within checks |
packages/core/src/extension/agent-plugins-v1/skills.ts |
Direct-child SKILL.md loader - parsed and validated, grants nothing |
packages/core/src/extension/extension-converter.ts |
Detection hook - marketplace selection wins over root plugin.json; carried manifests dropped from converted dirs |
packages/core/src/extension/extension-store.ts |
Stable per-plugin PLUGIN_DATA directory keyed by a validated sha256 id |
packages/core/src/extension/extensionManager.ts |
Install, link, update, reinstall wiring - format on Extension, link-metadata validation, realpath copy for symlinked sources |
packages/core/src/extension/github.ts |
Records originSource through git-based installs |
packages/core/src/tools/mcp-client.ts |
Credential-safe redirect stop for agentPluginV1 HTTP servers and launch-time stdio containment validation |
packages/sdk-typescript/src/daemon/types.ts |
SDK origin union gains AgentPlugins |
packages/cli/src/commands/extensions/utils.test.ts |
Tests the Origin row |
packages/cli/src/services/FileCommandLoader.test.ts |
Pins that Agent Plugin bundled commands load nothing |
packages/core/src/extension/agent-plugins-v1/manifest.test.ts |
Schema status gating and the non-regular manifest guard |
packages/core/src/extension/agent-plugins-v1/mcp.test.ts |
Server loading, expansion, isolation, URL and header safety, cwd creation |
packages/core/src/extension/agent-plugins-v1/skills.test.ts |
Skill parsing, containment, BOM and CRLF normalization |
packages/core/src/extension/extension-converter.test.ts |
Marketplace selection precedence over root plugin.json, including future schemas |
packages/core/src/extension/extensionManager.test.ts |
Install, link, update, reinstall flows plus symlinked source and broken-metadata isolation |
packages/core/src/tools/mcp-client.test.ts |
Redirect stop and launch-time validation paths |
| …and 3 more files | consent.test.ts, extension-store.test.ts, github.test.ts |
Test evidence
Unattended CI run — I do not build or execute PR code; the evidence below is the PR's own CI on the reviewed commit (fetched via API), plus the two isolated lanes that ran afterwards. All pull_request-event workflow runs on this head completed green (Qwen Code CI, Qwen Live Host CI, SDK Java). The macOS/Windows Node suites and the CLI integration suite remain gated-skipped for this commit, so CI itself settles the Linux unit surface only.
| Check | Conclusion |
|---|---|
Test (ubuntu-latest, Node 22.x) |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
Live Host (macos-latest) |
✅ success |
Real daemon E2E / Java 11 |
✅ success |
SDK Java (ubuntu 11/17/21, macos 21, windows 21) |
✅ success |
Classify PR |
✅ success |
Post Coverage Comment (ubuntu-latest, 22.x) |
✅ success |
Skipped (gated) on this commit: Test (macos-latest / windows-latest, Node 22.x), Integration Tests (CLI, No Sandbox), build-cli. / 本 commit 上被门控跳过：macOS/Windows Node 测试套件、CLI 集成测试、build-cli。
What closes the behavioural gap CI cannot:
- Sandboxed verification (
@qwen-code /verify) — ✅ passed, merge-ready verdict on this head (report earlier in this thread): the load-bearing A/B held — the base build rejects an unchangedplugin.jsonpackage, this head installs and links it withOrigin: AgentPlugins. - Maintainer-run local E2E (@wenshao, macOS) — not an author claim: the maintainer built base and head from source and reproduced the whole Reviewer Test Plan against real stdio and Streamable HTTP servers and a live daemon, with server-side header logging. Notably: configured
Authorization/Hostnever reached the server, cross-origin redirects stopped dead, symlink escapes were dropped at install,PLUGIN_DATAstayed stable across update/uninstall/reinstall, and a mutation A/B proved themcp-client.tsredirect guard is load-bearing. Verdict: LGTM, followed by a formal approving review of this exact head. /tmuxreal-user run — timed out before completing. The TUI surfaces it would have exercised (consent prompt wording, extensions-list Origin row) were instead driven in the maintainer E2E above, so this is noted, not left open.
Not verified anywhere: Windows/Linux end-to-end runtime behavior (unit tests only on those platforms), and git/npm/release install sources for Agent Plugins (local path, link, and archive were exercised). Both are acknowledged in the PR's Risk & Scope.
中文说明
代码审查（在 0f8a8ffa 上的最终一轮）
本轮独立重读了安全关键代码段，而非沿用早前轮次的结论：MCP loader 的 URL/header 策略、包容性辅助函数、mcp-client.ts 中限定作用域的重定向拦截、converter 的格式检测优先级、manager 的 link 元数据校验。没有新发现——姿态始终是 fail-closed：HTTP MCP 仅允许绝对 HTTP(S)（非 loopback 明文 HTTP、user-info、fragment 被拒绝），client-owned header 被丢弃；stdio 的 command/cwd 被限制在插件根或 PLUGIN_DATA 内且展开后复查包容性，变量展开为单趟不递归；所有边界检查先 realpath 再比较，安装时丢弃符号链接，启动时二次校验；redirect: 'manual' 仅作用于带凭据的 agentPluginV1 server，其余 MCP 配置走原路径；schema 严格门禁、marketplace 显式选择优先并清理携带的 plugin.json、link 元数据仅在 source 为非空字符串时生效（两处调用点）、PLUGIN_DATA id 校验为 64 位十六进制。两个 fix commit 的六项修复本轮逐条在当前 head 代码中重新核实（见上表）。作者承诺的后续跟进项（双 manifest 静默优先级、extensions list 空小节）均不阻塞。
测试证据
无人值守 CI 运行——不构建或执行 PR 代码。该 head 上所有 pull_request 事件工作流全绿（Qwen Code CI、Qwen Live Host CI、SDK Java）；macOS/Windows Node 套件与 CLI 集成套件仍被门控跳过，CI 本身仅覆盖 Linux 单元面。CI 表格见上方标记区域。
补足 CI 无法覆盖的行为证据：沙箱验证 /verify ✅ 通过、结论 merge-ready（关键 A/B 成立：base 拒绝、head 接受未改动的 plugin.json 包）；维护者 @wenshao 的本地 E2E（维护者亲自执行，非作者声明）：base/head 均从源码构建，对真实 stdio 与 Streamable HTTP server、真实 daemon 复现了全部 Reviewer Test Plan，服务端 header 记录证实凭据不外泄、跨源重定向被拦截、符号链接安装时被丢弃、PLUGIN_DATA 全生命周期稳定，并对重定向防护做了变异 A/B 证明其真实有效，结论 LGTM 并对当前 head 正式批准；/tmux 运行超时，但其目标界面（consent 文案、Origin 行）已由维护者 E2E 实际演练。仍未被任何证据覆盖：Windows/Linux 端到端运行时行为（仅单元测试）、git/npm/release 安装来源——两者均已在 PR 的 Risk & Scope 中声明。
— Qwen Code · qwen3.8-max
Reviewed at 0f8a8ffa46f66408b741f1be42fa75181fa9f717 · re-run with @qwen-code /triage
|
Confidence: 4/5 — clean review at this head, and both independent lanes landed green (sandboxed The previous pass deferred with four asks for @wenshao. All four are now answered, so this pass acts instead of deferring again:
Stepping back: this is what review iteration should look like. Two adversarial Follow-ups for @callmeYe (keep them out of this PR, as agreed): file issues for the silent dual-manifest precedence (doc line or ✅ Approving, pinned to the reviewed commit. Note for the thread: this approval is the bot's latest review, so it supersedes the two stale 中文说明置信度：4/5 —— 当前 head 审查干净，两条独立证据通道均已落地且为绿（沙箱 上一轮带着四个问题转交 @wenshao，现已全部得到回应，因此本轮直接裁决而不再 defer：1）核心规模策略签核（核心路径约 1,166 生产行）——以最强方式回应：维护者亲自端到端验证后对当前 head 正式批准；该升级项的意义在于确保有人类对这一规模的核心改动负责，现在已经有了。2）最终卸载后的 整体评价：这是评审迭代应有的样子。两轮对抗性 给 @callmeYe 的后续跟进项（按约定不要塞进本 PR）：为双 manifest 静默优先级（补文档或 ✅ 批准，锚定在被审查的 commit 上。线程说明：本批准是机器人最新的评审，因此取代此前锚定在较早 commit 上的两个过期 — Qwen Code · qwen3.8-max Reviewed at |
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. |
|
@qwen-code /verify |
|
@qwen-code /tmux |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 569 passed · 0 failed · 569 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：569 通过 · 0 失败 · 569 总计 Verification reportPR 8834 Deep Verification —
|
| cell | oracle | base (57e6c310) |
head (79003284) |
|---|---|---|---|
| control: qwen-extension installs (both arms) | install succeeds, name ctrl-ext |
✅ PASS | ✅ PASS |
| install-local agent plugin | install result | ❌ Configuration file not found .../qwen-extension.json (expected red = assertion PASS) |
✅ installed |
| link agent plugin | link result | ❌ same error (expected red = PASS) | ✅ linked, path === source |
| origin / version / skills | metadata + discovery | — | ✅ AgentPlugins, 1.0.0 default, [greet] |
| non-portable capabilities inactive | commands/agents/context/hooks/settings/channels | — | ✅ all empty/undefined |
| MCP gating | loaded server keys | — | ✅ [local, remote] (sse skipped) |
PLUGIN_ROOT/PLUGIN_DATA/args expansion |
env values vs realpath | — | ✅ expanded, data dir outside package |
byte-identical package, no qwen-extension.json, escaping symlink dropped |
file comparison | — | ✅ 4/4 files identical |
| consent payload | originSource, skill list |
— | ✅ AgentPlugins |
update + reinstall keep PLUGIN_DATA |
same path, state.txt survives |
— | ✅ both |
| link manifest fingerprint | refreshCacheIfSourcesChanged + reload |
— | ✅ edit → detected → version 9.9.9 reloaded |
Counts: head 21/21, base 4/4 (the two base reds are expected-failure
assertions and count as passes). 25/25 A/B assertions.
Secondary claims verified:
- Security surface (
security-matrix.mjs, 68/68 —03-security-matrix.png):
URL gate across 17 accept/reject shapes includinghttp://2130706433/,
http://127.1/, trailing-dot, ideographic-dot。, Arabic-Indic digits,
localhost.evil.com,[::ffff:127.0.0.1], userinfo/fragment/file/ftp;
client-owned header drop, duplicate/control-char/invalid-name headers;
stdio containment (./bin/../../evil, absolute commands,${PLUGIN_*}and
absolute cwd escapes, reserved-env overrides); skills (nested not
discovered, symlinked dir skipped, name/description/frontmatter rules,
allowed-toolsvalidated but not granted); manifest/schema gating
(unsupported version explicit, unrelated schema falls through, escaping
plugin.jsonsymlink rejects rather than falls back). Every invalid entry
was isolated — valid siblings always loaded. - Redirect credential guard (
http-wire.mjs, 4/4 against real loopback
redirector/target pair —04-http-redirect-wire.png): with configured
headers or requestAuthorization, the redirect is stopped (redirect: manual) and the target server receives zero requests; without secrets
the redirect still follows; non-agent-plugin servers keep pre-existing
follow-and-forward behavior (see Finding 1). - Real MCP connection (
stdio-wire.mjs, 8/8 —05-stdio-mcp-wire.png):
a real@​modelcontextprotocol/sdkstdio server spawned via
createTransportfrom the normalized config reports back the env it
actually received: expandedPLUGIN_ROOT,PLUGIN_DATA, custom env, and
cwdinsidePLUGIN_DATA; a tampered config missing runtime roots is
rejected before spawn. - Tests are load-bearing (
mutations.mjs, 7/7 kills —
06-mutation-matrix.png): converter AgentPlugins branch, HTTP loopback
gate, client-owned-header drop, redirect guard, reserved-env check, consent
origin exemption, and symlink-free copy (M5b) each turn exactly their pinned
PR test red with a behavioral assertion (e.g. M5b:
AssertionError: expected true to be falseon theoutside-linkexistence
check). Unmutated suites green (positive control for every suite).
Corrections
None — no prior round or bot claims needed correction.
Findings
No blocking findings. Three reviewer-worthy observations, ordered:
- Suggestion — redirect credential hardening is scoped to Agent Plugins
only (pre-existing sibling left open).http-wire.mjscell
c4-ordinary-extension-unchangedproves that for a non-agent-plugin
streamable-http config, a redirect from the configured host is still
followed with the request headers forwarded to the target
(x-api-keyobserved at the redirect target). This is pre-existing
behavior — the base build has the same fetch path and the PR explicitly
gates the guard onagentPluginV1 === true— so it is not a regression
and not attributable to this PR. Naming it because it is the same defect
class this PR chose to close; generalizing the guard later would be a
behavior change for ordinary extensions and deserves its own discussion.
Repro:node http-wire.mjs packages/core/dist(cell c4 row). - Nit — one redundant clause in
copyExtension's symlink handling (dead
guard, defense-in-depth). Mutation M5 (disabling only
if (options.skipSymlinks && stats.isSymbolicLink()) return false;)
survived: withskipSymlinks, the filter alreadylstats and admits
onlyisFile() || isDirectory(), which a symlink is not, so the early
return changes nothing. The behavior itself IS pinned — the finer mutation
M5b (reverting todereference: true+stat) is killed by the install
test'soutside-linkassertion. Classifying per the mutation protocol:
the clause is redundant dead code, not a coverage gap; removing it or
keeping it as a named invariant are both fine. - Nit — URL-gate over/under-reach documentation. Two shapes resolve
differently than a string-reading suggests, both safely:
http://[::ffff:127.0.0.1]/(IPv4-mapped loopback) is rejected
(fail-closed over-rejection), whilehttp://2130706433/andhttp://127.1/
are accepted because WHATWGURL— the same implementation used by
both the validator andfetch/undici — normalizes them to127.0.0.1
before either validation or connection, so there is no
parser-differential bypass. No change requested; worth a line in the
design doc so the next reviewer does not flag the decimal form.
Not covered
- Windows behavior (PR self-reports
⚠️ ):reservedEnvironmentName
case-folding,path.win32bare-command root check, and backslash rejections
were verified by code reading and unit tests only; this run is Linux, which
also partially fills the PR's Linux E2E gap (real install/link/connect ran
here). - Real GitHub / archive / npm install transports: exercised only through
the PR's mocked unit tests (100 github tests green); no network fetch in
this sandbox. The local-source path — the one the A/B targets — ran for real. - Full CLI binary E2E (
qwen extensions install <dir>): the lifecycle was
driven at theExtensionManagerAPI level (the exact code the CLI calls);
CLI-side surfaces are covered by the consent/utils/PluginDetailView unit
tests in the gate. - Repo-wide lint/typecheck/test: not re-run (PR CI covers them); gates
here were the affected test files only (457 tests). - Per-commit attribution: trivially satisfied — the snapshot lists one
commit and exactly that commit (79003284) is locally reachable between
HEAD^1..HEAD^2in the depth-2 checkout. previous-report.md: absent — first verification round.
Methodology
Environment: the CI verify container (node:22-bookworm), working tree at
refs/pull/8834/merge (depth 2), npm ci + npm run build pre-run at HEAD.
A/B: git worktree add tmp/base-tree HEAD^1, rebuilt only
packages/core inside it (tsc --build emitted despite pre-existing
standalone type errors in files untouched by the PR — runtime JS complete;
packages/core has no internal workspace deps, and its non-hoisted external
deps were linked in from the root install, valid because the PR changes
neither package.json nor the lockfile). The harness imports each arm's
dist by explicit file URL and asserts the loaded module's realpath is inside
the arm's tree (control-loaded-from-dist-root, both arms). Wire harnesses
use real loopback HTTP servers and a real @modelcontextprotocol/sdk stdio
server — no stubs of code under test. Mutations were applied one at a time
in-tree, each followed by its pinned vitest file and git restore (final
git status clean). Harnesses, per-mutation logs, and the base build log
live alongside this report (*.mjs, logs/); six evidence captures in
evidence/. Assertion accounting: A/B 25 + security matrix 68 + HTTP wire 4 +
stdio wire 8 + vitest gates 457 + mutation kills 7 = 569 executed, 0 failed.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
tmux real-user testing: timeout - workflow run The tmux test did not complete before the time limit. This is not a pass/fail result for the affected flow; see the workflow run and artifacts for details. Full tmux transcript— Qwen Code · tmux real-user testing |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (windows-latest / macos-latest) platform test jobs were skipped in CI; the suite ran locally on Linux only (the Windows gap is the subject of finding R1-1).
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: (none — all planned checks completed within budget).
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (windows-latest / macos-latest) platform test jobs were skipped in CI; the suite ran locally on Linux only (the Windows gap is the subject of finding R1-1)。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：(none — all planned checks completed within budget)。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| let config = loadedManifest.config; | ||
| if (loadedManifest.format === 'qwen') { | ||
| config = resolveEnvVarsInObject(config); | ||
| } |
There was a problem hiding this comment.
[Suggestion] Untested changed behavior (mutation-verified): the deliberate exemption of Agent Plugins configs from resolveEnvVarsInObject (gated to format 'qwen') has no test — no fixture puts $VAR/${VAR} in a portable manifest field. Mutation: removing the gate leaves all 145 tests green; probe: with the gate gone, a manifest description: "Uses ${R114_PROBE_VAR}" is rewritten from process.env at load — Concrete cost: the 'portable files are left unchanged' contract silently breaks; a future refactor drops the guard and plugin manifests get env-substituted, suite green.
Fix: in an install/load test, stub an env var and give plugin.json a description/version containing ${THAT_VAR}; assert the loaded config retains the literal placeholder.
中文说明
未测试的变更行为（已突变验证）：Agent Plugins 配置有意豁免 resolveEnvVarsInObject（门控为 'qwen' 格式）这一行为没有测试——没有 fixture 在可移植清单字段中放 $VAR/${VAR}。突变：移除门控后全部 145 个测试仍绿；探针：移除门控后，清单 description: "Uses ${R114_PROBE_VAR}" 会在加载时被 process.env 重写。具体代价：'可移植文件保持不变'的契约被静默破坏；未来重构删除该防护后插件清单会被环境变量替换，套件绿灯。修复：在安装/加载测试中 stub 一个环境变量，给 plugin.json 的 description/version 加入 ${THAT_VAR}，断言加载后的配置保留字面占位符。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| const effectiveRoot = | ||
| installMetadata?.type === 'link' | ||
| ? installMetadata.source | ||
| : extensionRoot; |
There was a problem hiding this comment.
[Suggestion] Untested changed behavior (trace-verified): change detection for linked extensions in this fingerprint rewrite is only tested for Agent Plugins — the only type: 'link' installs through ExtensionManager in the suite are the new agent-plugin tests; every refreshCacheIfSourcesChanged test uses qwen-format extensions written directly into the extensions dir (no linked installs). The linked branch (sidecar read, effectiveRoot from installMetadata.source, manifest selection) is all new in this PR — Concrete cost: a regression in the linked-entry branch (e.g. stamping a nonexistent manifest when metadata is absent, or an always-constant stamp) breaks refreshCacheIfSourcesChanged for linked qwen extensions — edits to the linked source are never picked up — with no failing test.
Fix: add a test: link a qwen-format extension, edit its qwen-extension.json at the source path, expect refreshCacheIfSourcesChanged() true then the new version loaded.
中文说明
未测试的变更行为（已溯源验证）：这次指纹重写中链接扩展的变更检测只为 Agent Plugins 测试过——套件中唯一通过 ExtensionManager 的 type: 'link' 安装就是新的 agent-plugin 测试；所有 refreshCacheIfSourcesChanged 测试都使用直接写入扩展目录的 qwen 格式扩展（无链接安装）。链接分支（sidecar 读取、来自 installMetadata.source 的 effectiveRoot、清单选择）都是本 PR 新增。具体代价：链接条目分支的回归（如元数据缺失时戳记不存在的清单，或戳记恒为常量）会破坏链接 qwen 扩展的 refreshCacheIfSourcesChanged——对链接源的编辑永远不会被拾取——且没有失败测试。修复：新增测试：链接一个 qwen 格式扩展，在源路径编辑其 qwen-extension.json，期望 refreshCacheIfSourcesChanged() 为 true 且新版本被加载。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| try { | ||
| manifestPath = resolveContainedExistingPath( | ||
| effectiveRoot, | ||
| manifestPath, | ||
| ); | ||
| } catch { | ||
| followManifestSymlink = false; | ||
| } |
There was a problem hiding this comment.
[Suggestion] Untested changed behavior (mutation-verified): this containment fallback for a symlinked plugin.json has no test — plugin.json symlinks appear only in manifest.test.ts (helper-level), and the linked-plugin fingerprint test uses a plain-file manifest. The path is reachable: an escaping-symlink plugin.json classifies as 'supported', so the loop takes this branch. Two mutants survive the whole suite: (1) deleting the try/catch makes extensionDirFingerprint throw → every refreshCacheIfSourcesChanged status read rejects; (2) dropping the followManifestSymlink wiring makes stampPath follow the escaping symlink and stamp the out-of-root target (probe: stamp flips from the link's own metadata to the outside file's) — Concrete cost: either regression ships green — status reads breaking, or the fingerprint coupled to unrelated external file edits.
Fix: add a case where a linked plugin's plugin.json symlinks outside the source root: assert fingerprinting does not throw and refreshCacheIfSourcesChanged() stays stable.
中文说明
未测试的变更行为（已突变验证）：符号链接 plugin.json 的这个包含性回退没有测试——plugin.json 符号链接只出现在 manifest.test.ts（辅助函数层级），链接插件指纹测试使用纯文件清单。该路径可达：逃逸符号链接的 plugin.json 会被分类为 'supported'，循环因此进入此分支。两个突变体在整个套件中存活：(1) 删除 try/catch 会使 extensionDirFingerprint 抛错 → 每次 refreshCacheIfSourcesChanged 状态读取都 reject；(2) 去掉 followManifestSymlink 接线会使 stampPath 跟随逃逸符号链接并戳记根外目标（探针：戳记从链接自身元数据翻转为外部文件元数据）。具体代价：任一回归都会绿灯通过——状态读取损坏，或指纹与无关外部文件编辑耦合。修复：新增用例：链接插件的 plugin.json 符号链接到源根外，断言指纹计算不抛错且 refreshCacheIfSourcesChanged() 保持稳定。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| } else if (agentPluginStatus === 'supported') { | ||
| originSource = 'AgentPlugins'; | ||
| } else if (fs.existsSync(configFilePath)) { |
There was a problem hiding this comment.
[Suggestion] Untested changed behavior (mutation-verified): the documented precedence 'supported plugin.json beats an existing qwen-extension.json' (design doc, Compatibility) has no test pinning it — the three new converter tests cover supported plugin.json alone, unsupported + qwen-extension.json, and unrelated + qwen-extension.json; none writes a supported plugin.json alongside qwen-extension.json. Mutation: reordering the chain so the qwen manifest is checked before the 'supported' branch leaves all three tests green, and a directory with both manifests then installs as QwenCode — Concrete cost: a future refactor silently inverts the precedence (different origin, different activated components), no test fires.
Fix: add a converter test that writes both a supported plugin.json and a qwen-extension.json into the same root and asserts convertCompatibleExtension resolves with originSource 'AgentPlugins'.
中文说明
未测试的变更行为（已突变验证）：文档声明的优先级"受支持的 plugin.json 优先于已存在的 qwen-extension.json"（设计文档 Compatibility 节）没有测试固定——三个新 converter 测试分别覆盖：单独的受支持 plugin.json、unsupported + qwen-extension.json、unrelated + qwen-extension.json；没有一个同时写入受支持 plugin.json 和 qwen-extension.json。突变：调整链顺序使 qwen 清单先于 'supported' 分支检查后三个测试全绿，此时含两个清单的目录会按 QwenCode 安装。具体代价：未来重构静默反转优先级（不同来源、不同激活组件），无测试报警。修复：新增 converter 测试：同一根目录写入受支持 plugin.json 和 qwen-extension.json，断言 convertCompatibleExtension 以 originSource 'AgentPlugins' 完成。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| </InfoRow> | ||
| )} | ||
| {ext.installMetadata?.originSource && ( | ||
| <InfoRow label={t('Origin:')}> |
There was a problem hiding this comment.
[Suggestion] Untested changed behavior (trace-verified): this new Origin: InfoRow is untested — the only test touching PluginDetailView (ExtensionActionsView.test.tsx) mocks the entire view to null and asserts on the mock's props; there is no PluginDetailView.test.tsx. The CLI string twin (extensionToOutputString) did gain a test — Concrete cost: a change that drops or breaks this InfoRow (e.g. an installMetadata field rename) passes CI; users inspecting an installed Agent Plugin in the TUI silently stop seeing its origin.
Fix: render PluginDetailView directly with an extension whose installMetadata.originSource is 'AgentPlugins' and assert the Origin row renders (and is absent when originSource is undefined).
中文说明
未测试的变更行为（已溯源验证）：这个新的 Origin: InfoRow 没有测试——唯一涉及 PluginDetailView 的测试（ExtensionActionsView.test.tsx）把整个视图 mock 为 null 并对 mock 的 props 断言；不存在 PluginDetailView.test.tsx。CLI 字符串孪生（extensionToOutputString）是有测试的。具体代价：删除或破坏该 InfoRow 的改动（如 installMetadata 字段重命名）会通过 CI；在 TUI 中查看已安装 Agent Plugin 的用户会静默看不到来源。修复：直接渲染 PluginDetailView，传入 installMetadata.originSource 为 'AgentPlugins' 的扩展，断言 Origin 行渲染（且 originSource 为 undefined 时不渲染）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (windows-latest / macos-latest) platform test jobs were skipped in CI; the suite ran locally on Linux only.
Not explored to full depth (tool budget reached): chunk 7: I did not actually run extensionManager.test.ts — because node_modules is not installed in the worktree or the parent checkout, and I opted not to perform a ful…; chunk 7: did not execute extensionManager.test.ts — node_modules is absent in this worktree and the parent checkout, and a full monorepo install exceeded the remaining….
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (windows-latest / macos-latest) platform test jobs were skipped in CI; the suite ran locally on Linux only。
未探索到全部深度（达到工具调用预算）：chunk 7：I did not actually run extensionManager.test.ts — because node_modules is not installed in the worktree or the parent checkout, and I opted not to perform a ful…；chunk 7：did not execute extensionManager.test.ts — node_modules is absent in this worktree and the parent checkout, and a full monorepo install exceeded the remaining…。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| } | ||
|
|
||
| function isLoopbackHost(hostname: string): boolean { | ||
| const normalized = hostname.replace(/^\[|\]$/g, '').toLowerCase(); |
There was a problem hiding this comment.
[Suggestion] R2-4: isLoopbackHost coerces each label with Number(), but WHATWG URL parsing already rejected labels like 1e0/+1 as IPv4 and keeps them as opaque DNS hostnames — new URL('http://127.0.0.1e0/mcp') parses with hostname 127.0.0.1e0. Number('1e0') and Number('+1') both yield 1, so this classifies those hostnames as loopback and validateHttpUrl waives the HTTPS requirement. Probe through loadAgentPluginMcpServers: http://127.0.0.1e0/mcp and http://127.0.0.+1/mcp with configured headers survive while the http://example.com/mcp control is rejected; a per-label decimal-digit restriction flips the probe — Concrete cost: under any resolver that maps such a name off-host (hosts file, corporate/wildcard DNS), plugin-configured headers travel over plaintext HTTP despite the "non-loopback endpoints must use HTTPS" policy. (Legitimate alternate IPv4 notations are unaffected — WHATWG canonicalizes them to dotted quads before this check.)
| const normalized = hostname.replace(/^\[|\]$/g, '').toLowerCase(); | |
| function isLoopbackHost(hostname: string): boolean { | |
| const normalized = hostname.replace(/^\[|\]$/g, '').toLowerCase(); | |
| if (normalized === 'localhost' || normalized === '::1') return true; | |
| const labels = normalized.split('.'); | |
| return ( | |
| labels.length === 4 && | |
| labels.every( | |
| (label, index) => | |
| /^\d{1,3}$/.test(label) && | |
| Number(label) <= 255 && | |
| (index !== 0 || Number(label) === 127), | |
| ) | |
| ); |
(suggestion shows the top of the replacement; drop the old ipv4 lines below)
中文说明
isLoopbackHost 用 Number() 强制转换每个标签，但 WHATWG URL 解析早已拒绝 1e0/+1 这类标签作为 IPv4，并将其保留为不透明 DNS 主机名——new URL('http://127.0.0.1e0/mcp') 解析出的主机名就是 127.0.0.1e0。由于 Number('1e0') 和 Number('+1') 都等于 1，这些主机名会被判定为 loopback，validateHttpUrl 随之豁免 HTTPS 要求。已通过 loadAgentPluginMcpServers 探针确认：带配置 headers 的 http://127.0.0.1e0/mcp 与 http://127.0.0.+1/mcp 通过校验，而对照组 http://example.com/mcp 被拒绝；按标签限制为十进制数字后探针翻转。具体代价：在任何会把此类名字解析到非本机的解析器下（hosts 文件、企业/通配 DNS），插件配置的 headers 会违反"非 loopback 端点必须使用 HTTPS"的策略、以明文 HTTP 传输。（合法的 IPv4 变体写法不受影响——WHATWG 在到达本检查前已将其规范化为点分十进制。）
— qwen3.8-max via Qwen Code /review (v0.21.8)
| mcpServers: await loadAgentPluginMcpServers( | ||
| effectiveExtensionPath, | ||
| this.extensionStore.agentPluginDataRoot(extensionId), | ||
| { createDataDir: true }, |
There was a problem hiding this comment.
[Suggestion] R2-5: loadExtension unconditionally passes { createDataDir: true }, and installExtensionInternal runs this load on the staging copy before the prepareOnly branch — so any agent plugin with a stdio server mkdirs <storeDir>/plugin-data/agent-plugins/<id> (plus ${PLUGIN_DATA} cwd subdirs) during the staged preview. If the prepared mutation is never applied (user declines the batched update, apply step fails), cleanupPreparedExtension removes staging + settings but has no callback for the data dir — settings in the same flow get explicit commit/discard, this side effect gets neither. Probe via prepareExtensionInstall + disposePreparedExtension: the data dir exists after both; deferring creation to the committed load flips the probe with all 129 extensionManager tests still passing — Concrete cost: every prepare-then-discard cycle orphans a persistent data directory in the extension store permanently.
Fix: pass createDataDir: false for the staged preview load (the post-commit loadExtension({ extensionDir: destinationPath }) already creates the dirs).
中文说明
loadExtension 无条件传入 { createDataDir: true }，而 installExtensionInternal 在 prepareOnly 分支之前就对 staging 副本执行该加载——因此任何带 stdio server 的 agent plugin 在预演阶段就会 mkdir <storeDir>/plugin-data/agent-plugins/<id>（以及 ${PLUGIN_DATA} cwd 子目录）。若准备好的变更从未应用（用户拒绝批量更新、apply 失败），cleanupPreparedExtension 会删除 staging 与 settings，却没有针对数据目录的回调——同一流程中的 settings 有明确的 commit/discard，这个副作用两者皆无。已通过 prepareExtensionInstall + disposePreparedExtension 探针确认：prepare 与 dispose 之后数据目录均存在；将创建推迟到提交后的加载可翻转探针，且全部 129 个 extensionManager 测试仍通过。具体代价：每次"预演后丢弃"都会在扩展商店目录中永久遗留一个数据目录。修复：预演加载传 createDataDir: false（提交后的 loadExtension({ extensionDir: destinationPath }) 已会创建目录）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| method: 'POST', | ||
| redirect: 'manual', | ||
| }); | ||
| expect(fetchFn.mock.calls[2]?.[1]).toEqual({ method: 'POST' }); |
There was a problem hiding this comment.
[Suggestion] R2-6: This test covers credentialed agentPluginV1 servers (manual) and one header-less non-plugin server (passthrough), but never an agentPluginV1: true server with no configured headers and no Authorization — which must keep default redirect following. Probe: simplifying stopAgentPluginRedirect to mcpServerConfig?.agentPluginV1 === true passes all 113 mcp-client tests; the proposed credential-less case fails under that mutant (redirect: 'manual' appears) and passes on the PR code — Concrete cost: a future refactor dropping the credentials precondition ships silently, and credential-less plugin servers whose endpoints redirect (http→https, trailing-slash) get a raw 3xx surfaced to the SDK transport and fail to connect with an opaque error.
Fix: add a fifth case — createStreamableHttpCompatibilityFetch('plugin-no-credentials', fetchFn, { httpUrl: 'https://example.com/mcp', agentPluginV1: true }), call with { method: 'POST' }, assert strict toEqual({ method: 'POST' }) (no redirect key).
中文说明
该测试覆盖了带凭据的 agentPluginV1 server（manual）和一个无 headers 的非插件 server（透传），但从未覆盖无配置 headers 且无 Authorization 的 agentPluginV1: true server——它必须保持默认的重定向跟随。探针：把 stopAgentPluginRedirect 简化为 mcpServerConfig?.agentPluginV1 === true 后全部 113 个 mcp-client 测试仍通过；补充无凭据用例后该突变体会失败（出现 redirect: 'manual'），在 PR 代码上则通过。具体代价：未来重构若丢弃凭据前置条件将静默通过测试，而无凭据、端点会重定向（http→https、结尾斜杠）的插件 server 会收到透出的原始 3xx，连接以晦涩错误失败。修复：新增第五个用例——createStreamableHttpCompatibilityFetch('plugin-no-credentials', fetchFn, { httpUrl: 'https://example.com/mcp', agentPluginV1: true })，以 { method: 'POST' } 调用，严格断言 toEqual({ method: 'POST' })（不得出现 redirect 键）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| } | ||
| for (const [field, fieldValue] of Object.entries(value)) { | ||
| if (!AUTHOR_FIELDS.has(field)) { | ||
| throw new Error(`Unknown Agent Plugins author field "${field}".`); |
There was a problem hiding this comment.
[Suggestion] R2-7: This interpolates a manifest-controlled value into a thrown error without stripAnsiAndControl — the same applies to the Unsupported Agent Plugins schema throw (~line 109) and the unknown-field debugLogger.warn. claude-converter.ts documents this exact threat ("a hostile plugin … could otherwise smuggle ANSI/OSC sequences to the terminal during a failed install") and aliases stripAnsiAndControl as sanitizeForError; qoder-converter.ts does the same. loadExtensionManifest wraps the raw message and the TUI install catch renders redactUrlCredentials(getErrorMessage(error)), which strips URL userinfo but not control characters. Probe: author keys / $schema carrying ESC/OSC sequences survive into the rendered error (has_esc=true); wrapping the values flips to has_esc=false — Failure scenario: installing a plugin from a hostile repo whose plugin.json author key or $schema contains ANSI/OSC escapes clears/rewrites the terminal output during the failed install.
| throw new Error(`Unknown Agent Plugins author field "${field}".`); | |
| throw new Error(`Unknown Agent Plugins author field "${stripAnsiAndControl(field)}".`); |
(plus stripAnsiAndControl(schema) at the schema throw; import from ../../utils/textUtils.js)
中文说明
此处把清单可控的值直接插入抛出的错误信息而未使用 stripAnsiAndControl——Unsupported Agent Plugins schema 抛出（约第 109 行）与未知字段的 debugLogger.warn 同样如此。claude-converter.ts 记录了完全相同的威胁（"恶意插件……可能在安装失败时向终端走私 ANSI/OSC 序列"）并将 stripAnsiAndControl 别名为 sanitizeForError；qoder-converter.ts 亦然。loadExtensionManifest 原样包装错误信息，TUI 安装 catch 渲染 redactUrlCredentials(getErrorMessage(error))——它只剥离 URL userinfo，不处理控制字符。探针：携带 ESC/OSC 序列的 author 键/$schema 会原样进入渲染后的错误（has_esc=true）；包裹这些值后翻转为 has_esc=false。失败场景：安装来自恶意仓库、plugin.json 的 author 键或 $schema 含 ANSI/OSC 转义的插件时，失败安装过程中终端输出被清空/改写。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| )} | ||
| {ext.installMetadata?.originSource && ( | ||
| <InfoRow label={t('Origin:')}> | ||
| {ext.installMetadata.originSource} |
There was a problem hiding this comment.
[Suggestion] R2-8: This renders sidecar-derived originSource raw, while the adjacent rows in this same component sanitize extension-controlled values (stripUnsafeCharacters(ext.version ?? '') above, redactUrlCredentials(ext.installMetadata.source) below); stripUnsafeCharacters is already imported here. The identical raw interpolation also ships in packages/cli/src/commands/extensions/utils.ts:100. loadInstallMetadata is JSON.parse with zero field validation, and manually placed extension directories are loaded without an installer — a crafted .qwen-extension-install.json is a reachable input. Probe on the CLI sink: originSource carrying screen-clear/cursor-home/OSC-title escapes reaches the output verbatim; wrapping in stripAnsi flips it — Concrete cost: an attacker-placed extension directory spoofs the terminal (cleared screen, rewritten title bar, fake UI text) when the user views the extension in the manager or runs extensions list.
| {ext.installMetadata.originSource} | |
| {stripUnsafeCharacters(String(ext.installMetadata.originSource))} |
(and stripAnsi(ext.installMetadata.originSource ?? '') at utils.ts:100)
中文说明
此处原样渲染来自 sidecar 的 originSource，而同一组件中相邻的行都会对扩展可控值做净化（上方 stripUnsafeCharacters(ext.version ?? '')、下方 redactUrlCredentials(ext.installMetadata.source)）；且 stripUnsafeCharacters 已在本文件导入。packages/cli/src/commands/extensions/utils.ts:100 也带着同样的原样插值。loadInstallMetadata 是零字段校验的 JSON.parse，手动放置的扩展目录无需安装器即可被加载——伪造的 .qwen-extension-install.json 是可达输入。CLI 出口探针：携带清屏/光标归位/OSC 标题转义的 originSource 原样到达输出；用 stripAnsi 包裹后翻转。具体代价：攻击者放置的扩展目录可在用户于扩展管理器查看或运行 extensions list 时欺骗终端（清屏、改写标题栏、伪造 UI 文本）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| body: match[2].trim(), | ||
| level: 'extension', | ||
| }; |
There was a problem hiding this comment.
[Suggestion] R2-9: parseAgentPluginSkill silently drops the Qwen-semantic frontmatter fields the sibling parser (parseSkillContent, skill-load.ts) extracts and the skill system honors — notably the invocation controls disable-model-invocation and user-invocable — with no diagnostic, unlike loadAgentPluginManifest in this same directory, which warns on every unrecognized plugin.json field. Probe: a SKILL.md with disable-model-invocation: true parses to a SkillConfig lacking the flag; SkillTool's listing filter (.filter((s) => !s.disableModelInvocation && ...)) then exposes the skill to the model. skill-load.ts:146-155 documents this exact failure as a prior regression ("the flag silently stripped") — Failure scenario: an Agent Plugin ships skills/my-tool/SKILL.md with disable-model-invocation: true; the skill parses fine and becomes model-invocable despite the author's restriction, with no log line indicating the drop. (The allowed-tools drop is deliberate and test-asserted; these restriction-side controls are different — dropping them makes skills more permissive.)
Fix: map the applicable control fields (at minimum disable-model-invocation and user-invocable, reusing skill-load.ts's lenient coercion), or emit a debugLogger.warn for unrecognized frontmatter keys mirroring loadAgentPluginManifest.
中文说明
parseAgentPluginSkill 静默丢弃了兄弟解析器（skill-load.ts 的 parseSkillContent）会提取、且 skill 系统会生效的 Qwen 语义 frontmatter 字段——尤其是调用控制 disable-model-invocation 与 user-invocable——且无任何诊断；而同目录下的 loadAgentPluginManifest 对每个无法识别的 plugin.json 字段都会告警。探针：带 disable-model-invocation: true 的 SKILL.md 解析出的 SkillConfig 缺少该标志；SkillTool 的列表过滤（.filter((s) => !s.disableModelInvocation && ...)）随后把该 skill 暴露给模型。skill-load.ts:146-155 把完全相同的失败记录为历史回归（"该标志被静默剥离"）。失败场景：Agent Plugin 携带 disable-model-invocation: true 的 skills/my-tool/SKILL.md 时，skill 正常解析却尽管作者限制仍变为可被模型调用，且无任何日志提示该丢弃。（allowed-tools 的丢弃是有意且已有测试断言；这些限制性控制字段不同——丢弃它们会使 skill 变得更宽松。）修复：映射相应控制字段（至少 disable-model-invocation 与 user-invocable，复用 skill-load.ts 的宽松强制转换），或仿照 loadAgentPluginManifest 对无法识别的 frontmatter 键输出 debugLogger.warn。
— qwen3.8-max via Qwen Code /review (v0.21.8)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: cross-file tracer — internals of readMcpServers on the ACP inventory path not audited against the new agentPluginV1 field — stopped at the agent tool budget.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest / windows-latest) platform test jobs were skipped in CI; the suite ran locally on Linux only.
Not explored to full depth (tool budget reached): chunk 3: running mcp.test.ts under vitest (worktree has no node_modules ; skipped full monorepo install — all verification was static tracing plus the errno probe).; 本 PR（#8834）为 Qwen Code 扩展系统新增 Agent Plugins v1 原生加载支持。: none — all checks above completed within budget.; 本 PR（#8834）为 Qwen Code 扩展系统新增 Agent Plugins v1 原生加载支持。: none — all checks I started were completed within budget.; 本 PR（#8834）为 Qwen Code 扩展系统新增 Agent Plugins v1 原生加载支持。: none — all planned checks completed within budget.; 本 PR（#8834）为 Qwen Code 扩展系统新增 Agent Plugins v1 原生加载支持。: none — all checks above completed within budget., and 2 more.
中文说明
已审查。 建议见行内评论。
未审查：cross-file tracer — internals of readMcpServers on the ACP inventory path not audited against the new agentPluginV1 field — stopped at the agent tool budget。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — Test (macos-latest / windows-latest) platform test jobs were skipped in CI; the suite ran locally on Linux only。
未探索到全部深度（达到工具调用预算）：chunk 3：running mcp.test.ts under vitest (worktree has no node_modules ; skipped full monorepo install — all verification was static tracing plus the errno probe).；本 PR（#8834）为 Qwen Code 扩展系统新增 Agent Plugins v1 原生加载支持。：none — all checks above completed within budget.；本 PR（#8834）为 Qwen Code 扩展系统新增 Agent Plugins v1 原生加载支持。：none — all checks I started were completed within budget.；本 PR（#8834）为 Qwen Code 扩展系统新增 Agent Plugins v1 原生加载支持。：none — all planned checks completed within budget.；本 PR（#8834）为 Qwen Code 扩展系统新增 Agent Plugins v1 原生加载支持。：none — all checks above completed within budget.，另有 2 条。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| for (const [name, server] of Object.entries(servers)) { | ||
| if (server.command) delete servers[name]; | ||
| } |
There was a problem hiding this comment.
[Suggestion] One stdio server's cwd-subdir mkdir failure silently disables every stdio server in the plugin. The createDataDir block wraps the pluginDataRoot mkdir and the per-server cwd mkdir loop in one try; this catch deletes every server with a command — including servers whose cwd lives under ${PLUGIN_ROOT} and never needed the data directory. Probe-confirmed at this commit: two stdio servers (A with cwd: "${PLUGIN_DATA}/work", B with default cwd) plus a regular file pre-created at <dataRoot>/work → mkdir throws EEXIST (recursive mkdir tolerates existing directories, not files) → surviving servers []; a per-server try/catch fix flips it to ['b']. The only diagnostic is a debugLogger.warn gated on QWEN_DEBUG_LOG_FILE. — Failure scenario: a previous run legitimately created a regular file at <dataRoot>/work → on the next session start server B, which does not use PLUGIN_DATA, silently never starts (the same over-broad outcome follows from EACCES on any single cwd subdir).
Fix: wrap the per-server await fs.promises.mkdir(server.cwd, { recursive: true }) in its own try/catch and delete only the failed server (needs Object.entries(servers) so the name is available), keeping the outer catch for pluginDataRoot-level failures:
for (const [name, server] of Object.entries(servers)) {
if (
server.command &&
server.cwd &&
isPathWithin(resolvedPluginDataRoot, server.cwd)
) {
try {
await fs.promises.mkdir(server.cwd, { recursive: true });
} catch {
delete servers[name]; // only the server whose cwd could not be created
}
}
}(Implementing this also requires updating the fixture of 'keeps HTTP servers when the stdio data directory cannot be created' — see the existing thread on mcp.test.ts:188.)
中文说明
单个 stdio server 的 cwd 子目录 mkdir 失败会静默禁用插件内的所有 stdio server。createDataDir 块把 pluginDataRoot 的 mkdir 与逐 server 的 cwd mkdir 循环包在同一个 try 中，而这个 catch 会删除所有带 command 的 server——包括 cwd 位于 ${PLUGIN_ROOT} 下、根本不需要数据目录的 server。已在本提交探针确认：两个 stdio server（A 的 cwd 为 ${PLUGIN_DATA}/work，B 为默认 cwd）且 <dataRoot>/work 处预先存在一个普通文件 → mkdir 抛 EEXIST（递归 mkdir 容忍已存在的目录但不容忍文件）→ 存活的 server 为 []；改为按 server 的 try/catch 后翻转为 ['b']。唯一诊断是受 QWEN_DEBUG_LOG_FILE 门控的 debugLogger.warn。失败场景：上一次运行在 <dataRoot>/work 合法创建了普通文件 → 下次会话启动时完全不使用 PLUGIN_DATA 的 server B 静默地永远不启动（任一 cwd 子目录 EACCES 同样导致过大范围删除）。修复：把逐 server 的 mkdir 包进各自的 try/catch，只删除失败的 server（需改用 Object.entries(servers) 以拿到名称），外层 catch 保留给 pluginDataRoot 级失败。（实现该修复还需更新 'keeps HTTP servers when the stdio data directory cannot be created' 的 fixture——见 mcp.test.ts:188 上的既有线程。）
— qwen3.8-max via Qwen Code /review (v0.21.8)
| (ext) => | ||
| ext.isActive && | ||
| ext.format !== 'agent-plugins-v1' && | ||
| ext.installMetadata?.originSource !== 'AgentPlugins', |
There was a problem hiding this comment.
[Suggestion] The R2-1 fix's originSource clause is load-bearing but pinned by no test — mutation-verified at this commit: removing ext.installMetadata?.originSource !== 'AgentPlugins' leaves the whole 41-test suite green, while the new regression test exercises only the format clause (mock with format: 'agent-plugins-v1', no installMetadata). — Failure scenario: a linked Agent Plugin whose source directory later drops plugin.json and gains a qwen-extension.json reloads with format: 'qwen' while installMetadata.originSource stays 'AgentPlugins' (no re-consent on reload); without this clause — silently removable today — its commands/ directory would activate as live slash commands, violating the documented "commands are ignored for Agent Plugins" invariant that consent relied on.
Fix: add a test case where the mock extension has no format but installMetadata: { type: 'link', source: '…', originSource: 'AgentPlugins' }, and assert commands is still [].
中文说明
R2-1 修复中的 originSource 子句是承重条款但没有任何测试固定——已在本提交做突变验证：删除 ext.installMetadata?.originSource !== 'AgentPlugins' 后全部 41 个测试仍绿，而新增回归测试只覆盖了 format 子句（mock 只带 format: 'agent-plugins-v1'、无 installMetadata）。失败场景：一个链接安装的 Agent Plugin，其源目录后来删除 plugin.json 并新增 qwen-extension.json，重载后 format: 'qwen' 而 installMetadata.originSource 仍为 'AgentPlugins'（重载不会重新征求同意）；若没有这个今天可被静默删除的子句，其 commands/ 目录将激活为可用的斜杠命令，违反同意时所依赖的"Agent Plugins 忽略 commands"文档契约。修复：新增测试用例——mock 扩展不带 format 但带 installMetadata: { type: 'link', source: '…', originSource: 'AgentPlugins' }，断言 commands 仍为 []。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| (ext) => | ||
| ext.isActive && | ||
| ext.format !== 'agent-plugins-v1' && |
There was a problem hiding this comment.
[Suggestion] Agent Plugins v1's manifest files are never classified in ExtensionFileWatcher (packages/cli/src/config/extension-file-watcher.ts), so live-reload semantics break in both directions for the new format — the diff's own CLI-side special-casing (this filter) shows consumers needed per-format handling the watcher never got. Probe-confirmed against the real watcher at this commit: (1) linked plugins — an edit of mcp.json/plugin.json reaches getRuntimePathRefreshAction, matches none of EXTENSION_FILES/STALE_DIRS/AUTO_REFRESH_DIRS and returns false: no refresh is ever marked (the same edit to a linked qwen-format extension marks 'stale'); (2) installed plugins — getUserExtensionRefreshAction's !fs.existsSync(.../qwen-extension.json) catch-all returns 'stale' for ANY file change under the extension dir (Agent Plugins never have that file), including an stdio server writing runtime state into its default cwd ${PLUGIN_ROOT} (the package directory): 2 spurious stale marks observed vs 0 for a qwen-format dir. The suggested fix flips both arms. — Failure scenario: a developer links a plugin (qwen extensions link ./my-agent-plugin, advertised by this PR's new docs) and edits mcp.json mid-session → the session serves stale/absent MCP servers until restart; separately, runtime writes by an installed plugin trigger spurious "Extensions changed on disk. Run /reload-plugins" prompts (the reconcile itself is incremental, so the real cost is the misleading prompt plus a full cache-refresh cycle).
Fix: in extension-file-watcher.ts, add 'plugin.json' and 'mcp.json' to EXTENSION_FILES (fixes linked reloads), and treat a directory containing plugin.json as a classified package in getUserExtensionRefreshAction instead of the blanket 'stale' (fixes spurious markers).
中文说明
Agent Plugins v1 的清单文件从未在 ExtensionFileWatcher（packages/cli/src/config/extension-file-watcher.ts）中分类，导致新格式的热重载语义双向都坏了——本 diff 自己在 CLI 侧的特殊处理（即这个过滤器）正说明各消费方需要按格式处理，而 watcher 从未得到。已在本提交用真实 watcher 探针确认：(1) 链接插件——对 mcp.json/plugin.json 的编辑到达 getRuntimePathRefreshAction 后不匹配 EXTENSION_FILES/STALE_DIRS/AUTO_REFRESH_DIRS 中任何一组，返回 false：永远不会标记刷新（同样的编辑对链接的 qwen 格式扩展会标记 'stale'）；(2) 已安装插件——getUserExtensionRefreshAction 的 !fs.existsSync(.../qwen-extension.json) 兜底会对扩展目录下任何文件变更返回 'stale'（Agent Plugins 从没有该文件），包括 stdio server 向默认 cwd ${PLUGIN_ROOT}（即包目录）写运行时状态：观察到 2 个虚假 stale 标记，而 qwen 格式目录为 0。建议的修复可使两侧翻转。失败场景：开发者链接插件（qwen extensions link ./my-agent-plugin，本 PR 新文档宣传的工作流）并在会话中编辑 mcp.json → 会话一直使用过期/缺失的 MCP server 直到重启；另外，已安装插件的运行时写入会触发虚假的 "Extensions changed on disk. Run /reload-plugins" 提示（reconcile 本身是增量的，真实代价是误导性提示加一次完整缓存刷新）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| } catch (error) { | ||
| debugLogger.warn( | ||
| `Disabling Agent Plugins MCP: ${error instanceof Error ? error.message : String(error)}`, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Every load-time rejection of mcp.json goes only through this gated debugLogger.warn — probe-confirmed at this commit: with the default environment (no QWEN_DEBUG_LOG_FILE), a typo'd $schema or one stray unknown top-level field disables the entire MCP surface and writes nothing to stdout/stderr, while the extension loads active with zero servers; mirroring the warn to stderr flips the probe. No visible surface carries this state (Extension has no warnings field; the TUI hides an empty MCP section; consent lists only server names/URLs). The subsystem's own convention already documents this exact gate problem and mirrors to stderr — corruptFile.ts: "debugLogger.warn is gated behind QWEN_DEBUG_LOG_FILE (unset for almost all users) … Surface … on stderr too." The skills loader has the same pattern — see the companion comment on skills.ts. — Failure scenario: a plugin author ships mcp.json with a typo'd $schema (or one stray unknown top-level field): the whole MCP surface is disabled, the extension shows installed and enabled, no tools ever appear, and nothing anywhere explains why.
| } catch (error) { | |
| debugLogger.warn( | |
| `Disabling Agent Plugins MCP: ${error instanceof Error ? error.message : String(error)}`, | |
| ); | |
| } catch (error) { | |
| const message = `Disabling Agent Plugins MCP: ${ | |
| error instanceof Error ? error.message : String(error) | |
| }`; | |
| // debugLogger.warn is gated behind QWEN_DEBUG_LOG_FILE (unset for almost | |
| // all users); mirror the corruptFile.ts convention and surface on stderr. | |
| process.stderr.write(`[warn] ${message}\n`); | |
| debugLogger.warn(message); |
中文说明
对 mcp.json 的所有加载期拒绝都只经过这个受门控的 debugLogger.warn——已在本提交探针确认：默认环境（未设 QWEN_DEBUG_LOG_FILE）下，$schema 拼写错误或多出一个未知顶层字段会禁用整个 MCP 面且 stdout/stderr 无任何输出，扩展却以零 server 的状态显示为已加载；把该 warn 镜像到 stderr 后探针翻转。没有任何可见面承载该状态（Extension 没有 warnings 字段；TUI 会隐藏空的 MCP 区块；同意界面只列出 server 名称/URL）。扩展子系统自身的惯例已明确记录了这个门控问题并镜像到 stderr——corruptFile.ts："debugLogger.warn is gated behind QWEN_DEBUG_LOG_FILE (unset for almost all users) … Surface … on stderr too."。skills 加载器存在同样的模式——见 skills.ts 上的配套评论。失败场景：插件作者的 mcp.json 写错了 $schema（或多了一个未知顶层字段）：整个 MCP 面被禁用，扩展显示已安装已启用，却永远没有任何工具出现，也没有任何地方解释原因。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| debugLogger.warn( | ||
| `Skipping Agent Plugins skill "${entry.name}": ${error instanceof Error ? error.message : String(error)}`, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] Same invisible-diagnostic pattern as the companion comment on mcp.ts, in the skills loader: every per-skill rejection (name mismatch, invalid frontmatter, escaping symlink, unreadable SKILL.md) and every whole-root disable (lines 25/30/42) goes only through gated debugLogger.warn. Probe-confirmed at this commit: a plugin with skills/MySkill/SKILL.md containing name: myskill (case mismatch) loads with zero skills and zero stdout/stderr in the default environment; with QWEN_DEBUG_LOG_FILE=1 the warn appears in the debug log only. Mirroring the warn to stderr flips the probe. A fix anchored on mcp.ts alone would not cover these sites. — Failure scenario: a ported v1 plugin whose skill directory case-mismatches its name, or typos a frontmatter field: the skill is skipped with no trail; the extension shows active, /skills lists nothing from it.
| debugLogger.warn( | |
| `Skipping Agent Plugins skill "${entry.name}": ${error instanceof Error ? error.message : String(error)}`, | |
| ); | |
| const message = `Skipping Agent Plugins skill "${entry.name}": ${ | |
| error instanceof Error ? error.message : String(error) | |
| }`; | |
| process.stderr.write(`[warn] ${message}\n`); | |
| debugLogger.warn(message); |
中文说明
与 mcp.ts 上的配套评论相同的"诊断不可见"模式，出现在 skills 加载器：每一次按 skill 的拒绝（名称不匹配、frontmatter 非法、符号链接逃逸、SKILL.md 不可读）以及每一次整目录禁用（第 25/30/42 行）都只经过受门控的 debugLogger.warn。已在本提交探针确认：skills/MySkill/SKILL.md 内含 name: myskill（大小写不匹配）的插件在默认环境下加载结果为零个 skill 且 stdout/stderr 无任何输出；设 QWEN_DEBUG_LOG_FILE=1 后该 warn 只出现在 debug 日志文件中。把 warn 镜像到 stderr 后探针翻转。只修 mcp.ts 无法覆盖这些位置。失败场景：移植的 v1 插件 skill 目录名与名称大小写不一致，或 frontmatter 字段拼错：skill 被跳过且无任何痕迹；扩展显示为激活，/skills 却列不出它的任何内容。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| function isLoopbackHost(hostname: string): boolean { | ||
| const normalized = hostname.replace(/^\[|\]$/g, '').toLowerCase(); | ||
| if (normalized === 'localhost' || normalized === '::1') return true; |
There was a problem hiding this comment.
[Suggestion] isLoopbackHost misclassifies three machine-local host forms as non-loopback, so validateHttpUrl rejects plain-HTTP mcp.json servers that never leave the machine — probe-confirmed through the real loader at this commit: http://[::ffff:127.0.0.1]:3000/mcp (WHATWG URL normalizes the IPv4-mapped IPv6 loopback to hostname [::ffff:7f00:1], matching neither the ::1 arm nor the dotted-quad arm), http://0.0.0.0:3000/mcp (INADDR_ANY — connects to loopback on Linux/macOS/Windows, and the form server tooling commonly prints), and http://localhost.:3000/mcp (FQDN trailing dot) are all rejected with "Non-loopback Streamable HTTP endpoints must use HTTPS" — factually wrong for loopback hosts — while the extension loads active. The direction is fail-safe (URL pre-normalizes octal/hex/shorthand IPv4 like 0177.0.0.1/127.1/2130706433 to canonical dotted quads before the classifier; no non-loopback host is admitted), hence Suggestion. — Failure scenario: a plugin ships a machine-local streamable-http server using any of the three forms; it is dropped at load with a misleading HTTPS error.
Fix: strip one trailing dot before comparison; accept 0.0.0.0; and for ::ffff:-prefixed hosts decode the mapped tail and re-run the 127-first-octet test — note the tail serializes as compressed hex (::ffff:7f00:1), so a dotted-quad-only tail test does NOT cover it (verified: it flips 0.0.0.0/localhost. but not the mapped form).
中文说明
isLoopbackHost 把三种本机 host 形态误判为非回环，导致 validateHttpUrl 拒绝根本不离开本机的纯 HTTP mcp.json server——已在本提交通过真实 loader 探针确认：http://[::ffff:127.0.0.1]:3000/mcp（WHATWG URL 把 IPv4 映射 IPv6 回环规范化为 hostname [::ffff:7f00:1]，既不匹配 ::1 分支也不匹配点分四段分支）、http://0.0.0.0:3000/mcp（INADDR_ANY——在 Linux/macOS/Windows 上都连接到回环，且是服务器工具常打印的形态）、http://localhost.:3000/mcp（带尾点的 FQDN）全部被以 "Non-loopback Streamable HTTP endpoints must use HTTPS" 拒绝——对回环 host 而言该报错与事实不符——而扩展仍显示为激活。方向是失败安全的（URL 会在分类前把 0177.0.0.1/127.1/2130706433 等八进制/十六进制/简写 IPv4 预先规范化为标准点分四段；不会放行任何非回环 host），故为 Suggestion。失败场景：插件携带使用上述任一形态的本机 streamable-http server，加载时即被误导性 HTTPS 错误丢弃。修复：比较前去掉一个尾点；接受 0.0.0.0；对 ::ffff: 前缀的 host 解码映射尾部并重跑 127 首字节判断——注意尾部序列化为压缩十六进制（::ffff:7f00:1），只处理点分四段的尾部测试覆盖不到（已验证：该写法能翻转 0.0.0.0/localhost. 但翻转不了映射形态）。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| if (!CLIENT_OWNED_HEADERS.has(lowercaseName)) { | ||
| Object.defineProperty(normalized, name, { |
There was a problem hiding this comment.
[Suggestion] Client-owned headers set by the plugin are dropped with zero diagnostics — the only header case that neither throws nor warns (not even a debugLogger.warn), while every other header problem (invalid name, invalid value, duplicate) throws a clear load-time error. The drop itself is deliberate hardening (design doc; test-blessed), but the portable spec's streamable-http headers allow arbitrary header names (additionalProperties: string in the live schema), so a fully compliant plugin may ship headers: { "Authorization": "Bearer …" }. Consent shows only server names/URLs — nothing at install or runtime hints at the drop. — Failure scenario: a compliant plugin declares an Authorization header; the entry is dropped silently, the server loads active and appears in consent, requests go out without the header, and the endpoint answers 401/403 with nothing pointing at the cause.
Fix: emit at least a debugLogger.warn — ideally the user-visible channel proposed in the companion mcp.ts:69 comment — when a provided header is dropped as client-owned (an else branch here).
中文说明
插件设置的 client-owned 头部被静默丢弃且零诊断——这是唯一既不抛错也不告警（连 debugLogger.warn 都没有）的头部情形，而其他所有头部问题（非法名称、非法取值、重复）都会抛出清晰的加载期错误。丢弃本身是有意加固（设计文档有述、测试已固定），但 portable 规范中 streamable-http 的 headers 允许任意头部名（线上 schema 为 additionalProperties: string），因此完全合规的插件可以携带 headers: { "Authorization": "Bearer …" }。同意界面只显示 server 名称/URL——安装时和运行时都没有任何信息提示该丢弃。失败场景：合规插件声明了 Authorization 头部；该条目被静默丢弃，server 正常加载并出现在同意列表，请求却不带该头部发出，端点返回 401/403，且没有任何线索指向原因。修复：在提供的头部因 client-owned 被丢弃时（此处加一个 else 分支）至少输出 debugLogger.warn——理想情况是走 mcp.ts:69 配套评论建议的用户可见通道。
— qwen3.8-max via Qwen Code /review (v0.21.8)
| let allowedRoot: string; | ||
| if (cwd === './' || cwd.startsWith('./')) { | ||
| allowedRoot = pluginRoot; |
There was a problem hiding this comment.
[Suggestion] normalizeCwd classifies the allowed root from the pre-expansion string, so a cwd mixing ./ with a plugin variable passes load validation but can never exist. Probe-confirmed at this commit: './${PLUGIN_DATA}/work' matches this ./ branch (allowedRoot = pluginRoot) before expandPluginVariables runs; expansion yields ./<absolute-data-path>/work, which path.resolve re-nests as <pluginRoot>/<absolute-data-path>/work — literally inside the plugin root, so containment passes and the server loads active; createDataDir never creates it (not inside the data root), and validateAgentPluginStdioRuntimePaths throws a raw ENOENT at every connect. Control '${PLUGIN_DATA}/work' creates the dir and validates OK; rejecting variable tokens in the ./ branch flips the probe. The intended load-time error never fires and no test covers the mixed form. Direction is fail-safe (no containment escape), hence Suggestion. — Failure scenario: a plugin ships cwd: "./${PLUGIN_DATA}/work" (an easy authoring mistake — both prefixes are advertised): it installs, consents, and loads active, then every connect attempt fails with a raw ENOENT pointing at a nonsense nested path.
| let allowedRoot: string; | |
| if (cwd === './' || cwd.startsWith('./')) { | |
| allowedRoot = pluginRoot; | |
| let allowedRoot: string; | |
| if (cwd === './' || cwd.startsWith('./')) { | |
| if (/\$\{PLUGIN_(ROOT|DATA)\}/.test(cwd)) { | |
| throw new Error( | |
| 'Stdio cwd must not mix "./" with ${PLUGIN_ROOT} or ${PLUGIN_DATA}.', | |
| ); | |
| } | |
| allowedRoot = pluginRoot; |
中文说明
normalizeCwd 依据展开前的字符串判定允许的根，因此混用 ./ 与插件变量的 cwd 能通过加载校验却永远不可能存在。已在本提交探针确认：'./${PLUGIN_DATA}/work' 在 expandPluginVariables 运行前就命中这个 ./ 分支（allowedRoot = pluginRoot）；展开得到 ./<绝对数据路径>/work，path.resolve 把它重新嵌套为 <pluginRoot>/<绝对数据路径>/work——字面上位于插件根内，于是包含性检查通过、server 加载为激活；createDataDir 从不创建它（它不在数据根内），validateAgentPluginStdioRuntimePaths 在每次连接时抛出原始 ENOENT。对照组 '${PLUGIN_DATA}/work' 正常创建目录并通过校验；在 ./ 分支拒绝变量 token 后探针翻转。预期的加载期错误从不触发，也没有测试覆盖混合形态。方向是失败安全的（无包含性逃逸），故为 Suggestion。失败场景：插件携带 cwd: "./${PLUGIN_DATA}/work"（很容易写错——两种前缀都是文档宣传的）：安装、同意、激活都正常，随后每次连接都以指向无意义嵌套路径的原始 ENOENT 失败。
— qwen3.8-max via Qwen Code /review (v0.21.8)
|
@wenshao @tanzhenxin @yiliang114 @LaZzyMan @doudouOUC Could one of you please review the current head |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 1100 passed · 0 failed · 1100 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：1100 通过 · 0 失败 · 1100 总计 Verification reportPR 8834 Deep Verification — Round 2 —
|
| # | finding (round 1) | severity | status at 0f8a8ffa |
|---|---|---|---|
| F1 | Redirect credential guard scoped to Agent Plugins only; ordinary extensions still forward configured headers across redirects | Suggestion | stands — re-measured, not diffed: http-wire.mjs cell c4-ordinary-extension-unchanged at the new head shows the redirect target still receives x-api-key for a non-agent-plugin config (guard code unchanged; explicitly keyed on agentPluginV1 === true). Not a regression; same accepted-scope conclusion. |
| F2 | Redundant clause in copyExtension symlink handling (explicit early-return dead alongside the lstat + type filter) |
Nit | stands — re-mutated at the new head: M5 (drop the clause) survived again; M5b (lstat→stat) killed again by the outside-link absence assertion. The clause remains defense-in-depth. |
| F3 | URL-gate over/under-reach: decimal IPs accepted, [::ffff:127.0.0.1] rejected |
Nit | stands, extended — re-measured: http://2130706433/ and http://127.1/ still accepted because WHATWG URL normalizes both to 127.0.0.1 before validation and connection (no parser differential); http://[::ffff:127.0.0.1]/ still rejected (fail-closed). New this round: ideographic/fullwidth-dot forms (127.0.0。1, 127．0．0．1) are UTS46-mapped to 127.0.0.1 and accepted for the same reason, while the security-critical sibling http://10。0.0.1/ normalizes to 10.0.0.1 and is rejected. |
Central claim + A/B (re-measured at the new head)
Central claim (unchanged): an unmodified Agent Plugins v1 package installs and
links through the existing extension lifecycle without conversion — identified as
AgentPlugins, portable skills and stdio/Streamable HTTP MCP loaded, stable
PLUGIN_DATA, byte-identical package, no generated qwen-extension.json — where
base fails with Configuration file not found .../qwen-extension.json.
Base arm: git worktree add tmp/base-tree HEAD^1 (962dc8eadc — note main
advanced since round 1's 57e6c310; this A/B tests exactly what lands), rebuilt
only packages/core there. Evidence: 01-ab-install-link-base-vs-head.png.
| cell | oracle | base (962dc8eadc) |
head (0f8a8ffa) |
|---|---|---|---|
| realpath control | loaded module inside arm tree | ✅ | ✅ |
| control: qwen-extension installs | install succeeds | ✅ ctrl-ext |
✅ ctrl-ext |
| install agent plugin | result | ❌ Configuration file not found .../qwen-extension.json (expected red = PASS) |
✅ installed |
| link agent plugin | result | ❌ same (expected red = PASS) | ✅ linked, path === source, format agent-plugins-v1 |
| origin / version / format | metadata | — | ✅ AgentPlugins, 1.0.0, agent-plugins-v1 |
| skills | discovery | — | ✅ [direct], allowed-tools validated but not granted |
| non-portable inactive | commands/agents/hooks/settings/channels | — | ✅ all empty/undefined |
| MCP gating | loaded keys | — | ✅ [local, remote] (sse skipped) |
| consent payload | origin + skills | — | ✅ AgentPlugins, skills listed, commands/subagents empty |
| byte fidelity | 4 files + no qwen-extension.json + escaping symlink dropped |
— | ✅ 6/6 |
| env expansion | PLUGIN_ROOT/PLUGIN_DATA/args/cwd vs realpath |
— | ✅ all expanded, data dir outside package |
| update + reinstall | same PLUGIN_DATA, state.txt survives, version bumps |
— | ✅ both |
Counts: base 4/4, head 29/29 — 33/33 A/B assertions (ab-install.mjs).
Delta probes — the two fix commits (22/22)
Commit attribution (reconstructed pairwise from local objects; the shallow
graft hides commit 3's parent from rev-list, so git show on it alone is
misleading): commit 2 68f678ac = FileCommandLoader exclusion (originSource),
manifest isFile(), stdio cwd mkdir, converter pluginName gating, copyExtension
realpath; commit 3 0f8a8ffa = FileCommandLoader format check, carried
plugin.json removal, link-source guards + format field. Evidence:
02-delta-fix-commit-probes.png.
- FileCommandLoader exclusion — a real installed agent plugin carrying
commands/deploy.tomlloads zero commands through the real
FileCommandLoader(dist), while an ordinary extension'shello.tomlin the
same run still loads (bystander control). - Non-regular
plugin.json— directory →unrelated; symlink-to-file still
read (statSyncfollows); dangling symlink →unrelated; escaping symlink
fails closed end-to-end: detection returnssupported(deliberate, per
round 1) andinstallExtensionthen rejects with
Path "..." resolves outside plugin root "..."— never installs, never falls
through to another format. - stdio cwd in
PLUGIN_DATA—${PLUGIN_DATA}/workcreated and passes
runtime validation;${PLUGIN_DATA}/../evilrejected at normalize (not
created, valid sibling loads); a symlinked component inside the data root
pointing outside is rejected (realpath-based containment), no mkdir through
it. - Marketplace precedence + carried manifest removal — explicit selection →
Claude, requested name, carriedplugin.jsonabsent from the converted
copy; future-schema (2.0.0) carried manifest also removed; without a
selection the root agent manifest still wins (AgentPlugins). - Link-metadata isolation —
source: undefined | 42 | ''in
.qwen-extension-install.json:refreshCache()resolves, valid siblings
load, broken entries don't. - Symlinked source root — install through a dir symlink succeeds, files
copied (exercises thecopyExtensionrealpath change).
Wire oracles at the new head
- HTTP redirect guard (
http-wire.mjs, 10/10 —
03-http-redirect-wire.png): real loopback redirector/target pair. Agent
plugin with configured headers →307returned, target sees 0 requests;
with requestAuthorizationonly → same; without secrets → redirect follows;
ordinary config → follows and forwardsx-api-key(F1 re-measure). - Real MCP stdio connection (
stdio-wire.mjs, 9/9 —
04-stdio-mcp-wire.png): a real@modelcontextprotocol/sdkserver launched
through corecreateTransportwith the normalized config reports the env/cwd
it actually received — expandedPLUGIN_ROOT,PLUGIN_DATA, custom env, cwd
PLUGIN_DATA/work(delta-3's mkdir exercised through the real connection
path); a tampered config missing runtime roots is rejected before spawn. - Security matrix (
security-matrix.mjs, 55/55): 20 URL-gate accept/reject
shapes + 2 normalization corroboration cells, client-owned header drop,
duplicate/control-char/invalid-name header rejection, 10 stdio containment
shapes with 9 valid-sibling isolation assertions, transport gating
(sse/unknown/schema/top-level-field/mcp.json-as-directory), skills discovery
(nested/missing-description/symlinked excluded), manifest schema gating.
Mutation matrix (all runs this round, tree restored clean after each)
New-delta mutants — evidence: 05-mutation-matrix.png:
| mutant | guard | pinned test | result |
|---|---|---|---|
| M1 | FileCommandLoader agent-plugin exclusion | FileCommandLoader.test.ts new test |
killed — expect(commands).toEqual([]) receives the deploy command |
| M2 | converter pluginName gating |
extension-converter.test.ts marketplace test |
killed — expected 'AgentPlugins' to be 'Claude' |
| M3 | link-source guard, loadExtension site |
extensionManager.test.ts isolation tests |
survived — isolation holds anyway: a bogus source fails manifest load and the entry is skipped (defense-in-depth, classified) |
| M3b | link-source guard, fingerprint site | same | killed — refreshCache() rejects with TypeError ERR_INVALID_ARG_TYPE (path … received undefined): without this guard one corrupt metadata file crashes the whole refresh |
| M4 | stdio cwd mkdir in PLUGIN_DATA |
mcp.test.ts new test |
killed — ENOENT: ... stat '.../data/work' |
| M5 | copyExtension explicit symlink early-return |
install test outside-link assertion |
survived — lstat + isFile()/isDirectory() filter already drops symlinks (round-1 result stands) |
| M5b | copyExtension lstat→stat |
same | killed — outside-link existence flips false→true |
| M6 | manifest isFile() guard |
manifest.test.ts new non-regular test |
survived — see Finding 1 |
Round-1 mutants re-run at the new head (rerun-round1-mutations.mjs, 6/6
killed, logs mutant-R*.log): redirect guard
(expected { method: 'POST' } to match object { ... redirect: 'manual' }),
reserved-env check, client-owned-header drop, HTTP loopback gate, converter
AgentPlugins branch, consent origin exemption
(expected '...' not to contain 'Some features may not work perfectly'). The
7th round-1 mutant (symlink-free copy) is covered by M5/M5b above.
Positive controls: unmutated gates green before and after the mutation
round (identical 782 + 178 counts), proving the harness can both fail and pass.
Targeted gates
packages/core:src/extension/**+src/tools/mcp-client.test.ts— 27 files, 782/782 passed (includes all new delta tests).packages/cli:src/services/FileCommandLoader.test.ts+src/commands/extensions/**— 13 files, 178/178 passed.
Corrections
None this round.
Findings
No blocking findings. One new nit, plus the three carried-over items.
- Nit (new) — the new "non-regular root manifest" test does not pin the
guard it was written for along the directory axis; the guard's load-bearing
axis is FIFO non-blocking (untested). Mutation M6 (deleting only
if (!fs.statSync(resolvedManifestPath).isFile()) return 'unrelated';)
survived: for a directoryplugin.json, the surroundingcatchalready
returnsunrelatedbecausereadFileSyncthrowsEISDIR. The guard's real
value is special files: without it,readFileSyncon a FIFO blocks
indefinitely (open-for-read waits for a writer) and stalls extension refresh.
Measured with the guard in place: a FIFOplugin.jsonreturnsunrelatedin
0 ms. The fixture that would pin this axis is a FIFO namedplugin.json
(the directory fixture pins nothing the catch didn't already). Same family:
M3'sloadExtension-site link-source guard survived for the analogous
reason — isolation is redundantly provided by manifest-load failure, while
the fingerprint-site guard (M3b) is the one that prevents a refresh-wide
crash. All three are defense-in-depth that works; the observation is about
the tests' pinned axes, not the code. - (carried F1) Suggestion — redirect credential hardening remains Agent
Plugins-only. Re-measured at the new head (http-wire.mjsc4): an
ordinary streamable-http config still follows redirects with configured
headers forwarded. Pre-existing, explicitly scoped by the PR, not a
regression. - (carried F2) Nit —
copyExtension's explicit symlink early-return
remains redundant with thelstat+ type filter (M5 survived, M5b
killed). - (carried F3) Nit — URL-gate normalization worth a design-doc line:
decimal (2130706433,127.1) and UTS46 dot forms (127.0.0。1) are
accepted because WHATWGURL— used by both the validator andfetch—
normalizes them to127.0.0.1;[::ffff:127.0.0.1]is rejected
(fail-closed over-rejection); mapped non-loopback (10。0.0.1→
10.0.0.1) is rejected.
Not covered
- Windows behavior (PR self-reports
⚠️ ):reservedEnvironmentName
case-folding,path.win32bare-command root check, backslash rejections —
code reading + unit tests only; this run is Linux (which again partially
fills the PR's Linux E2E gap: real install/link/connect ran here). - Real GitHub / archive / npm transports: exercised only through mocked
unit tests (100 github tests green in the gate); no network in this sandbox. - Full CLI binary E2E (
qwen extensions install <dir>): lifecycle driven
at theExtensionManagerAPI level;FileCommandLoaderdriven through its
constructor seam with a minimal config shim (the class under test is real,
itsConfigcollaborator is a thin object) — not through a fullConfig. - Repo-wide lint/typecheck: not re-run (PR CI covers them). Note the base
control build carried 16 pre-existing type errors in files untouched by the
PR (emit proceeded; same as round 1). - Round-1 68-cell matrix: rebuilt this round as a focused 55-cell matrix —
every shape family represented, but a few round-1 edge cells (e.g. astral /
length-boundary skill frontmatter shapes) were not rebuilt; those surfaces
are covered by the green unit-test gate. - Round-1 "link manifest fingerprint reload" A/B cell: not rebuilt as a
standalone harness;refreshCacheIfSourcesChangedbehavior is covered by
extensionManager.test.tsin the gate. - Per-commit attribution caveat: the depth-2 graft makes commit 3
(0f8a8ffa) appear as a root togit show/rev-list; its true diff was
computed pairwise against the locally present commit objects
(68f678ac..0f8a8ffa). The aggregateHEAD^1..HEADdiff is what was
verified end to end.
Methodology
Environment: the CI verify container (node:22-bookworm), working tree at
refs/pull/8834/merge (depth 2; HEAD merge commit, HEAD^1 base tip,
HEAD^2 PR head), npm ci + npm run build pre-run at HEAD. A/B: base
worktree at HEAD^1, rebuilt only packages/core inside it with the
root's tsc (16 pre-existing unrelated type errors; emit complete); the base
tree's package-level node_modules was symlinked from the head tree —
packages/core has no internal @qwen-code/* dependencies (verified from
its package.json) and the PR changes neither package.json nor the lockfile,
so both arms share one dependency tree and differ only in packages/core
sources; each arm's harness asserts the loaded extensionManager.js realpath
is inside that arm's tree. Harnesses (ab-install.mjs, delta-probes.mjs,
http-wire.mjs, stdio-wire.mjs, security-matrix.mjs,
mutation-matrix.mjs, rerun-round1-mutations.mjs) import compiled dist
output by explicit file URL and run against real filesystem fixtures, real
loopback HTTP servers, and a real @modelcontextprotocol/sdk stdio server —
no stubs of code under test. Mutations were applied one at a time in-tree
(exact string replacement), each followed by its pinned vitest file and
git checkout -- restore; final git status clean (asserted by the matrix
scripts). Gates ran from the package directories with vitest. Raw logs
(logs/), per-mutant logs for R1–R6 (mutant-R*.log; M-series results are
quoted in the mutation matrix and its capture), the delta diff, and the base
build log live alongside this report; five evidence captures in evidence/. Assertion accounting: A/B 33 + delta
probes 22 + HTTP wire 10 + stdio wire 9 + security matrix 55 + new mutation
kills 5 + round-1 re-kills 6 + vitest gates 960 = 1100 executed, 0 failed.
Surviving mutants (M3, M5, M6) are classified in the matrix and Finding 1, and
are not counted as failures — they document defense-in-depth and coverage-gap
status, consistent with round 1's treatment.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed a full review pass and recorded a verdict. ✅ This pass re-reviewed the unchanged head 中文说明✅ 本轮 re-run 完成了完整审查并给出裁决：重新审查了未变化的 head |
Local end-to-end verification (maintainer run)I built both sides from source and exercised this against a real stack: the bundled CLI, real stdio and Streamable HTTP MCP servers, a real Environment
Sanity check on the bundles before running anything: The MCP servers are evidence recorders, not stubs — the stdio probe appends the argv/cwd/ 1. Before → after, on a package that is never modified
The consent screen for an Agent Plugin correctly omits the "installing an extension from {origin}" cross-ecosystem warning while still listing the MCP servers and Skills it will bring in. 2. The installed package is the original packageFour files compared by sha256, and no generated manifest anywhere under
3. Capability boundary and per-component isolationThe fixture deliberately ships everything outside the portable core: bundled commands, agents, hooks, a nested Skill, a Skill with no frontmatter, a Skill whose name disagrees with its directory, plus six MCP entries of which five are invalid or unsupported. The counts come from
The five refused MCP entries cover each rejection path individually: legacy 4. Streamable HTTP policy — checked on the wireFour of seven entries are refused at load (non-loopback plaintext HTTP, URL user-info, URL fragment,
The plugin configures For the redirect case I pointed a plugin at an endpoint that answers Mutation A/B on the redirect guard. I patched out the 5.
|
| PR head | 0f8a8ff |
| Base | af372e5（与 main 的 merge-base） |
| 构建 | 各 worktree 内 npm ci && npm run bundle → dist/cli.js |
| 平台 | macOS 15（darwin 25.6.0），Node v24.18.1 |
跑用例前先做过一次基本核对：agent-plugins.org/schemas 只出现在 head/dist，base/dist 中不存在，确认 A/B 对比的确实是本 PR 的代码。
这里的 MCP server 是取证记录器而非空壳：stdio probe 启动时会把宿主实际传入的 argv、cwd、PLUGIN_ROOT、PLUGIN_DATA 写入 PLUGIN_DATA 内的 ledger；HTTP 端点在服务端记录每一次请求的完整 header。下文关于变量展开和 header 处理的结论均取自服务端记录，而非 CLI 输出。
1. 前后对比，插件包全程未做任何修改
base 两个入口都拒绝；head 两个入口都通过，并且额外验证了插件包被嵌套一层目录的 .tar.gz 归档安装。
Agent Plugin 的 consent 界面正确地省略了「正在安装来自 {origin} 的扩展」这条跨生态警告，同时仍然列出将要运行的 MCP server 和将要安装的 Skill。
2. 安装后的包就是原始包
四个文件按 sha256 逐一比对一致，QWEN_HOME 下不存在任何生成的 manifest。stdio server 从安装副本启动并连接成功。
3. 能力边界与组件级错误隔离
fixture 刻意携带了全部超出 portable core 的内容：bundled commands、agents、hooks，一个嵌套 Skill、一个无 frontmatter 的 Skill、一个 name 与目录不一致的 Skill，以及六个 MCP entry（其中五个非法或不受支持）。plugin.json 不含 version 字段。
计数取自真实 daemon 的 GET /workspace/extensions，因此不是从 list 输出格式推断出来的：磁盘上 4 个 Skill 只加载 1 个，6 个 MCP entry 只加载 1 个，commandCount / agentCount / hookCount / contextFileCount / channelCount 全为 0，hasSettings: false。版本按预期回落到 1.0.0。损坏的同级组件不会连累有效组件。
被拒绝的五个 MCP entry 分别覆盖了各条拒绝路径：legacy sse、未知 transport、逃逸插件根目录的 cwd、绝对路径 command，以及试图自行设置 PLUGIN_DATA 的 entry。
4. Streamable HTTP 策略——在网络层验证
七个 entry 中四个在加载期即被拒绝（非 loopback 明文 HTTP、URL user-info、URL fragment、file:）。loopback HTTP 与真实 HTTPS（自签 CA，经 NODE_EXTRA_CA_CERTS 信任）均连接成功。
插件在自有 header 之外还配置了 Authorization、User-Agent、Content-Type 和 Host。服务端实际收到的是：完全没有 authorization，host: 127.0.0.1:39181（而非配置的 attacker.example），content-type: application/json，user-agent: undici；而 x-plugin-token 与 x-trace-id 原样送达。
重定向用例中，我让插件指向一个对 MCP POST 返回 307 跳转到另一个 origin 的端点。重定向源记录到 1 次请求，重定向目标记录到 0 次，且该 server 被报告为 Disconnected，没有静默成功。
针对重定向防护的变异 A/B。 我把 mcp-client.ts 中的 stopAgentPluginRedirect 判断改掉、重新 bundle，再跑同一个用例：重定向目标随即收到 4 次请求，并且带着 x-plugin-token: redirect-secret-must-not-leak。说明这道防护是真正起作用的，而非顺带产生的效果。（源码与 bundle 事后均已还原。）
5. PLUGIN_DATA 在整个生命周期中的一致性
install → extensions update → uninstall → reinstall，并在 update 之前往 PLUGIN_DATA 写入一个状态文件。
三次 server 启动上报的 PLUGIN_DATA 完全相同，状态文件在 uninstall + reinstall 后仍然存在，磁盘上始终只有一个数据目录，server 的 cwd 也确实是配置的 PLUGIN_DATA。其环境中的 PLUGIN_* 变量只有宿主注入的那两个。
6. 边界收敛、schema 门禁与既有格式
- 携带三个逃逸插件根目录符号链接的包可以正常安装，三个链接全部被丢弃；被指向的文件从未出现在
QWEN_HOME中的任何位置。 - 版本不是 1.0.0 的
agent-plugins.orgschema 会被明确报错拒绝，既不猜测也不回落到其他格式。 - 根目录
plugin.json若不带 Agent Plugins$schema，判定仍为unrelated，同级的qwen-extension.json依旧生效。 - Claude standalone（
.claude-plugin/plugin.json）→Origin: Claude且命令正常加载；原生 Qwen extension →Origin: QwenCode且命令正常加载，两者行为均未改变。 - 归档安装的报错信息中已包含
plugin.json (Agent Plugins)。
7. 单元测试
packages/core 涉及的 8 个文件共 430 条通过；packages/cli 的 consent、extensions/utils、FileCommandLoader 共 75 条通过。无失败。
两条观察——均不阻塞合并
a. 格式优先级是静默的。 convertCompatibleExtension 中 getAgentPluginSchemaStatus() 先于 qwen-extension.json 检查执行，因此同时携带 v1 plugin.json 和 qwen-extension.json 的包会被当作 Agent Plugin 加载：Qwen manifest 的 name、version 和 commands/ 会被忽略且没有任何提示（见最后一张图，安装结果是 dual-manifest 7.7.7 而不是 dual-manifest-qwen 1.1.1）。这个优先级本身是合理的，但如果作者给现有 Qwen extension 补一个 plugin.json，其命令会无声失效。建议在 docs/users/extension/agent-plugins.md 补一句说明，或在两种 manifest 同时存在时打一条 debugLogger.warn。
b. extensions list 的空小节（仅观感问题）。 Agent Plugins 分支总会给 mcpServers 赋值（没有 MCP 时为 {}），而 Qwen extension 保持 undefined；extensionToOutputString 按真值判断，于是没有 MCP server 的 Agent Plugin 会打印一个空的 MCP servers: 标题。上文 symlink-plugin 和 dual-manifest 的输出中可见。加一个 Object.keys(...).length > 0 判断即可。
未覆盖范围
仅 macOS，未跑 Windows 与 Linux。安装来源覆盖了本地路径、link 和本地归档，未覆盖 Agent Plugins 的 git / GitHub-release / npm / marketplace 来源。Skill 与 MCP 工具的发现和连通性已验证，但未驱动真实模型回合去实际调用它们。
结论：LGTM。 实际行为与 PR 描述一致，安全相关的防护在对抗性 fixture 和变异 A/B 下均成立。
|
Thank you for the thorough maintainer E2E verification and LGTM. GitHub still reports CHANGES_REQUESTED because the two earlier bot reviews remain blocking, while the latest bot result and this LGTM are comments rather than approving reviews. Could you please submit a formal approval and, if you have permission, dismiss the stale bot change-request reviews? The branch protection also needs a second approving review. I will keep the two explicitly non-blocking observations as follow-up items so this late review round does not widen the PR. |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 1067 passed · 0 failed · 1067 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：1067 通过 · 0 失败 · 1067 总计 Verification report<!-- qwen-triage:verify --> PR 8834 Deep Verification — Round 3 —
|
| # | finding (round 2) | severity | status at 0f8a8ffa (re-measured this round) |
|---|---|---|---|
| F1 | Redirect credential guard scoped to Agent Plugins only; ordinary extensions still forward configured headers across redirects | Suggestion | stands — http-wire.mjs c4: ordinary config with a configured x-api-key (applied to the request the way the SDK transport does) follows the 307 and the target receives the header; c5: ordinary config follows with request Authorization (the header itself is stripped cross-origin by the fetch platform — INFO c5 authorization-forwarded…=false — so F1's practical surface is configured custom headers). Guard code unchanged, explicitly keyed on agentPluginV1 === true. Not a regression; same accepted-scope conclusion. |
| F2 | Redundant clause in copyExtension symlink handling |
Nit | stands — re-mutated: M5 (drop the explicit isSymbolicLink() early-return) survived again; M5b (lstat→stat) killed again by the outside-link absence assertion. Defense-in-depth. |
| F3 | URL-gate normalization: decimal/fullwidth-dot forms accepted, [::ffff:127.0.0.1] rejected |
Nit | stands — re-measured in security-matrix.mjs: http://2130706433/, http://127.1/, http://127.0.0。1/ accepted (WHATWG URL normalizes to 127.0.0.1 before validation and connection — same parser both sides, no differential); http://[::ffff:127.0.0.1]/ and http://10。0.0.1/ rejected (fail-closed). |
| N1 | New "non-regular root manifest" test does not pin the guard along the directory axis; guard's load-bearing axis is FIFO non-blocking | Nit | stands — M6 (delete isFile() guard) survived again on the directory fixture (the surrounding catch already returns unrelated on EISDIR); FIFO probe: with the guard in place, getAgentPluginSchemaStatus on a FIFO plugin.json returns unrelated in 0.06 ms (delta-probes.mjs). The fixture that would pin the FIFO axis remains absent. |
Central claim + A/B
Central claim (unchanged): an unmodified Agent Plugins v1 package installs and links through the existing extension lifecycle without conversion — identified as AgentPlugins, portable skills and stdio/Streamable HTTP MCP loaded, stable PLUGIN_DATA, byte-identical package, no generated qwen-extension.json — where base fails with Configuration file not found .../qwen-extension.json.
Base arm: git worktree add tmp/base-tree HEAD^1, rebuilt only packages/core there. This round improved the control over round 2: symlinking the head tree's packages/core/node_modules (the nested dedupe-conflict directory: diff, mime, ajv, fdir, ignore, picomatch, undici) into the base tree reduced the base build's pre-existing type errors from 16 to 1 (@lydell/node-pty declaration resolution, in a PR-untouched file not on the harness import path; emit complete). The PR touches neither package.json nor the lockfile, so both arms share one dependency tree; packages/core has no internal @qwen-code/* dependencies (verified from its package.json), and each arm's harness asserts the loaded extensionManager.js realpath is inside that arm's tree. Evidence: 01-ab-install-link-base-vs-head.png.
| cell | oracle | base (962dc8eadc) |
head (0f8a8ffa) |
|---|---|---|---|
| realpath control | loaded module inside arm tree | ✅ | ✅ |
| control: qwen-extension installs | install succeeds | ✅ ctrl-ext |
✅ ctrl-ext |
| install agent plugin | result | ❌ Configuration file not found .../qwen-extension.json (expected red = PASS) |
✅ installed |
| link agent plugin | result | ❌ same (expected red = PASS) | ✅ linked |
| link path/format | metadata | — | ✅ path === source, agent-plugins-v1 |
| origin / version / format | metadata | — | ✅ AgentPlugins, 1.0.0, agent-plugins-v1 |
| skills | discovery | — | ✅ [direct], allowed-tools validated but not granted |
| non-portable inactive | commands/agents/context/hooks/settings/channels | — | ✅ all empty/undefined |
| MCP gating | loaded keys | — | ✅ [local, remote] (sse skipped), both agentPluginV1 |
| consent payload | origin + skills | — | ✅ AgentPlugins, skills listed, commands/subagents empty |
| byte fidelity | 4 files + no qwen-extension.json + escaping symlink dropped |
— | ✅ 6/6 |
| env expansion | PLUGIN_ROOT/PLUGIN_DATA/args/cwd |
— | ✅ all expanded, data dir outside package |
| update + reinstall | same PLUGIN_DATA, state.txt survives, version bumps |
— | ✅ both |
| default version | manifest without version |
— | ✅ 1.0.0 |
Counts: base 4/4, head 16/16 — 20/20 A/B assertions (ab-install.mjs).
Delta probes — the two fix commits (14/14)
Commit attribution reconstructed pairwise from locally present commit objects (79003284..68f678ac: 10 files +175/−8; 68f678ac..0f8a8ffa: 6 files +75/−5 — matching round 2's attribution; the depth-2 graft hides commit 3's parent link, and rev-list HEAD^1..HEAD^2 returns 1, so per-commit rows are cited only for the two fix commits whose pairwise diffs were locally computable). Evidence: 02-delta-fix-commit-probes.png.
- FileCommandLoader exclusion — through the real cli
FileCommandLoader(dist) with a config shim: an agent-plugin extension (format: 'agent-plugins-v1') carryingcommands/deploy.tomlAND a qwen-format extension withoriginSource: 'AgentPlugins'carryingcommands/conv.tomlboth load zero commands (both OR clauses exercised), while an ordinary extension'shello.tomlin the same run still loads (bystander control). - Escaping-symlink
plugin.json— detection returnssupported(deliberate, per round 1/2), andinstallExtensionthen fails closed withPath "…" resolves outside plugin root "…"— never installs, never falls through to another format. - FIFO
plugin.json— returnsunrelatedin 0.06 ms (N1's load-bearing axis, measured live). ${PLUGIN_DATA}cwd creation + escape rejection —${PLUGIN_DATA}/workcreated undercreateDataDir: true;${PLUGIN_DATA}/../evilrejected at normalize (not created, valid sibling loads); a symlinked component inside the data root pointing outside is rejected (realpathcontainment) and nothing is created through it.- Marketplace precedence + carried manifest removal — explicit selection →
Claude+ requested name + carriedplugin.jsonabsent from the converted copy; without a selection the root agent manifest wins (AgentPlugins, dir unchanged). - Link-metadata isolation —
source: undefined | 42 | ''entries:refreshCache()resolves, valid linked agent-plugin sibling loads, broken entries don't. - Symlinked source root — install through a dir symlink succeeds, files copied,
installMetadata.sourceis the symlink path (exercises thecopyExtensionrealpath change).
Wire oracles
- HTTP redirect guard (
http-wire.mjs, 6/6 —03-http-redirect-wire.png): real loopback redirector→target pair driven through the realcreateStreamableHttpCompatibilityFetchwith realfetch. Agent plugin + configured headers → 307 returned, target sees 0 requests; agent plugin + requestAuthorization→ same; agent plugin without secrets → redirect followed; agent plugin with the configured header present on the request → still stopped; ordinary config → follows and forwardsx-api-key(F1 re-measure). - Real MCP stdio connection (
stdio-wire.mjs, 7/7 —04-stdio-mcp-wire.png): a real@modelcontextprotocol/sdkserver (the reference implementation as oracle) runs as the plugin's./bin/server.mjs; core'sloadAgentPluginMcpServersnormalizes the config,createTransportspawns it, an SDKClientconnects, and the child reports what it actually received: expandedPLUGIN_ROOT/PLUGIN_DATA, expanded custom env and args, cwd =${PLUGIN_DATA}/work(the delta-4 mkdir exercised through the real connection path). A tampered config missing runtime roots is rejected before spawn.
Security matrix (53/53)
security-matrix.mjs — 05-security-matrix.png: 18 URL-gate accept/reject shapes (including the F3 normalization cells and http://999.0.0.1/ opaque-host rejection), client-owned header silent drop, custom headers kept, duplicate-case/invalid-name/control-char-value header rejection, 11 stdio containment shapes with a valid-sibling isolation assertion plus Linux case-sensitivity of the reserved-env check, transport gating (sse/unknown/future-schema/unknown-top-field/mcp.json-as-directory), skill discovery gating (only valid direct-child loads; nested, name-mismatched, description-less, bad-case, symlinked, and file-as-skills all excluded), manifest schema gating (supported/unsupported/unrelated/directory/absent/invalid-JSON) and name validation.
Mutation matrix (4 killed / 3 survived-as-classified, tree clean after)
Each mutant applied to source, pinned vitest target run, tree restored (GIT_CLEAN_AFTER_MATRIX=true). Positive controls green before (core pinned files 148/148, cli 41/41) and after (gates below). Evidence: 06-mutation-matrix-as-recorded.png, per-mutant logs logs/mutant-M*.log.
| mutant | guard | pinned target | result |
|---|---|---|---|
| M1 | FileCommandLoader agent-plugin exclusion | FileCommandLoader.test.ts new test |
killed — expected [ { name: 'deploy', … } ] to deeply equal [] |
| M3 | link-source guard, loadExtension site |
extensionManager.test.ts isolation tests |
survived — isolation holds anyway: a bogus source fails manifest load and the entry is skipped (defense-in-depth) |
| M3b | link-source guard, fingerprint site | same | killed — promise rejected "TypeError: The "path" argument must be of…" (ERR_INVALID_ARG_TYPE) instead of resolving: without it one corrupt metadata file crashes the whole refresh |
| M4 | stdio cwd mkdir in PLUGIN_DATA |
mcp.test.ts new test |
killed — ENOENT: … stat '…/data/work' |
| M5 | copyExtension explicit symlink drop |
install test outside-link assertion |
survived — the lstat + isFile()/isDirectory() filter already drops symlinks (F2) |
| M5b | copyExtension lstat→stat |
same | killed — outside-link existence flips false→true |
| M6 | manifest isFile() guard |
manifest.test.ts non-regular test |
survived — directory fixture pins nothing the catch didn't already (N1); the FIFO axis is what the guard carries |
Every kill failed the intended behavioral assertion (quoted above), not an import/compile break. Survivors M3/M5/M6 are classified defense-in-depth/coverage-gap observations consistent with rounds 1–2 — not counted as failures.
Targeted gates
packages/core:src/extension/**+src/tools/mcp-client.test.ts— 27 files, 782/782 passed (identical count to round 2).packages/cli:src/services/FileCommandLoader.test.ts+src/commands/extensions/**— 13 files, 178/178 passed (identical count to round 2).
Corrections
None this round. One mechanism clarification, not a correction: round 2's F1 cell applied configured headers to the request the way the SDK transport does; this round's first attempt left them off the request and measured forwarded=false — confirming the configured headers are attached by the transport layer, not by the compatibility fetch wrapper, and that the wrapper's guard only decides the redirect policy. The F1 statement stands as written.
Findings
No new findings. The four carried items (F1 Suggestion; F2, F3, N1 nits) all stand, re-measured above. None block.
Not covered
- Windows behavior (PR self-reports
⚠️ ):reservedEnvironmentNamecase-folding,path.win32bare-command root check, backslash rejections — code reading + unit tests only; this run is Linux. - Real GitHub / archive / npm transports: unit tests only (in the gate); no network in this sandbox.
- Full CLI binary E2E (
qwen extensions install <dir>): lifecycle driven at theExtensionManagerAPI level;FileCommandLoaderthrough its constructor seam with a minimal config shim (the class under test is real; theConfigcollaborator is a thin object). - Repo-wide lint/typecheck: not re-run (PR CI covers them). Base control build carried 1 pre-existing type error in a PR-untouched file after the nested-
node_modulesfix (16 before it); emit complete in both cases. - Round-1 68-cell matrix edge cells (astral/length-boundary skill frontmatter shapes): not rebuilt; those surfaces are covered by the green unit-test gate.
packages/acp-bridge/packages/sdk-typescriptchanges: type-only union additions of'AgentPlugins'; no behavioral surface to A/B; covered by PR CI typecheck.- Per-commit attribution: commits 1–3 reachable only as pairwise diffs under the depth-2 graft; the aggregate
HEAD^1..HEADdiff (33 files, +2475/−64) is what was verified end to end.
Methodology
Environment: the CI verify container (node:22-bookworm), working tree at refs/pull/8834/merge (depth 2; HEAD merge, HEAD^1 base tip, HEAD^2 PR head), npm ci + npm run build pre-run at HEAD. A/B: base worktree at HEAD^1 with only packages/core rebuilt there (root tsc on PATH; the head tree's packages/core/node_modules symlinked in as the nested dependency directory — the PR changes no dependency manifests, so both arms share one dependency tree). Harnesses (harness/ab-install.mjs, http-wire.mjs, stdio-wire.mjs, security-matrix.mjs, delta-probes.mjs, mutation-matrix.mjs) import compiled dist output by explicit file URL and run against real filesystem fixtures, real loopback HTTP servers, and a real @modelcontextprotocol/sdk stdio server — no stubs of code under test. Mutations applied one at a time in-tree (exact string replacement), each followed by its pinned vitest target and git checkout -- restore; final git status clean. Gates ran from the package directories with vitest. Raw logs in logs/ (per-arm A/B, per-mutant vitest, gates, base build), six evidence captures in evidence/ (01–05 live re-runs through scripts/verify-capture.mjs; 06 renders the recorded matrix log). Assertion accounting: A/B 20 + delta probes 14 + HTTP wire 6 + stdio wire 7 + security matrix 53 + mutation matrix 7 + vitest gates 960 = 1067 executed, 0 failed.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
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. ✅ Full review at this head in the stage comments — sandboxed verification and maintainer E2E both green; the two stale change-requests on earlier commits are superseded by this approval.
|
Released in v0.21.11. |
3 participants
What this PR does
Adds native Agent Plugins v1 loading to the existing extension lifecycle without converting or rewriting the package. A standard
plugin.jsonpackage can be installed, linked, updated, enabled, disabled, and displayed through the existing extension entry points while preserving its original files.The runtime support intentionally matches Codex's current portable capability boundary: direct-child Agent Skills plus stdio and Streamable HTTP MCP servers are enabled; bundled commands, agents, hooks, apps, channels, context, settings, client namespaces, and legacy SSE MCP are ignored. The implementation also isolates malformed components, persists
PLUGIN_DATAacross updates and reinstalls, contains filesystem access within the plugin root, and applies credential-safe HTTP validation and redirect behavior.Why it's needed
Qwen Code currently requires third-party extension packages to be converted into
qwen-extension.json, so valid Agent Plugins packages fail installation and linking even when they only use portable Skills and MCP capabilities. Native loading lets authors distribute the same standards-based package unchanged while keeping Qwen-specific extension behavior and existing formats unaffected.Reviewer Test Plan
How to verify
Install and link a valid Agent Plugins v1 fixture that contains a direct-child Skill and stdio MCP server. Confirm the extension is identified as AgentPlugins, the Skill and MCP server are available, the MCP server connects with expanded
PLUGIN_ROOTand stablePLUGIN_DATA, and the installed package contains the originalplugin.json,mcp.json,SKILL.md, and server files byte-for-byte with no generatedqwen-extension.json.Install a capability fixture without a version and confirm it defaults to
1.0.0. Confirm only valid direct-child Skills and supported stdio or Streamable HTTP MCP entries load, while nested or malformed Skills, legacy SSE MCP, commands, agents, and hooks remain unavailable. Confirm one invalid Skill or MCP entry does not disable valid siblings.Update and reinstall the same plugin identity, reconnect its MCP server, and confirm the same
PLUGIN_DATAdirectory and prior state are retained. For HTTP MCP, confirm HTTPS and loopback HTTP are accepted, unsafe URLs and client-owned headers are rejected, and configured credentials are not forwarded across redirects.Evidence (Before & After)
Before: the released Qwen Code CLI exits with
Configuration file not found .../qwen-extension.jsonfor both local install and link of an unchanged Agent Plugins v1 package.After: the bundled CLI installs and links the unchanged package, lists
Origin: AgentPlugins, discovers the expected Skill, connects both supported MCP transports, preserves source file hashes, and does not createqwen-extension.json.Tested on
Environment (optional)
macOS with Node.js 22, the bundled CLI, isolated
QWEN_HOMEdirectories, local stdio MCP probes, and a local Streamable HTTP MCP server.Risk & Scope
Linked Issues
N/A
中文说明
本 PR 做了什么
为现有 Extension 生命周期新增 Agent Plugins v1 原生加载能力，不转换也不改写插件包。标准
plugin.json插件现在可以通过现有入口安装、链接、更新、启停和展示，同时保留原始文件。运行时能力边界有意与 Codex 当前的 portable core 对齐：启用直接子目录中的 Agent Skills，以及 stdio 和 Streamable HTTP MCP；忽略 bundled commands、agents、hooks、apps、channels、context、settings、client namespace 和 legacy SSE MCP。实现同时提供组件级错误隔离、跨更新和重装持久化的
PLUGIN_DATA、插件根目录内的文件系统边界，以及避免凭据泄露的 HTTP 校验和重定向策略。为什么需要
Qwen Code 当前要求第三方 Extension 转换为
qwen-extension.json，因此即使标准 Agent Plugins 包只使用可移植的 Skills 和 MCP 能力，安装和链接仍会失败。原生加载允许作者不修改标准包即可分发给 Qwen Code，同时不影响 Qwen 专属 Extension 行为和现有格式。Reviewer Test Plan
如何验证
安装并链接一个包含直接子 Skill 和 stdio MCP server 的有效 Agent Plugins v1 fixture。确认 Extension 来源显示为 AgentPlugins，Skill 和 MCP server 可用，MCP 使用展开后的
PLUGIN_ROOT和稳定的PLUGIN_DATA成功连接；同时确认安装后的plugin.json、mcp.json、SKILL.md和 server 文件与源文件逐字节一致，且没有生成qwen-extension.json。安装一个缺失 version 的 capability fixture，确认版本默认显示为
1.0.0。确认仅加载有效的直接子 Skills 和受支持的 stdio 或 Streamable HTTP MCP entry；嵌套或错误 Skill、legacy SSE MCP、commands、agents 和 hooks 均不可见。确认单个错误 Skill 或 MCP entry 不会禁用有效的同级组件。更新并重新安装同一插件标识，重新连接 MCP server，确认复用相同的
PLUGIN_DATA目录并保留已有状态。对于 HTTP MCP，确认只接受 HTTPS 或 loopback HTTP，拒绝不安全 URL 和 client-owned headers，配置凭据不会经重定向转发。前后证据
之前：已发布的 Qwen Code CLI 对未改动的 Agent Plugins v1 包执行本地安装和链接时，都会以
Configuration file not found .../qwen-extension.json退出。之后：bundle 后的 CLI 可以安装和链接原始插件包，显示
Origin: AgentPlugins，发现预期 Skill，连接两种受支持的 MCP transport，保持源文件 hash 不变，且不会创建qwen-extension.json。测试平台
环境（可选）
macOS、Node.js 22、bundle 后的 CLI、隔离的
QWEN_HOME、本地 stdio MCP probe 和本地 Streamable HTTP MCP server。风险与范围
关联 Issue
N/A