feat: manage workspace-scoped extension skill state - #10600
Merged
Conversation
callmeYe
changed the title
callmeYe
marked this pull request as ready for review
August 31, 2026 08:33
callmeYe
requested review from
LaZzyMan,
doudouOUC,
qqqys,
tanzhenxin,
wenshao and
yiliang114
as code owners
August 31, 2026 08:33
Collaborator
Author
E2E verificationVerified the implementation tree committed as Before, the globally installed After, the local
Local test command: Verification limits: captured action/invocation ordering, permission/hook side effects, fault injection and refresh-mode concurrency are covered by focused unit tests, not claimed as E2E coverage. The first E2E attempt incorrectly used the session-reusing |
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
callmeYe
enabled auto-merge
August 31, 2026 08:36
Collaborator
|
Thanks for the PR — gate pass, moving to code review.
Moving on to code review. 🔍 中文说明感谢贡献——门禁通过，进入代码审查。
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "extension_state" |
— Qwen Code · serve A/B
Collaborator
Code reviewRead the full diff at What I verified against the design, in code and tests:
Two nits, neither blocking: the PUT handler uses non-null assertions ( The follow-up commit sequenceDiagram
participant P1 as SDK or REST client
participant P2 as state route
participant P3 as operation queue
participant P4 as ExtensionManager
participant P5 as ExtensionStore
participant P6 as ACP bridge
participant P7 as session agent
P1->>P2: PUT state, skills batch
P2->>P2: structure, trust, generation checks
P2->>P3: enqueue set_extension_state, 202
P3->>P4: commit handler
P4->>P4: ownership check under lock
P4->>P5: single mutation, generation plus 1
P5-->>P4: committed snapshot
P4->>P4: strict skill cache refresh
P3->>P6: skills-only refresh, selected runtime
P6->>P7: per-session refresh, mode kept separate
P7-->>P6: ok or error
P6-->>P3: refreshed and failed counts
P3-->>P1: ordered resourceStates result
Files changed (24 of 53 grouped)
Testing evidence — this PR's own CI (code never executed locally)History on this head: the first commit of the review ( Excerpt from the failing run on the earlier commit, for the record: The main unit suite, Serve A/B, the integration suite and the Java daemon E2E were still running on Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified: the author's macOS E2E with a real model (disabled body absent from tool results, restored after a settings opt-in, restart/update persistence) — author's claim, not independently re-run here; CI runs never execute PR code. Sandboxed verification would settle this: 中文说明代码审查：实现与设计文档高度一致，未发现被遗漏的更简路径。重点均已对照代码与测试核实——优先级链（settings 硬禁用 > 显式开启 > defaultDisabled > workspace 覆盖 > 清单默认 > 开， 测试证据：引用本 PR 自身 CI。首个被审提交在 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Author
|
CI follow-up in The repair only adds four lines across two existing test files. It preserves the full ordered equality assertions; there is no runtime change, new test, ignored capability or relaxed assertion. The original failing integration assertion was reproduced against the real local daemon and then passed with the same product bundle. The complete routes integration file passed 37/37, and the related capability unit tests passed 50/50. Full build, repository typecheck, formatting and scoped lint also passed. This is local verification; the new head's CI still needs to complete. |
Collaborator
|
Confidence: 4/5 — clean, design-faithful implementation with the one CI breakage already fixed mid-review; the two remaining notes are nits, not blockers. Stepping back: this is a design-first PR from the extension area owner, and it delivers exactly what the design promised. I went in looking for a simpler path and didn't find one — settings writes were correctly ruled out because settings must stay authoritative, and the vertical slice (persistence + source-aware execution + targeted refresh + SDK contract) is the minimum that makes the feature real rather than an advertised-but-unenforced API. The code reads like someone who knows this subsystem: it reuses the store's lock/generation machinery instead of inventing storage, keeps the settings-only disabled set unpolluted, and puts execution guards exactly where they must be — after body load, before tool grants and arg-file writes. The ~1245 test lines pin the subtle cases (stale invocations, generation conflicts, symlink workspace aliases, proto-named skills, refresh-mode coalescing), and the docs carry the one user-visible behavior change. In six months this is a thank, not a curse. The one thing that was standing in the way is already resolved: Verdict: approve — and since the unit suite, Serve A/B, integration and Java daemon E2E were still running on 中文说明整体评价：这是扩展领域 owner 的设计先行 PR，实现与设计承诺完全一致。我带着"有没有更简路径"的问题审查，没有找到——写 settings 被正确排除（settings 必须保持最高优先级），而"持久化 + 来源感知执行 + 定向刷新 + SDK 契约"的纵向切片正是让该功能真实生效的最小集合。代码复用了 store 的锁/代次机制而不是另造存储，settings-only 禁用集合保持不被污染，执行防线放在正确的位置（正文加载之后、授权与参数文件写入之前）。约 1245 行测试固定了细微场景（旧调用、代次冲突、符号链接别名、原型链名称、刷新模式合并），文档也如实记录了唯一的用户可见行为变更。 曾经的唯一阻碍已经解决： 结论：批准——由于主单元测试、Serve A/B、集成与 Java daemon E2E 在审查时仍在新提交上运行，批准推迟到该提交 CI 全绿后由 finalize 步骤按提交固定执行；若有检查变红则不批准并标记状态。另建议维护者之后用 — Qwen Code · qwen3.8-max Reviewed at |
Contributor
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-ci-bot
approved these changes
Aug 31, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
yiliang114
approved these changes
Aug 31, 2026
This was referenced Aug 31, 2026
Collaborator
|
Released in v0.23.0. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Implements the workspace-scoped Extension resource-state design previously proposed in this PR. An Extension can declare optional Skill defaults, and each workspace can persist independent internal overrides through a resource-grouped batch API. This implementation supports Skills only and includes the typed REST client.
Settings remain authoritative, a disabled parent Extension cannot be revived by a Skill switch, and source-aware execution checks keep unrelated same-named Skills and commands independent. Changes refresh Skill state, commands and model context across the selected runtime's bootstrap and live sessions, without requesting an unrelated MCP, LSP or hooks restart. Refresh failures remain visible after a successful durable commit.
The existing Skill management entry points continue to write settings. Their enable action now always records an explicit opt-in, including names not yet installed, so it can override an Extension's internal disablement.
Why it's needed
Workspaces can share an installed Extension while needing different Skill combinations. Internal defaults and overrides provide that separation without taking precedence over operator settings or changing the default-on behavior of existing Extensions.
Persistence, source-aware execution, targeted refresh and the client contract must land together: advertising the API without enforcing its state during execution would expose an incomplete feature. The change reuses the existing store, operation queue and runtime lifecycle; it does not introduce another service, storage layer or resource-handler framework.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: the globally installed CLI accepted the fixture Extension but ignored its declared-disabled Skill; the new state capability and routes were absent. That global version predates multi-workspace APIs, so its other missing routes are not presented as regressions in the current source.
After: two complete local-bundle E2E runs passed against an isolated daemon and the repository's deterministic mock model. A mixed batch increased generation once, refreshed two sessions with zero failures and preserved workspace B. Rejected batches preserved store bytes, generation, modification time and inode. Real model tool results excluded the disabled body and included it after an explicit settings enable. Parent disablement and restart persistence also passed.
Full build, repository typecheck and bundle passed. Relevant package-local tests passed: core 1,465; CLI/daemon/ACP 1,601; TypeScript client 391; bridge refresh 6. The final diff received complete self-audit and a local code-review pass. A separate comment contains the E2E evidence and its limits.
Tested on
Environment (optional)
Independent Git worktree, Node.js 22.17.0, isolated configuration/store/workspace directories, two live ACP sessions and a loopback mock model. No real model requests or changes to the user's running services.
Risk & Scope
Linked Issues
None; implementation of the design already proposed in this PR.
中文说明
本 PR 的内容
实现此前在本 PR 中提出的 Extension 内部资源 workspace 状态管理设计。Extension 可以声明可选的 Skill 默认值，每个 workspace 可以通过按资源类型分组的批量接口独立保存内部覆盖。本次只支持 Skill，并提供对应的类型化 REST 客户端。
settings 保持最高优先级，Skill 开关不能复活已关闭的父 Extension；执行时按真实来源判断，避免影响其他来源的同名 Skill 或普通命令。变更会刷新选中 runtime 的 bootstrap 和全部 live session 的 Skill 状态、命令及模型上下文，不请求重启无关的 MCP、LSP 或 hooks。持久化成功后，刷新失败仍会明确报告。
现有 Skill 管理入口继续写 settings。开启操作现在始终记录显式 opt-in，包括尚未安装的名称，使其能够覆盖 Extension 内部关闭。
为什么需要
不同 workspace 可能共享同一个已安装 Extension，但需要不同的 Skill 组合。内部默认值和覆盖提供这种隔离，同时不越过操作者 settings，也不改变旧 Extension 默认全开的行为。
持久化、来源感知执行、定向刷新和客户端契约需要一起交付：如果只暴露接口，却没有在实际执行中落实状态，就是不完整的功能。本次复用现有 store、操作队列和 runtime 生命周期，不新增管理服务、存储层或通用资源处理框架。
评审验证计划
如何验证
验证证据（前后对比）
之前：全局 CLI 可以安装 fixture Extension，但忽略了声明关闭的 Skill；新状态能力和路由不存在。该全局版本早于多 workspace API，因此不把它缺少其他路由当作当前源码的回归。
之后：本地 bundle 使用隔离 daemon 和仓库现有确定性 mock 模型完成两次完整 E2E，均通过。混合批次只增加一次 generation，两个会话刷新成功且零失败，workspace B 保持不变。拒绝批次后，store 内容、generation、修改时间和 inode 均不变。真实模型工具结果在关闭时不包含 Skill 正文，显式 settings 开启后包含正文。父级关闭优先和重启持久化也通过。
完整 build、仓库 typecheck 和 bundle 通过。相关 package-local 测试通过：core 1,465 项；CLI/daemon/ACP 1,601 项；TypeScript 客户端 391 项；bridge 刷新 6 项。最终差异完成完整自审及本地代码复核。E2E 证据和验证边界会单独附在评论中。
验证环境
运行环境
独立 Git worktree，Node.js 22.17.0，隔离的配置/store/workspace 目录，两个 live ACP session 和本机 mock 模型。没有请求真实模型，也没有改动用户正在运行的服务。
风险与范围
关联 Issue
无；实现本 PR 已有设计。