fix(cli): exclude disabled skills from context usage details - #9346
Merged
Conversation
github-actions
Bot
added
the
review/self-reported
Collaborator
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. |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with evidence. #9344 reports a concrete reproduction — a skill disabled before session creation still shows up in Direction: aligned. The detail array and the model-facing available-skills list are built from two divergent paths ( Size: core paths not touched — Approach: the scope feels right. Reusing the existing Risk: no elevated risk signals — no high-risk paths matched; the three consumers of Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有证据。#9344 给出了具体复现——会话创建前被禁用的 Skill 仍在 方向：对齐。明细数组与模型可见 Skill 列表来自两条分叉的路径（ 规模：未触及核心路径—— 方案：范围合理。复用现有 风险：无升级风险信号——未命中高风险路径； 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewThe approach matches what I would have proposed: apply the same disabled-set comparison the runtime already uses, at the point where the detail array is returned, without touching the aggregate accounting. Verified against source:
Testing evidenceUnattended CI run — per triage rules I do not build or execute PR code; the evidence below is the PR's own CI read through the API for commit Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The Linux unit suite is the primary signal — it includes the new focused test above. The author's claim of "14/14 focused context command tests" and of an independent end-to-end probe flipping Sandboxed verification would settle this: 中文说明代码审查方案与我独立设想的修复一致：在明细数组返回处套用运行时已在使用的禁用集合比较语义，不动汇总口径。已对照源码核实：
测试证据无人值守 CI 运行——按规则不构建、不执行 PR 代码；以下为通过 API 读取的该 commit 的 CI 结果。Linux 单元测试套件（含上述新测试）通过是主要信号；作者自述的"14/14 聚焦测试通过"与端到端探针翻转属于作者自报，未在此独立复跑。作者提到的本地全量构建被既有 沙箱验证可收尾： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — a small, correct fix that mirrors the runtime's existing disabled-skill semantics, with a regression test that actually pins the change; the remaining point is CI still in flight, not doubt about the code. Stepping back: this is exactly the kind of fix the gate should wave through. The problem was observed, not hypothesized — a disabled skill reporting 493 tokens of phantom context in a fresh session — and the fix is the minimal version of what I would have written: reuse The two open items are both non-blocking and already named above: whether the detail array should eventually follow the full model-availability predicate ( The Linux unit suite, desktop shell, dependency audit, and secret scan are all green on this commit; the web-shell E2E smoke is still running. Approval is deferred until CI lands green on 中文说明置信度：4/5 —— 小而正确的修复，完全沿用运行时已有的禁用 Skill 比较语义，回归测试也真正钉住了改动；剩余的一点是 CI 尚未全部完成，而非对代码有疑虑。 整体来看：这正是门槛应当放行的修复。问题是观测到的而非假设——禁用的 Skill 在全新会话里报出 493 token 的幻影占用；修复也是我本人会写的最小版本：复用 两个未决项均不阻塞且上文已点名：明细数组将来是否应跟随完整的模型可用性谓词（ Linux 单元测试、桌面 Shell、依赖审计、密钥扫描在该 commit 上均已通过；web-shell E2E 冒烟仍在运行。批准将推迟到 CI 在该 commit 上全绿后由收尾步骤自动完成；若出现红色结果则需重新审视。 — Qwen Code · qwen3.8-max Reviewed at |
ytahdn
approved these changes
Aug 17, 2026
Collaborator
|
Released in v0.21.14. |
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
Filters Skills configured in
skills.disabledout of the detailed context-usageskills[]response. The comparison follows the existing case-insensitive disabled-Skill behavior used by runtime invocation checks.Why it's needed
The runtime already excludes disabled Skills from the model-facing available Skill list, but
/context-usage?detail=trueenumerated the full discovered catalog. This made a disabled, unloaded Skill appear to consume context even in a new session and made the endpoint misleading for context-cost analysis.Reviewer Test Plan
How to verify
Configure Skill discovery to return one enabled Skill and one disabled Skill, using mixed case for the discovered disabled name and lowercase in
skills.disabled. Request detailed context usage and confirm thatskills[]contains only the enabled Skill. The focused context command test suite passes 14/14 tests, and an independent public-seam probe changed fromcontainsDisabled: truebefore the fix tocontainsDisabled: falseafter it.Evidence (Before & After)
Before: a Skill present in
skills.disabledwas still returned inskills[]withloaded: false.After: the disabled Skill is omitted from
skills[]; enabled Skill details remain present.Tested on
Environment (optional)
Local Node.js 22 workspace. Focused Vitest suite and Prettier check passed. The full workspace build was attempted but is currently blocked before reaching this change by the existing Core error
Module 'sharp' has no exported member 'SharpConstructor'.Risk & Scope
breakdown.skillsaccounting or remove historical Skill body/tool-result content from existing chat history.Linked Issues
Fixes #9344
中文说明
本 PR 的改动
从上下文用量详情响应的
skills[]中过滤通过skills.disabled配置关闭的 Skill。比较方式沿用运行时调用校验已有的大小写不敏感语义。为什么需要这个改动
运行时已经会从模型可用 Skill 列表中排除已关闭的 Skill，但
/context-usage?detail=true此前仍会枚举完整的已发现 Skill 目录。这会让一个已关闭且未加载的 Skill 在新会话中看起来仍在消耗上下文，也会误导对上下文成本的分析。Reviewer 测试计划
如何验证
让 Skill 发现结果包含一个开启的 Skill 和一个关闭的 Skill，并让发现结果中的关闭 Skill 名称使用混合大小写、
skills.disabled中使用小写。请求上下文用量详情，确认skills[]只包含开启的 Skill。聚焦的 context command 测试套件 14/14 通过；独立的公开入口探针也从修复前的containsDisabled: true变为修复后的containsDisabled: false。修复前后证据
修复前：
skills.disabled中的 Skill 仍会以loaded: false出现在skills[]中。修复后：关闭的 Skill 会从
skills[]中省略，开启的 Skill 详情仍会保留。测试平台
环境（可选）
本地 Node.js 22 workspace。聚焦 Vitest 测试和 Prettier 检查通过。已尝试全 workspace 构建，但在到达本次改动前被 Core 中已有的
Module 'sharp' has no exported member 'SharpConstructor'错误阻塞。风险与范围
breakdown.skills的汇总口径，也不从现有聊天历史中移除 Skill body 或工具结果内容。关联 Issue
Fixes #9344