fix(core): sync loaded-skill state with history eviction; add user /unskill command - #8900
fix(core): sync loaded-skill state with history eviction; add user /unskill command#8900ZijianZhang989 wants to merge 24 commits into
Conversation
…nskill command History rewrites (pre-send microcompaction, /compress-fast, memory-pressure compact_history, LLM /compress) blanked skill bodies without updating the loaded-skill set. The dedup guard then answered every re-invocation with 'already loaded in context', leaving the skill permanently unusable while /context kept reporting it active. Microcompaction now reports blanked skill names in its meta; all four rewrite paths consume it to sync the tracking (targeted unload, wholesale clear when a name cannot be resolved); and a new user-only /unskill <name> command replaces a loaded skill's body with a placeholder, adjusts the token estimate downward, and re-arms the dedup guard so the next invocation reloads the full body. Closes #6762
|
|
E2E 测试报告验证环境：macOS · 本分支构建产物（ V1 驱逐同步（/compress-fast 路径）— PASS
V2 /unskill 命令 — PASS
V3 最终 bundle 冒烟 — PASS在补齐全部测试缺口 + 命令文件 kebab-case 重命名后的最终 bundle 上复验：active +103 → 验证 gotcha（供复测参考）
单测与质量门
|
|
Thanks for the PR! Template looks good ✓ Problem: observed bug, confirmed in the current code. Direction: aligned — this is squarely the context-lifecycle gap the roadmap item describes. Claude Code's CHANGELOG has no direct analogue of an unload command, but skills are an active area there too (fork/background skills, model-invocation controls), and the eviction-sync half is a correctness fix regardless of direction debates. Size: touches core paths. Production logic ≈393 lines vs tests ≈713 lines — under the 500-line maintainer-awareness threshold, no large-PR advisory. Approach: the scope feels right. The eviction-sync half mirrors the existing issue-#4239 pattern ( Risk: Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题： 已观测到的 bug，且在当前代码中确认存在。 方向： 对齐——这正是 roadmap 条目所描述的上下文生命周期缺口。Claude Code 的 CHANGELOG 没有卸载命令的直接对应物，但 skills 在那边也是活跃领域（fork/background skills、模型调用控制），且驱逐同步这一半本身就是正确性修复。 规模： 触及核心路径。生产逻辑约 393 行，测试约 713 行——低于 500 行维护者关注阈值，也不触发大 PR 建议。 方案： 范围合理。驱逐同步一半沿用了 issue #4239 的既有模式（ 风险： 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI formed an independent proposal before reading the diff — report evicted skill names via the request-side What I verified against the code:
Findings — none blocking. One follow-up worth filing:
The eviction-sync flowsequenceDiagram
participant P1 as User
participant P2 as History rewrite path
participant P3 as MicrocompactMeta
participant P4 as syncSkillEvictions
participant P5 as SkillTool tracking
P1->>P2: sends a message or runs a compress command
P2->>P2: blanks old skill bodies from history
P2->>P3: evictedSkillNames plus unresolvedEvictedSkills
P3->>P4: consumed at all four rewrite sites
alt every eviction resolved
P4->>P5: unloadSkills with the named skills
else any eviction unresolved
P4->>P5: clearLoadedSkills blanket clear
end
P5-->>P1: dedup guard re-arms, next invoke reloads the full body
Files changed (15)
Testing evidenceThis is an unattended CI run — the PR's code is never executed here. Evidence below is the PR's own CI, fetched once via the API (no polling; the Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle this: Not verified: end-to-end TUI behavior on Linux/Windows — no independent live run in this pass (unattended CI). 中文说明代码审查：先独立构思方案再对照 diff——本 PR 的做法与我独立提出的方案基本一致：通过请求侧 functionCall 参数恢复被清空的 skill 名（与 issue #4239 的文件读取机制同一模式）、在每个历史改写点取消跟踪、无法归因时整体清空。没有遗漏更简单的路径。 已核对：三个 无阻塞项。一个后续建议： 测试证据：本次为无人值守 CI 运行，不执行 PR 代码。以上证据来自 PR 自身 CI（API 一次性抓取，不轮询；CI 结束后由 finalize 任务更新表格）。抓取时无红色检查；ubuntu 单测套件（本改动的权威验证）仍在运行，macOS/Windows 单测作业被仓库 CI 画像分类跳过（非失败）。行为层面的最终确认建议由维护者触发 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — solid, well-scoped fix that matches the idiomatic pattern in this codebase; only non-blocking nits (an uncovered Stepping back: this PR earns its merge. The ghost-skill state is a real dead end I confirmed in the current code — once compaction blanks a body, the skill is unrecoverable for the rest of the session — and the fix extends the exact mechanism this repo already built for the analogous file-read problem, which is the right instinct. The reservations, named plainly: Approval deferred until CI lands green on 中文说明信心度：4/5 —— 扎实、范围得当的修复，与本代码库的惯用模式一致；仅有非阻塞的小问题（ 总体评价：这个 PR 配得上合并。幽灵 skill 状态是我在当前代码中确认过的真实死路——压缩清空正文后，该 skill 在会话余下时间里无法恢复——而修复方案恰好扩展了本仓库为同类文件读取问题已建立的机制，这是正确的直觉。 坦率地说保留意见有两点： 批准推迟到 CI 在被审提交上全绿——审查时 ubuntu 单测仍在运行，不会为尚不存在的结果背书。CI 全绿后由 finalize 任务发布绑定该提交的批准。 — 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-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 explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — finished within budget; no check was left unfinished..
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — finished within budget; no check was left unfinished.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— deepseek-v4-flash via Qwen Code /review (v0.21.8)
- microcompaction: suppress a skill's eviction report when a kept result still holds its reloaded body (a stale dedup confirmation from an earlier load cycle no longer un-tracks a resident skill); treat the /unskill placeholder as already-cleared so it neither absorbs a keepRecent protection slot nor gets re-blanked - hooks: dedup registerSkillHooks so unload/reload cycles don't stack duplicate session hooks - /unskill: reject names that are not real skills (the skill tool's command-executor fallback also tracks command names); fall back to locating the body in history when in-memory tracking was lost (--resume); filter completion candidates to real skills; fix import ordering in BuiltinCommandLoader - i18n: add the command's strings to the en baseline and all 8 locale files, fixing the strict-parity mustTranslateKeys CI failure - tests: BuiltinCommandLoader registration test plus review-driven cases for all of the above
Review round 2 — all findings addressed in
|
The dedup compares stored SessionHookEntry.config values, whose type includes function/prompt hooks; keying those by a best-effort JSON blob (they never originate from skill frontmatter, so they simply never match).
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 explored to full depth (tool budget reached): chunk 1: did not run BuiltinCommandLoader.test.ts (worktree has no node_modules; install was out of budget) — test verified by inspection only..
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 1：did not run BuiltinCommandLoader.test.ts (worktree has no node_modules; install was out of budget) — test verified by inspection only.。
— qwen3.8-max via Qwen Code /review (v0.21.9)
R2-1 (Critical): buildKeptSkillNames counted SkillTool error outputs
("Skill x not found."/"is disabled."/"Failed to load") as residency proof,
suppressing eviction and leaving the skill permanently unreloadable behind
the dedup guard. Switch to a positive body check (the buildSkillLlmContent
"Base directory for this skill:" prefix) so only a real body proves
residency; errors/confirmations/placeholders/cleared messages do not.
R2-3: an ambiguous call-id (one id mapped to multiple skill names) now
protects NONE, matching buildKeptFilePaths' length!==1 guard.
R2-9: filter on the clear-set (not the keepRecent set) so a body that
survives the size path via the low-watermark early break is no longer
over-un-tracked (token doubling).
R2-10: reword the evictedSkillNames field doc, which still claimed "No
kept-suppression" — flatly contradicting buildKeptSkillNames above it.
R2-6: hasSkillBodyInHistory now returns true only for a body OR a dedup
confirmation, excluding SkillTool error text (so /unskill after --resume
no longer claims a body exists for a failed/disabled load).
R2-5: the cached-skills gate no longer rejects a skill deleted/renamed
mid-session (body still in history) — it falls through to the resume
fallback instead of mislabeling it as a command.
R2-15 (1/3): wire clearLoadedSkillTracking into truncateHistory so /rewind
past a skill load no longer leaves the dedup guard blocking every reload.
R2-8: addSessionAllowRule deduplicates on raw, mirroring addPersistentRule
and the dangerous-stash branch, so reload cycles stop accumulating the
skill's allowedTools list.
Tests: hasSkillBodyInHistory unit coverage (R2-11); kept-suppression cases
for error-output (R2-1) and ambiguous call-id (R2-3).
Deferred to a follow-up issue: R2-2/R2-7 (hook dedup depth +
unregisterSkillHooks wiring), R2-14 (false warn on fully-deduped reload),
R2-4 (completion after --resume), R2-12/R2-13 (error/HTTP branch tests),
R2-15 2/3+3/3 (/restore + ACP restoreHistory + startNewSession).
|
已提交修复 已修复（已标记 resolve）
延后到 follow-up issue（未标 resolve，已在各线程逐条回复）
全部 typecheck + 单测通过（microcompact 86、geminiChat 328、unskill-command 10、registerSkillHooks 9、permission-manager 332、skill 82、client truncateHistory 4）。 |
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 explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; chunk 8: did not execute npx vitest run src/tools/skill.test.ts — the review worktree has no node_modules (checked worktree root and packages/core ), so running tes…; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all planned checks completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks completed within budget., and 3 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；chunk 8：did not execute npx vitest run src/tools/skill.test.ts — the review worktree has no node_modules (checked worktree root and packages/core ), so running tes…；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all planned checks completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks completed within budget.，另有 3 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.9)
…arity R3-7 (Critical): syncSkillEvictions was called AFTER disarmFileReadCacheAfterEviction; if disarm threw, sync was skipped while setHistory had already committed, leaving the skill tracking in a ghost state that blocked reloads. Swap the two lines so sync runs first (it is synchronous and internally guarded), then disarm (failure only degrades the file-read cache). Fixed in both microcompaction and compress-fast call sites. R3-14 (Critical): stripOrphanedUserEntriesFromHistory (failed-then-retry path) stripped user turns containing skill bodies but only cleared the FileReadCache, not the loaded-skill tracking — same ghost deadlock as R3-7. Added clearLoadedSkillTracking after the cache clear. R3-2: unloadSkillBody matched a body to the wrong skill when a call-id was shared by multiple skill invocations. Now refuses to clear when the mapping is ambiguous (length !== 1). R3-16: unloadSkillBody treated SkillTool error text (e.g. "Skill x not found.") as a clearable body. Now guarded by isSkillBodyOutput, so only a real body is cleared. R3-3: The eviction-record block in microcompaction counted any SkillTool part with a non-cleared response as a kept body, including error outputs. Added isSkillBodyOutput guard so only real bodies suppress the eviction report. R3-12: Catalan locale used "La habilitat" (phonetically wrong for the elision). Fixed to "L'habilitat" in 3 places. R3-10: mustTranslateKeys.ts was missing the 8 unskill-command i18n keys, so the strict-parity check would not catch missing translations in fork locales. R3-4/R3-5/R3-6: Added tests for truncateHistory tracking clear (2), addSessionAllowRule dedup (1), and mid-session deleted skill bypass (1).
R3-3's isSkillBodyOutput guard requires the buildSkillLlmContent prefix to recognize a skill body. Two test fixtures (client.test.ts and memoryPressureMonitor.test.ts) used a bare 'skill body '.repeat(50) string without the prefix, causing the guard to skip the eviction record and the sync to never un-track the skill — failing the assertion.
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: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5; round-5 findings left unverified (compose-floor budget stop).
Not explored to full depth (tool budget reached): PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...: none — all checks above completed within budget.; PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...: none — all planned checks completed within budget.; chunk 8: could not execute packages/core vitest ( skill-utils.test.ts , skill.test.ts ) — the review worktree has no installed node_modules and vitest fails to load i…; chunk 1: running packages/cli vitest suites for BuiltinCommandLoader.test.ts and mustTranslateKeys.test.ts (dependencies not installed).; PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...: (none — all planned checks completed within budget)., and 3 more.
— [unverified] tag when the loop ended — the verifier never ruled on them, and they are not confirmed.
中文说明
已审查。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：reverse audit — did not converge within the reverse-audit round cap of 5; round-5 findings left unverified (compose-floor budget stop)。
未探索到全部深度（达到工具调用预算）：PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...：none — all checks above completed within budget.；PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...：none — all planned checks completed within budget.；chunk 8：could not execute packages/core vitest ( skill-utils.test.ts , skill.test.ts ) — the review worktree has no installed node_modules and vitest fails to load i…；chunk 1：running packages/cli vitest suites for BuiltinCommandLoader.test.ts and mustTranslateKeys.test.ts (dependencies not installed).；PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...：(none — all planned checks completed within budget).，另有 3 条。
— [unverified] 标记——验证者从未对它们作出裁决，它们不算已确认。
— qwen3.8-max via Qwen Code /review (v0.21.9)
Replace negative-enumeration skip (error key / placeholder / cleared message) with positive check: isSkillBodyOutput || isSkillDedupConfirmation. SkillTool error texts (plain response.output with no error key) are now naturally skipped instead of being rewritten into success-shaped placeholders. Remove unused MICROCOMPACT_CLEARED_MESSAGE import. Test fixture updated to use buildSkillLlmContent prefix so the body passes the positive check.
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. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8900 syncs loaded-skill tracking with history rewrite...: none — all checks I planned completed within budget.; PR #8900 syncs loaded-skill tracking with history rewrite...: none — all checks above were completed within the tool budget.; PR #8900 syncs loaded-skill tracking with history rewrite...: did not trace the final top-level rendering of the unhandled rejection in the interactive path beyond confirming slashCommandProcessor.ts has no try/catch aro…; PR #8900 syncs loaded-skill tracking with history rewrite...: none — all checks above completed within budget..
中文说明
已审查。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8900 syncs loaded-skill tracking with history rewrite...：none — all checks I planned completed within budget.；PR #8900 syncs loaded-skill tracking with history rewrite...：none — all checks above were completed within the tool budget.；PR #8900 syncs loaded-skill tracking with history rewrite...：did not trace the final top-level rendering of the unhandled rejection in the interactive path beyond confirming slashCommandProcessor.ts has no try/catch aro…；PR #8900 syncs loaded-skill tracking with history rewrite...：none — all checks above completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.10)
Move clearLoadedSkillTracking calls from GeminiClient wrappers down into GeminiChat.truncateHistory and GeminiChat.stripOrphanedUserEntriesFromHistory so both TUI and ACP paths are covered (ACP sessions call GeminiChat methods directly, bypassing client-level wrappers). Remove now-redundant client-level calls and unused import. Remove two client-level tests that verified the clear at the wrong layer.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查。 2 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
— deepseek-v4-flash via Qwen Code /review (v0.21.10)
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: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...: none — all checks above completed within budget.; PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...: none — all checks above completed within budget.; PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...: none — all planned checks completed within budget (~24 tool calls).; PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...: 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。
未探索到全部深度（达到工具调用预算）：PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...：none — all checks above completed within budget.；PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...：none — all checks above completed within budget.；PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...：none — all planned checks completed within budget (~24 tool calls).；PR #8900 (QwenLM/qwen-code): syncs loaded-skill tracking ...：none — all planned checks completed within budget.。
— qwen3.8-max via Qwen Code /review (v0.21.10)
Resolve the BuiltinCommandLoader.test.ts additive conflict by keeping both the /unskill and /advisor registration tests; adopt main's Session.ts and all other main updates alongside this PR's changes.
# Conflicts: # docs/developers/daemon/02-serve-runtime.md # docs/developers/daemon/12-auth-security.md # docs/developers/daemon/18-error-taxonomy.md # docs/users/qwen-serve.md # packages/cli/src/serve/fast-path.test.ts # packages/cli/src/serve/live/live-session-coordinator.test.ts # packages/cli/src/services/BuiltinCommandLoader.test.ts # packages/web-shell/README.md
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — full chunk read (diff lines 1779-2168, untruncated) and all dependency reads completed within budget..
Deferred under the convergence posture (round 14, not a blocker) — recorded, not requested in this round:
packages/core/src/core/geminiChat.ts:4556 (+2 locations) — [review] two comments state the tracked/un-tracked failure direction backwards (twin at skill-utils.ts:545)packages/cli/src/services/BuiltinCommandLoader.ts:175 — [review] /unskill is registered but missing from docs/users/features/commands.md
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none — full chunk read (diff lines 1779-2168, untruncated) and all dependency reads completed within budget.。
收敛姿态下延后（第 14 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
@qwen-code /verify |
|
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: 2168 passed · 0 failed · 2168 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2168 通过 · 0 失败 · 2168 总计 Verification reportPR #8900 Deep Verification — sync loaded-skill state with history eviction; add
|
| Cell | Build | Oracle (scripted) | Result |
|---|---|---|---|
| C1 ghost | base 5c56b671 |
post-eviction tracking + re-invoke output | ghost reproduced: still tracked; re-invoke → Skill "demo-poem" is already loaded in context. |
| C1 fix | head de794a69 |
meta.evictedSkillNames, tracking, re-invoke output |
evictedSkillNames=['demo-poem'], unresolved=0; un-tracked; re-invoke → full body; re-tracked |
| C2 kept-suppression | both | eviction report + tracking when a newer body survives | head: report suppressed (evicted=[]), skill stays tracked (no over-clear); base: stays tracked (never un-tracks) |
| C3 unresolved | both | tracking after an unresolvable blanked body | head: unresolvedEvictedSkills=1 → wholesale clear (tracked=[]); base: no consumer exists, ghost persists |
Counts: base 13/13, head 18/18 (logs-ab-base.txt, logs-ab-head.txt). Witnesses: 01-ab-base-ghost-reproduces.png, 02-ab-head-ghost-fixed.png.
/unskill core — wire-oracle harness
unskill-harness.mjs invokes the real compiled GeminiChat.prototype.unloadSkillBody / hasSkillBodyInHistory on a minimally constructed instance (the fields those methods read; setHistory as plain assignment — the partial-push reset it additionally performs is irrelevant to freshly built histories) plus real microcompactHistory for the placeholder interaction. 30/30 passed (logs-unskill-head.txt, witness 03-unskill-head-matrix.png):
- body + dedup confirmations → placeholder,
tokensSaved>0,lastPromptTokenCountadjusted and marked estimated; - unknown name →
{cleared:false, tokensSaved:0}, history byte-identical; - ambiguous call-id (one id, two skill names) → refuse to clear;
- SkillTool error text is not a clearable body; prefix-only spoof string is not a body (
isSkillBodyOutputboundary); hasSkillBodyInHistorytruth table: body/confirmation → true; placeholder/cleared/error/other-name → false;- a
/unskillplaceholder is never re-blanked by microcompaction, absorbs no keepRecent slot, and the co-resident real body is still blanked and reported.
The base arm has no counterpart API (unloadSkillBody absent — harness records and exits); this is a new feature, so no base cell applies. The shipped bundle chunks contain the command and its i18n strings (Unloaded skill present in dist/chunks/*), confirming the accept path lands in the built artifact.
Vacuity / mutation matrix (positive controls landed)
Unmutated control: all gate suites green (below). Each mutation was applied to the source, run, and restored (git status clean afterwards):
| Mutation | Suite | Result |
|---|---|---|
M1: disable the eviction-report block (false &&) in microcompact.ts |
microcompact.test.ts |
Killed — 5 new tests fail with behavioral mismatches (e.g. expected [] to deeply equal [ 'demo-poem' ], expected +0 to be 1). The 4 remaining green tests in the block assert empty reports, which the mutant legitimately produces — explained greens, not survivors. |
M2: drop token adjustment in unloadSkillBody (geminiChat.ts) |
geminiChat.test.ts |
Killed — exactly the intended test fails on the intended assertion: expected 1000 to be less than 1000. |
M3: delete the syncSkillEvictions call on the pre-send microcompaction path (client.ts) |
client.test.ts |
Killed — exactly the intended test fails: expected "spy" to be called with arguments: [ [ 'demo-poem' ] ]. |
All three kills are quoted failure messages naming expected-vs-actual values — the reverts hit the behavioral assertions, not imports or fixtures. No surviving mutants.
Targeted gates
| Gate | Result |
|---|---|
| core: microcompact, skill-utils, skill, registerSkillHooks, permission-manager, memoryPressureMonitor, forkedAgent.cache tests | 618/618 |
| core: geminiChat + client tests | 681/681 |
| cli: unskill-command, BuiltinCommandLoader, acp Session, fast-path, live-session-coordinator tests | 774/774 |
| cli: i18n mustTranslateKeys + index tests | 31/31 |
tsc --noEmit core + cli |
clean |
| ESLint on the 11 changed production files | clean (gate proven live: a planted no-unused-vars violation was reported, then removed) |
Corrections
None.
Findings
None. No behavioral defects found on the changed surface; the PR's documented fallback directions (over-clear on unresolved names, ambiguous-id refusal, kept-body suppression) all held under probing, including the sibling shapes (dedup confirmation, error text, placeholder, cleared message, ambiguous/orphan call ids). No injection-style steering instructions were present in the PR text.
Not covered
- Interactive TUI/tmux run with a real model (Reviewer Test Plan steps as a user flow, incl.
/context detailrendering): requires live model calls; the same mechanism is proven here at the compiled-module level through the real SkillManager/SkillTool/microcompactHistory objects instead. This reproduces the handling (eviction → sync → reload), not the model-driven trigger shape end-to-end. - Per-commit attribution: metadata lists 19 commits; the depth-2 checkout makes only the merge + parents reachable (
git rev-list HEAD^1..HEAD^2returns 1 at the shallow boundary). The aggregateHEAD^1..HEADdiff was verified. - Repo-wide vitest suites — only the affected files were run (PR CI covers the rest).
- ACP live sessions and
--resumeruntime paths — covered by unit tests (Session.test.ts) and the U6 truth-table harness, not exercised live. - Windows / macOS behavior.
- Web-shell README changes (docs-only).
- One harness-side artifact, not a PR behavior: the minimal base rebuild (
tsconly) does not copy bundled-skill assets, so the base arm's SkillManager listed onlydemo-poemwhile head also listed bundled skills. Skill discovery is untouched by this PR (not in the diff); all cells assert ondemo-poemonly.
Methodology
Single container (node:22-bookworm lane runtime), working tree at merge ref 016ccc1b with npm ci + npm run build pre-run. A/B: base tree via git worktree add tmp/base-tree HEAD^1, rebuilt packages/core only with the root toolchain (lockfile and all package.jsons unchanged by the PR — verified with git diff --stat — so reusing root node_modules is a clean control; base dist realpath asserted inside the base tree; the only shim is a symlink of packages/core/node_modules for external deps like ajv v8, which resolve identically on both arms). Harnesses import the compiled dist/src/** ESM directly by absolute path and drive real objects end to end (no stubbing of code under test; the Config service locator is duck-typed exactly as in the repo's own unit tests). Each harness prints [PASS]/[FAIL] per scripted check and a JSON summary; raw outputs are logs-ab-head.txt, logs-ab-base.txt, logs-unskill-head.txt. Mutations were source-level, interface-preserving (false && guards / deleted call lines), run through the same vitest suites, and restored with git checkout (final git status clean). Evidence images were produced with scripts/verify-capture.mjs from live re-runs of the harnesses.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
…olvable Address review round 14: a stripped skill body whose call id cannot be resolved to a name now falls back to clearing all loaded-skill tracking in unloadSkillsFromEntries, matching syncSkillEvictions' policy — the body is gone, and leaving its skill tracked makes it unreloadable. The docstring clause claiming unresolvable results stay tracked is corrected.
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
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: reverse audit — stopped after the round 1+2 convergence pair: an external process sharing this runner deleted the review worktree twice mid-loop; rounds 3-5 never ran.
Not reviewed: reverse audit round 2 chunks 8, 11, 12 — auditors cut short by the external worktree deletion; round-1 receipts for these chunks stand.
Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at /home/github-runner/actions-runner-7/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-7--work-qwen-code-qwen-code/subagents/0b8433a1-fec3-45d8-8333-07551e6dd2b7 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-7/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-7--work-qwen-code-qwen-code/subagents/0b8433a1-fec3-45d8-8333-07551e6dd2b7'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.
Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at /home/github-runner/actions-runner-7/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-7--work-qwen-code-qwen-code/subagents/0b8433a1-fec3-45d8-8333-07551e6dd2b7 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-7/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-7--work-qwen-code-qwen-code/subagents/0b8433a1-fec3-45d8-8333-07551e6dd2b7'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).
Deferred under the convergence posture (round 15, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/commands/unskill-command.ts:141 — [review] !cleared branch conflates the ambiguous-call-id refusal with 'no body left' — false message and un-track while the body is residentpackages/core/src/core/client.ts:683 — [review] New comment misdescribes the unresolvable-body policy; twin comment in geminiChat.ts carries the same stale claimpackages/core/src/hooks/registerSkillHooks.ts:126 — [review] hookConfigKey ignores behavior-affecting fields — same-command hooks differing in shell/timeout/headers are silently deduped awaypackages/core/src/tools/skill-utils.ts:558 — [review] The R14-2 blanket-clear branch of unloadSkillsFromEntries has zero test coveragepackages/cli/src/acp-integration/session/Session.ts:4305 — [review] ACP settle gate keys on resolved names — an all-unresolvable strip skips the reconcile after a blanket clearpackages/core/src/core/geminiChat.ts:4493 — [review] truncateHistory reconcile is the only unguarded tracking-mutation site — inconsistent with the isForkedChat invariant this PR establishes
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：reverse audit — stopped after the round 1+2 convergence pair: an external process sharing this runner deleted the review worktree twice mid-loop; rounds 3-5 never ran。
未审查：reverse audit round 2 chunks 8, 11, 12 — auditors cut short by the external worktree deletion; round-1 receipts for these chunks stand。
未审查：覆盖情况——无法读取 agent 的运行记录（no subagent transcripts at /home/github-runner/actions-runner-7/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-7--work-qwen-code-qwen-code/subagents/0b8433a1-fec3-45d8-8333-07551e6dd2b7 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-7/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-7--work-qwen-code-qwen-code/subagents/0b8433a1-fec3-45d8-8333-07551e6dd2b7'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.），本次运行无法证明 diff 的任何部分被读过。
未审查：验证——无法检查步骤 4 与步骤 5 是否运行（no subagent transcripts at /home/github-runner/actions-runner-7/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-7--work-qwen-code-qwen-code/subagents/0b8433a1-fec3-45d8-8333-07551e6dd2b7 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-7/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-7--work-qwen-code-qwen-code/subagents/0b8433a1-fec3-45d8-8333-07551e6dd2b7'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.）。
收敛姿态下延后（第 15 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
Address review round 15: - unloadSkillsFromEntries blanket-clears tracking whenever ANY stripped body fails to resolve (missing or ambiguous call id), including the mixed shape where other bodies in the batch resolve — previously only the fully-unresolvable case was covered, leaving a half-blind ghost. - /unskill gains id-less-provider awareness: hasIdLessSkillResult() probes history for Skill bodies/dedup confirmations without a call id; unloadSkillBody returns a distinct unresolvable result and the command keeps tracking armed (dedup guard intact) instead of falsely reporting the body absent and enabling a duplicate injection. hasSkillBodyInHistory sees the same shape so --resume no longer answers "not loaded". New refusal message in all 9 locales.
Seven files unrelated to this PR (daemon/serve docs, serve tests, web-shell README) drifted into the branch tree as pure prettier formatting noise during an earlier conflicted merge resolution. Restore them to their merge-base versions so the PR diff contains only the skill-tracking work. Committed with --no-verify because the pre-commit prettier task rewrites the merge-base content back to the formatted variant, which would cancel the revert.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 16, not a blocker) — recorded, not requested in this round:
packages/core/src/core/client.ts:684 — [review] stale comments at client.ts:683 and geminiChat.ts:4593 contradict the blanket-clear-on-any-unresolvable policy the fix commits introducedpackages/core/src/hooks/registerSkillHooks.ts:124 — [review] hookConfigKey ignores behavior-affecting fields — same-command hooks differing in shell/timeout/headers are silently deduped awaypackages/core/src/tools/skill-utils.ts:577 — [review] unloadSkillsFromEntries' two defensive branches (unresolvable blanket clear, resident-name filter) have zero test coverage; deleting either ships greenpackages/cli/src/acp-integration/session/Session.ts:4985 — [review] ACP settle gate keys on resolved names only — an all-unresolvable strip skips the reconcile after a blanket clearpackages/core/src/core/geminiChat.ts:4493 — [review] truncateHistory/strip tracking mutations bypass the isForkedChat guard the PR establishes at its other sites — latentpackages/cli/src/ui/commands/unskill-command.ts:136 — [review] the R15-2 id-less refusal path (hasIdLessSkillResult, unresolvable return, refusal branch) has zero test coverage at both layerspackages/cli/src/ui/commands/unskill-command.ts:26 — [review] Skill-tool lookup uses getAllTools().find() where a direct getTool() exists, on a completion-hot pathpackages/cli/src/i18n/mustTranslateKeys.ts:36 — [review] the new unresolvable-refusal message is the only /unskill string omitted from MUST_TRANSLATE_KEYSpackages/cli/src/ui/commands/unskill-command.ts:101 — [probe] gate-1 'is this a skill' classification is defeated by the name-independent id-less fallback — any resident id-less body vouches for any queried namepackages/core/src/services/microcompaction/microcompact.ts:816 — [probe] eviction-recording gate misses command-executor-fallback results that created tracking — blanking them leaks the tracked name
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 16 轮，非阻断）——已记录，本轮不要求修改：共 10 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 2: could not get a green in-process run of mustTranslateKeys.test.ts / BuiltinCommandLoader.test.ts (blocked by the pre-existing channel-gitlab vite resolution….
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 16, not a blocker) — recorded, not requested in this round:
packages/core/src/core/client.ts:684 — [review] new comments claim unresolvable entries 'stay tracked'; the implementation blanket-clears (twin comment in geminiChat.ts:4592)packages/core/src/services/microcompaction/microcompact.test.ts:2088 — [review] size-path (sizeOnly) eviction meta has zero test coverage — the hot path, modified by this diffpackages/cli/src/ui/commands/unskill-command.ts:133 — [review] id-less unresolvable refusal path untested on both sides (chat helper + command branch)packages/core/src/core/geminiChat.ts:2227 — [review] ambiguous-call-id refusal branch untested; the same {cleared:false} shape is pinned by another test as un-trackpackages/cli/src/acp-integration/session/Session.ts:4307 — [review] ACP settle tests mock the resolver argument-blind; no negative case for an empty strippackages/core/src/tools/skill-utils.ts:577 — [probe] unloadSkillsFromEntries blanket-clear and resident-filter branches have zero tests (probe: branches live, mutants survive)packages/cli/src/ui/commands/unskill-command.ts:77 — [probe] pre-action guards (isInitialized / config-not-loaded) untestedpackages/core/src/core/geminiChat.ts:4529 — [review] isForkedChat guard missing on truncateHistory and stripOrphanedUserEntriesFromHistory — 3 of 5 sites enforcedpackages/core/src/core/geminiChat.test.ts:4523 — [review] hard-rescue restore and reconcile-wrapper fork gates have no forked-chat testspackages/cli/src/i18n/mustTranslateKeys.ts:32 — [review] MUST_TRANSLATE_KEYS omits the /unskill 'could not be unloaded safely' refusal stringpackages/cli/src/acp-integration/session/Session.ts:4985 — [review] ACP continuation strip-to-settle window leaves a resident skill untracked mid-turn (duplicate body on same-turn re-invoke)packages/cli/src/ui/commands/unskill-command.ts:185 — [review] tab completion blind to --resume restored bodies and mid-session-deleted skills the action handlespackages/core/src/core/client.ts:2433 — [probe] TUI retry re-push window leaves a resident skill untracked mid-turn (probe-verified)packages/core/src/services/microcompaction/microcompact.test.ts:2221 — [probe] kept-body suppression branch mutation-survives: filter removed, suite still 86/86 greenpackages/cli/src/ui/commands/unskill-command.ts:107 — [probe] id-less wildcard makes /unskill <unknown-name> print a factually false safety refusalpackages/core/src/core/geminiChat.test.ts:541 — [probe] fork test pins the tracker-clear skip but not that compression still applies to the forkpackages/core/src/core/geminiChat.ts:4596 — [review] blanket-clear residue unreconciled on the TUI cancel-strip sites (AppContainer.tsx:2844/:2990)packages/core/src/services/microcompaction/microcompact.test.ts:2088 — [probe] new eviction test block lacks clearEnv around QWEN_MC_KEEP_RECENT — probe: 8/9 spuriously red with the env set
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 2：could not get a green in-process run of mustTranslateKeys.test.ts / BuiltinCommandLoader.test.ts (blocked by the pre-existing channel-gitlab vite resolution…。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 16 轮，非阻断）——已记录，本轮不要求修改：共 18 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
The R15 id-less refusal was gated behind targetIds.size === 0, so mixed histories (resolvable entries plus an unattributable body), synthesized response ids whose model call has none, orphan responses, and ambiguous-id refusals all disarmed the dedup guard while a body stayed resident. Hoist the probe above the gate, widen it to 'no id or id not in the call-id map', and return unresolvable from the ambiguous refusal so tracking is kept in every entrance.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
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 — packages/sdk-typescript suite timed out in the scoped run; packages/vscode-ide-companion, packages/web-shell, packages/webui suites did not run (whole-call budget) — the diff touches none of these packages.
Not explored to full depth (tool budget reached): chunk 3: running the two new client.test.ts tests (worktree has no node_modules/dist; npm install + build exceeds the remaining tool budget) — all assertions were verifi…; chunk 11: execute forkedAgent.cache.test.ts to confirm the new test passes — the worktree has no node_modules installed ( vitest unresolvable at startup) and a full mono…; chunk 5: running geminiChat.test.ts under vitest — worktree has no node_modules / dist , and npm install + npm run build exceeds the tool budget; verification was….
Deferred under the convergence posture (round 17, not a blocker) — recorded, not requested in this round:
packages/core/src/core/client.ts:683 (+2 locations) — [review] new comment pair claims unresolvable stripped bodies 'stay tracked'; the implementation blanket-clears (twin comment in geminiChat.ts:4609)packages/cli/src/acp-integration/session/Session.ts:4985 — [review] ACP settle-reconcile gate keys on resolved names — an all-unresolvable strip skips the reconcile after a blanket clearpackages/cli/src/acp-integration/session/Session.ts:4325 — [review] ACP Retry strip never feeds the settle reconcile — the TUI retry twin repairs this case, ACP does notpackages/cli/src/ui/commands/unskill-command.ts:133 — [probe] the unresolvable refusal path broadened by this round's commit has zero tests at both layers (probe: both mutations ship green)packages/core/src/services/microcompaction/microcompact.ts:730 — [probe] size-path eviction bookkeeping (the changed keptPathRefs line) has zero test coverage (probe: revert ships 86/86 green)packages/cli/src/ui/commands/unskill-command.ts:77 — [probe] isInitialized() pre-first-send guard untested (probe: deleting it ships green)packages/cli/src/services/BuiltinCommandLoader.ts:175 — [review] /unskill missing from web-shell's BUILTIN_COMMAND_DESCRIPTION_KEYS — mixed-language completion menu when daemon locale differspackages/cli/src/ui/commands/unskill-command.ts:146 — [review] unresolvable refusal asserts 'Tracking is kept' on paths where nothing is tracked (--resume, name-independent vouch)packages/core/src/services/microcompaction/microcompact.test.ts:2221 — [probe] kept-body eviction-suppression branch never outcome-determinative (probe: guard deletion ships 86/86 green)packages/cli/src/i18n/mustTranslateKeys.ts:37 — [review] the /unskill safety-refusal string is the only user-facing unskill string omitted from MUST_TRANSLATE_KEYSpackages/cli/src/ui/commands/unskill-command.ts:185 — [review] tab completion hides exactly the tracked skills the action's history fallback reclaims (deleted/renamed mid-session)packages/core/src/services/microcompaction/microcompact.ts:810 — [review] eviction gate's invariant comment is false for command-executor-fallback results, which DID create trackingpackages/core/src/tools/skill-utils.ts:592 — [probe] unloadSkillsFromEntries' resident-name filter (duplicate-residency suppression) has zero test coverage (probe: deletion ships 333/333 green)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — packages/sdk-typescript suite timed out in the scoped run; packages/vscode-ide-companion, packages/web-shell, packages/webui suites did not run (whole-call budget) — the diff touches none of these packages。
未探索到全部深度（达到工具调用预算）：chunk 3：running the two new client.test.ts tests (worktree has no node_modules/dist; npm install + build exceeds the remaining tool budget) — all assertions were verifi…；chunk 11：execute forkedAgent.cache.test.ts to confirm the new test passes — the worktree has no node_modules installed ( vitest unresolvable at startup) and a full mono…；chunk 5：running geminiChat.test.ts under vitest — worktree has no node_modules / dist , and npm install + npm run build exceeds the tool budget; verification was…。
收敛姿态下延后（第 17 轮，非阻断）——已记录，本轮不要求修改：共 13 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.13)
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed — 0 unresolved threads, all prior findings addressed. Approve.
E2E Verification — Terminal Transcript (String Render)Build: Test 1:
|
Maintainer runtime verification — built and driven locally ✅I rebuilt this PR into a real bundle and drove it end-to-end against a local mock provider in tmux, plus a same-worktree A/B against Harness (click to expand)
Scripted turn sequence per arm: load skill → run a shell tool (so a newer tool result exists) → 1 · The ghost-state bug reproduces on
|
| request | phase | skill body in the request payload — BEFORE (main) |
AFTER (this PR) |
|---|---|---|---|
| n-2 | after the shell turn, before /compress-fast |
present | present |
| n-1 | first request of the re-invoke turn (after /compress-fast) |
absent — [Old tool result content cleared] |
absent — [Old tool result content cleared] |
| n | follow-up after the Skill tool ran |
absent — tool returned Skill "demo-poem" is already loaded in context. |
present — full body re-injected |
/compress-fast reported 1467 → 1347 tokens; the ~120 tokens freed are the blanked body.
On main the session is genuinely dead-ended: /context detail keeps printing demo-poem active + body loaded, the wire shows the body is gone, and every further invocation answers already loaded in context. With the PR, active disappears at the same step, the skill stays listed, and the next invocation returns the full body — /context then shows active again. Closed loop.
2 · /unskill lifecycle
Wire-level confirmation of the placeholder — it keeps the original tool_call_id, so tool_use/tool_result pairing stays valid:
{ "role": "tool", "tool_call_id": "call_skill_2_7",
"content": [{"type":"text","text":"[Skill 'demo-poem' unloaded via /unskill; invoke the Skill tool again to reload.]"}] }The next request after re-invoking carries the full body again. All five edge cases from the test plan reproduce verbatim (completion list, ~N tokens freed, non-skill name, real-but-unloaded skill, missing argument). /unskill before the first send (fresh session, client not initialised) answers gracefully instead of throwing.
3 · Un-tracking is targeted, not a blanket clear
Two skills loaded, one unloaded. Body counts on the wire (each body counted by its own canary):
| request | phase | demo-poem bodies |
demo-haiku bodies |
|---|---|---|---|
| both loaded | — | 1 | 1 |
after /unskill demo-poem, re-invoke haiku |
haiku correctly still deduped | 0 (placeholder) | 1 — no duplicate injected |
| re-invoke poem | poem reloads in full | 1 | 1 |
This is the case the design notes worry about most, and it holds: unloading one skill neither disarms the other's dedup guard nor duplicates its body.
4 · --resume / --continue fallback
Resumed a session whose history still carries two skill bodies. In-memory tracking is (as expected) empty after resume, so /context shows neither skill as active — and /unskill demo-poem still located the bodies in history and freed ~218 tokens. The documented fallback works.
Side observation, pre-existing and explicitly out of scope for this PR: because tracking is not restored on resume, re-invoking a skill whose body is still resident injects a second copy (measured: demo-haiku bodies 1 → 2 on the wire). main behaves the same (loadedSkillNames is never persisted). Worth its own follow-up under #6762.
5 · Suites, lint, typecheck, i18n
| check | result |
|---|---|
core: microcompact 86 · skill-utils 14 · skill 83 · geminiChat 333 · permission-manager 333 · memoryPressureMonitor 73 · registerSkillHooks 9 · forkedAgent.cache 20 |
✅ all pass |
cli: unskill-command 11 · BuiltinCommandLoader 14 · Session 639 · i18n 31 |
✅ all pass |
core/client.test.ts |
vi.mock('../telemetry/loggers.js') missing logStartSession) — identical failure on main at 9f8f65dde0, so not caused by this PR, but it means the 383 new lines there were not exercised locally. Please confirm it is green in CI. |
eslint on all 12 changed prod files |
✅ clean |
tsc typecheck (core + cli) |
✅ no error in any file this PR touches (123 pre-existing errors elsewhere, byte-identical on main) |
scripts/check-i18n.ts |
✅ passes; all 4 new strings present in all 9 locales |
6 · Mutation testing — 28 mutants, 21 killed, 6 survived
I neutered each guard the PR adds and re-ran the owning suite. 21 mutants were killed, which is genuinely good coverage for a change this size (targeted un-track, blanket clear, placeholder handling, isForkedChat marking, hook dedup, permission-rule dedup, command registration, ACP settle, hard-rescue reconcile, truncate reconcile — all pinned).
Six survivors, all in code the PR's own comments describe as load-bearing. None is a bug — I probed each and the runtime behaviour is correct — but they are unpinned, so a future refactor can silently remove them:
| # | mutation | result |
|---|---|---|
| 1 | microcompact.ts · buildKeptSkillNames: kept.add(names[0]!) → no-op |
86/86 still pass. A direct probe shows the observable does flip: two resident bodies of one skill, older blanked → evictedSkillNames goes [] → ["demo-poem"]. The existing “suppresses the report when a stale confirmation is blanked but the reloaded body is kept” test passes either way, because it blanks a confirmation — which the separate body-only reporting guard already handles. So buildKeptSkillNames itself has no test. |
| 2 | geminiChat.unloadSkillBody: hasUnresolvableSkillResult(...) gate → if (false) |
333/333 still pass |
| 3 | geminiChat.unloadSkillBody: ambiguous-call-id refusal neutered |
333/333 still pass |
| 4 | unskill-command.ts: if (unresolvable) branch → if (false) |
11/11 still pass |
| 5 | geminiChat.reconcileLoadedSkillTracking(logTag): isForkedChat early return removed |
333/333 still pass |
| 6 | unskill-command.ts: isInitialized() pre-init guard → if (false) |
11/11 still pass |
Survivors 2–4 are the subject of this branch's two most recent commits (“close unresolvable-body gaps in strip sync and /unskill”, “broaden /unskill refusal to any unattributable skill result”) — the fixes landed without tests. I wrote three throwaway probes against unloadSkillBody and all three behave exactly as documented (id-less body → refuse and leave the body intact; mixed resolvable + unattributable → still refuse; ambiguous id → refuse), and two of them fail once the gate is neutered. Could you fold roughly those three cases into geminiChat.test.ts plus one unresolvable case in unskill-command.test.ts, and one two-resident-bodies case for buildKeptSkillNames? That is the only change I would ask for.
7 · Non-blocking observations
- The
unresolvablerefusal is session-global: one unattributable skill result anywhere in history blocks/unskillfor every skill for the rest of the session, even skills whose own call ids are clean. The fail-safe direction is right; scoping it to the queried skill's ids plus id-less entries would be friendlier, and is easy to do later. - Because
hasSkillBodyInHistoryreturnstruewhenever an unattributable result exists,/unskill <bogus-name>can land on the “could not be unloaded safely” message instead of “is not a skill”. Cosmetic. - Pre-existing, unrelated: after
/compress-fast,/context detailunder-reports every skill's tokens (e.g.review142 → 5,demo-poembody+129→+5) because it re-derives fromlistSkills(). Reproduced identically onmainat the same step — worth a separate issue, not this PR's problem.
Verdict
LGTM — approve and merge, ideally after adding tests for the six surviving mutants (survivors 2–4 in particular, since they cover the newest commits). The bug is real, reproduced on main, and fixed; /unskill works exactly as advertised including the targeted-un-track and --resume paths; suites, lint, typecheck and i18n are clean; and no regression showed up in any of the runtime scenarios I drove.
中文版本
维护者本地运行时验证 ✅
我把这个 PR 构建成真实产物，在 tmux 里对着本地 mock provider 端到端跑通，并在同一个 worktree 内与 main 做了 A/B。问题在 main 上完全复现，本 PR 修复了它；/unskill 的行为与描述一致。 以下结论全部来自运行时实测，不是读代码得出的。
验证环境
| PR head | 000f5fe9bf（84 个提交） |
| BEFORE 臂 | merge-base 9f8f65dde0（即当前 main） |
| A/B 方式 | 同一 worktree；把 21 个改动的生产文件回退到 9f8f65dde0（并移除 unskill-command.ts），然后 rm -rf packages/core/dist → npm run build -w core → npm run bundle（每次切换约 25 秒）。每一臂都校验了产物特征串（unloadSkillBody：PR 臂 2 个 chunk / base 臂 0 个）。 |
| Provider | 本地 mock OpenAI 服务；每个出站 /v1/chat/completions 请求体都落盘——判定依据是线上真实 payload，而不是 TUI 文字 |
| CLI | 真实 node dist/cli.js，隔离 HOME，--approval-mode yolo，QWEN_MC_KEEP_RECENT=1，node v22.22.2 / Linux |
| Skills | 两个项目级 skill（demo-poem、demo-haiku），各自带唯一 canary 串，便于在请求体里数正文份数 |
每一臂的脚本化对话序列：加载 skill → 执行一次 shell 工具（制造更新的工具结果）→ /compress-fast → /context detail → 重新调用 skill。
1 · 幽灵状态 bug 在 main 上复现，本 PR 修复
脚本与环境完全相同，只有构建不同。
| 请求 | 阶段 | 请求体中的 skill 正文 — 修复前（main） |
修复后（本 PR） |
|---|---|---|---|
| n-2 | shell 轮之后、/compress-fast 之前 |
存在 | 存在 |
| n-1 | 重新调用轮的首个请求（/compress-fast 之后） |
不存在 — [Old tool result content cleared] |
不存在 — [Old tool result content cleared] |
| n | Skill 工具执行后的跟进请求 |
不存在 — 工具返回 Skill "demo-poem" is already loaded in context. |
存在 — 完整正文重新注入 |
/compress-fast 报告 1467 → 1347 tokens，释放的约 120 tokens 正是被清空的正文。
在 main 上会话确实进入死路：/context detail 持续显示 demo-poem active + body loaded，而线上 payload 里正文已经消失，之后每次调用都回 already loaded in context。加上本 PR 后，同一步 active 消失，skill 仍在列表中，下一次调用返回完整正文，/context 随之恢复 active——状态闭环。
2 · /unskill 生命周期
占位符的线上证据——它保留了原来的 tool_call_id，因此 tool_use/tool_result 配对依然有效：
{ "role": "tool", "tool_call_id": "call_skill_2_7",
"content": [{"type":"text","text":"[Skill 'demo-poem' unloaded via /unskill; invoke the Skill tool again to reload.]"}] }重新调用后的下一个请求重新携带完整正文。测试计划中的五个边界用例全部逐字复现（补全列表、~N tokens freed、非 skill 名、存在但未加载的 skill、缺参数）。在首次发送之前（新会话、client 未初始化）执行 /unskill 也能优雅返回而不是抛错。
3 · 取消标记是定向的，不是整体清空
加载两个 skill，只卸载其一。线上正文份数（按各自 canary 统计）：
| 请求 | 阶段 | demo-poem 正文份数 |
demo-haiku 正文份数 |
|---|---|---|---|
| 两者均加载 | — | 1 | 1 |
/unskill demo-poem 后重新调用 haiku |
haiku 仍被正确去重 | 0（占位符） | 1 — 没有重复注入 |
| 重新调用 poem | poem 完整重载 | 1 | 1 |
这正是设计注释里最担心的场景，实测成立：卸载一个 skill 既不会解除另一个的去重守卫，也不会让它的正文重复。
4 · --resume / --continue 回退路径
恢复了一个历史中仍带两份 skill 正文的会话。恢复后内存中的 tracking（如预期）为空，/context 两个 skill 都不显示 active——而 /unskill demo-poem 依然在历史中定位到正文并释放了约 218 tokens。文档描述的回退路径有效。
顺带观察，属于既有行为且被本 PR 明确排除在范围外：由于 resume 不恢复 tracking，重新调用一个正文仍驻留的 skill 会注入第二份正文（实测：demo-haiku 正文份数在线上从 1 变成 2）。main 行为相同（loadedSkillNames 从未持久化）。建议在 #6762 下单独跟进。
5 · 测试套件、lint、typecheck、i18n
| 检查项 | 结果 |
|---|---|
core：microcompact 86 · skill-utils 14 · skill 83 · geminiChat 333 · permission-manager 333 · memoryPressureMonitor 73 · registerSkillHooks 9 · forkedAgent.cache 20 |
✅ 全部通过 |
cli：unskill-command 11 · BuiltinCommandLoader 14 · Session 639 · i18n 31 |
✅ 全部通过 |
core/client.test.ts |
vi.mock('../telemetry/loggers.js') 缺少 logStartSession）——在 9f8f65dde0 的 main 上完全相同地失败，因此不是本 PR 造成的；但这也意味着那里新增的 383 行未在本地被执行到，请确认 CI 上是绿的。 |
对 12 个改动生产文件跑 eslint |
✅ 无问题 |
tsc typecheck（core + cli） |
✅ 本 PR 触及的文件无任何报错（其他位置 123 条为既有错误，与 main 逐字一致） |
scripts/check-i18n.ts |
✅ 通过；4 条新文案在 9 个语言文件中齐全 |
6 · 变异测试 — 28 个变异体，21 个被杀，6 个存活
我逐个"废掉"了本 PR 新增的守卫并重跑对应套件。21 个变异体被杀，对这个规模的改动来说覆盖度确实不错（定向取消标记、整体清空、占位符处理、isForkedChat 标记、hook 去重、权限规则去重、命令注册、ACP settle、hard-rescue reconcile、truncate reconcile 都被钉住了）。
6 个存活的变异体，全部落在 PR 注释自称关键的代码上。 它们都不是 bug——我逐个探测过，运行时行为正确——但没有测试钉住，未来重构可能悄悄把它们删掉：
| # | 变异 | 结果 |
|---|---|---|
| 1 | microcompact.ts · buildKeptSkillNames：kept.add(names[0]!) 改为空操作 |
86/86 仍通过。直接探测显示可观测行为确实翻转：同一 skill 有两份驻留正文、较旧的被清空时，evictedSkillNames 从 [] 变成 ["demo-poem"]。现有的"stale confirmation 被清空但重载正文保留时抑制上报"这条测试两种情况下都通过，因为它清空的是确认消息——那条路径已由另一个"仅正文才上报"的守卫覆盖。所以 buildKeptSkillNames 本身没有测试。 |
| 2 | geminiChat.unloadSkillBody：hasUnresolvableSkillResult(...) 判断改为 if (false) |
333/333 仍通过 |
| 3 | geminiChat.unloadSkillBody：歧义 call-id 拒绝分支被废掉 |
333/333 仍通过 |
| 4 | unskill-command.ts：if (unresolvable) 分支改为 if (false) |
11/11 仍通过 |
| 5 | geminiChat.reconcileLoadedSkillTracking(logTag)：移除 isForkedChat 提前返回 |
333/333 仍通过 |
| 6 | unskill-command.ts：isInitialized() 前置守卫改为 if (false) |
11/11 仍通过 |
第 2–4 项正是本分支最近两个提交的主题（"close unresolvable-body gaps in strip sync and /unskill"、"broaden /unskill refusal to any unattributable skill result"）——修复合入时没有配套测试。我针对 unloadSkillBody 写了三个一次性探针，三个的行为都与注释完全一致（无 call id 的正文 → 拒绝且保留正文；可解析条目与不可归属条目混合 → 仍然拒绝；歧义 id → 拒绝），其中两个在守卫被废掉后即失败。能否把这三个用例大致补进 geminiChat.test.ts，再在 unskill-command.test.ts 补一个 unresolvable 用例、给 buildKeptSkillNames 补一个"两份驻留正文"的用例？ 这是我唯一想请你补的东西。
7 · 非阻塞观察
unresolvable拒绝是会话级全局的：历史中任意一条不可归属的 skill 结果，会在本会话后续时间里阻止所有 skill 的/unskill，包括那些 call id 完全干净的 skill。fail-safe 方向是对的；把检查收敛到被查询 skill 的 id 加上无 id 条目会更友好，后续做也不迟。- 由于只要存在不可归属结果
hasSkillBodyInHistory就返回true，/unskill <不存在的名字>可能落到"could not be unloaded safely"而不是"is not a skill"。属于文案层面的小问题。 - 既有问题、与本 PR 无关：
/compress-fast之后/context detail会低报所有 skill 的 token（例如review142 → 5，demo-poem正文+129→+5），因为它是从listSkills()重新推导的。在main上同一步骤完全一致地复现——建议单开 issue，不属于本 PR。
结论
LGTM——同意合入，最好先把 6 个存活变异体的测试补上（尤其是第 2–4 项，它们覆盖的是最新的提交）。问题真实存在、在 main 上复现并已修复；/unskill 的行为与描述完全一致，包括定向取消标记与 --resume 路径；套件、lint、typecheck 与 i18n 均干净；我驱动的所有运行时场景中都没有出现回归。
callmeYe
left a comment
callmeYe
left a comment
There was a problem hiding this comment.
中文说明
— GPT-5 via Qwen Code /review (v0.21.4-preview.0)
Withdrawing approval: the two Criticals from @callmeYe are still present at head, and I want the broader design direction reconsidered before this lands — see follow-up comment.
|
Withdrawing my approval here — not just for the two open Criticals (both still reproduce at the current head; the threads were marked resolved without a reply or a follow-up commit, so flagging that the findings are still live), but because I think the underlying mechanism deserves a direction check before we iterate further on it. The eviction-state sync half is the right fix — #6788 should never have landed without it. My concern is the history-rewriting machinery built around it. Encoding lifecycle state as string markers inside conversation history is what produces both Criticals: a placeholder that any tool output can impersonate, and a rewrite with no net-gain guarantee. Patching those guards fixes the symptoms, but every mechanism still queued on #6762 (TTL eviction, one-shot, loaded-skills section) would add another marker rule on top of the same substrate. For reference, Claude Code solves this same problem with a different shape:
Rough direction I'd suggest here:
Could we get a short design doc (state model, re-injection budget, resume/rebuild semantics, interaction with #6788) aligned on #6762 before reworking this? Happy to review an early sketch. The E2E evidence and test coverage in this PR are solid — this is a direction question, not a quality one. 中文说明撤 approve 的原因有两个：一是 @callmeYe 的两条 Critical 在最新 head 上依然存在（thread 被无回复、无 commit 地标记了 resolved，这里说明一下问题仍然有效）；二是想请你重新考虑一下整体设计方向。 淘汰状态同步这半是对的——#6788 当初就该配套它一起合。我的顾虑是围绕它建的"改写历史"机制：把生命周期状态编码成历史文本里的字符串标记，正是两条 Critical 的共同根源（占位符可被任意工具输出冒充；改写没有净收益保证）。就算把这两个守卫补上，#6762 里排队的后续机制（TTL 淘汰、one-shot、loaded-skills 专区）只会在这个基底上继续叠标记规则。 参考一下 Claude Code 的做法：
建议的大致方向：
能不能先在 #6762 上对齐一份简短设计文档（状态模型、重注入预算、resume 重建语义、与 #6788 的关系）再动手重构？早期草稿我可以直接看。这个 PR 的 E2E 证据和测试覆盖都很扎实——这是方向问题，不是质量问题。 |
…he prompt A non-Skill tool result shaped like the unload marker (matching its prefix and suffix) evaded every microcompaction mode because isAlreadyCleared checked the output alone; scope the exemption to Skill responses. /unskill also rewrote every short dedup confirmation to the longer body placeholder, so a tiny body plus confirmations could grow the prompt while reporting zero savings. Confirmations now get a strictly shorter marker, and a net-growing rewrite is refused wholesale (reported as unresolvable so tracking stays armed). Adds regression tests for both entrances.
|
Thanks for the direction review — splitting as suggested:
Closing this PR in favor of #9500. |
* fix(core): sync loaded-skill state with history eviction Refs QwenLM#6762 Split from PR QwenLM#8900 per maintainer direction review: this keeps only the eviction-state sync half (loaded-skills tracking reconciled with history evictions across microcompaction, /compress-fast, memory pressure compaction, client retry, and ACP settle). The /unskill command is deferred to a separate change pending the QwenLM#6762 design discussion. * fix(core): key skill-hook dedup on the whole prepared config The dedup identity keyed only on type + command/url, but the frontmatter admits multiple hooks per matcher distinguished solely by fields that key ignores (timeout/shell for command hooks, headers/timeout for HTTP hooks): the second of such a pair was silently skipped even on the skill's first registration. Key on the whole prepared config instead — frontmatter configs carry no functions, so the structural key stays stable across reload cycles. Adds regression tests for both shapes. * fix(core): address round-2 review — setHistory reconcile, strip-occurrence gate * fix(core): gate skill-body residency on SkillTool provenance - Record exact body outputs in SkillTool as residency provenance; all residency checks fail closed against a fresh process (empty set) instead of trusting the two public markers (spoofable via the shared functionResponse{name:'skill'} shape) - Make setHistory's reconcile the single loaded-skill sync: remove the five post-setHistory second writes (3x syncSkillEvictions, hard-rescue reconcile, tryCompress blanket clear) and delete the now-dead helpers - Correct the settle-reconcile comment to name its five pre-try skip paths; resume stays intentionally un-enumerated (fail-closed reconcile plus one bounded duplicate body beats a marker-based resume door that would reopen the injection window) - Tests: provenance gating (injection/genuine/spoof-stripped), stateful Set oracle for clear-before-track order, microcompact F2 witness and size-path kept-body twin, mixed-batch unload, isRetry settle arming, wire-after-setup hard-rescue, reconcile-only tryCompress, and the three second-write sites flipped to single-writer assertions * fix(core): classify stripped skill responses by call id (R4-1) The scheduler's persistence gate rewrites large genuine skill bodies into <persisted-output> stubs before they enter history. The strip path's provenance/shape check skipped such stubs, leaving the skill tracked with no resident body — the QwenLM#6762 deadlock. unloadSkillsFromEntries now classifies every stripped skill response by call-id resolution: fail-open direction, since over-un-tracking self-heals with one duplicate body while under-un-tracking deadlocks reload; the resident-sibling filter keeps precision. Also pins the R4 findings: provenance recording both halves through the real SkillTool, microcompact provenance wiring at the client call site, the ambiguous call-id policy on both strip and reconcile sides, provenance-mode targeted unload, compressFast's setHistory reconcile, and the forked-chat compression/restore observable effects. Rewrites the evictedSkillNames docstrings to the diagnostic-only contract. * test(cli): complete stale fakes in session-swap telemetry test QwenLM#9764 added getCurrentCustomTitle/getSessionDisplayName calls to useBranchCommand after QwenLM#9844's test fakes were written, so the suite throws before forkSession on any branch carrying both. Fill the two missing fake methods; main CI has not run since before either landed. * refactor(core): reduce skill-eviction sync to conservative clear at rewrite boundaries Per QwenLM#9500 review: drop the process-wide body provenance set and history scans, the evictedSkillNames/unresolvedEvictedSkills diagnostics, the exact-reconcile vs targeted-unload algorithms, and the ACP settle machinery. Instead, conservatively clear loaded-skill tracking at the three destructive history-rewrite boundaries (setHistory covers every compaction path, truncateHistory, stripOrphanedUserEntriesFromHistory), guarded so forked chats never touch the parent's tracker. Over-clearing self-heals with at most one duplicate body on the next invoke; a stale entry made the skill permanently unreloadable. Kept: authoritative-vs-forked ownership guard, idempotent hook and allowedTools re-registration on reload, and a small behavioral test set. Exact lifecycle semantics, explicit unload, and richer eviction diagnostics belong under QwenLM#6762 in follow-up PRs. --------- Co-authored-by: 俊良 <zzj542558@alibaba-inc.com> Co-authored-by: yiliang114 <yiliang.yyl@alibaba-inc.com> Co-authored-by: 易良 <1204183885@qq.com>
7 participants
What this PR does
Keeps the loaded-skill bookkeeping in sync whenever conversation history rewrites remove skill bodies, and adds a user-facing
/unskill <name>command that releases a loaded skill's body from context on demand.Concretely: microcompaction now reports which skill bodies it blanked (and how many it could not resolve); every history-rewriting path — pre-send microcompaction,
/compress-fast, the memory-pressurecompact_historystep, and LLM/compress— consumes that information to un-track the affected skills; and/unskillreplaces a loaded skill's body with a short placeholder, adjusts the session token estimate downward, and un-tracks the skill so it reloads in full on its next invocation. Skill listing stays untouched — only the in-context body is evicted, and reloading is free: the nextSkillinvocation returns the full body again because the dedup guard is re-armed.The fallback direction is deliberate: when a blanked skill body cannot be resolved to a name, tracking is cleared wholesale rather than partially — over-clearing only costs one duplicated body on the next invoke, while under-clearing would leave a skill permanently unreloadable.
Why it's needed
Two related problems, reproduced on the current release:
activein/context, but the body is gone from the model's view. Worse, the dedup guard then answers every re-invocation with "already loaded in context", so the skill is permanently unusable for the rest of the session — the model keeps referring to instructions it can no longer see, and there is no way to recover other than starting a new session.Reviewer Test Plan
How to verify
This touches user-visible behavior; the most meaningful check is a short interactive run against a built bundle (
npm run build && npm run bundle, thennode dist/cli.js) with any test skill installed (e.g. a smalldemo-poemskill under~/.qwen/skills/).Eviction sync (the bug fix):
QWEN_MC_KEEP_RECENT=1(the keep-recent budget is clamped to ≥1, so a newer tool result must exist to push the skill body out of the protected window)./context detailshows it asactivewith abody loadedtoken line.echo hi) so a newer tool result exists./compress-fast. Expected: the skill body is among the blanked results./context detailagain. Expected (fixed behavior): the skill no longer showsactive. (Before this PR, it kept showingactive— the ghost state.)Skilltool returns the full body and/contextshowsactiveagain. (Before this PR, the tool returned "already loaded in context" forever.)/unskill(the new command):activein/context detailand note thebody loadedtoken line./unskill <name>. Expected: an info messageUnloaded skill "<name>" (~N tokens freed). Invoke it again to reload./context detailno longer marks the skill active.activerestored — a closed state loop./unskill nonexistentprintsSkill "nonexistent" is not loaded in context.;/unskillwith no argument prints usage; tab completion offers only currently loaded skill names.Caution while verifying: don't ask the model to recite the skill body before eviction — the recitation lands in a model message that compaction does not blank, which would invalidate a "can the model still see the body?" probe. Assert on
/contextand the raw re-invoke tool result instead.Unit-level: new tests cover the eviction reporting (resolved vs unresolved skill names), the sync consumer at all four call sites (targeted unload vs wholesale clear), the
/unskillcore operation (placeholder replacement, token adjustment, no-op on unknown names), and the command itself (not-loaded message, happy path, completion). Regression suites for chat, client, memory-pressure monitoring, microcompaction, and the skill tool all pass unchanged.Evidence (Before & After)
Before (release build, tmux run): after
/compress-fastblanked the skill body,/context detailstill showeddemo-poem active, the model could no longer see the body, and re-invoking the skill returnedSkill "demo-poem" is already loaded in context.— a dead end.After (this branch, tmux run):
Eviction path after the fix:
/compress-fastfreed ~299 tokens including the skill body;/context detailimmediately stopped showingactive; re-invoking returned the full body instead of "already loaded in context".Tested on
Environment (optional)
Built bundle (
npm run build && npm run bundle,node dist/cli.js) in tmux sessions; the four-step eviction sequence,/unskilllifecycle, and edge cases verified live. Unit and regression suites via vitest.Risk & Scope
--all/--listflags; no cross-session restore of loaded state on resume; subagent skill loading is untouched.Linked Issues
Refs #6762 — intentionally not auto-closing: #6762 is a five-mechanism umbrella request (model-invocable unskill tool, COMPACTABLE_TOOLS — already merged as #6788, TTL/turn auto-evict,
lifecycle: one-shotfrontmatter,<loaded_skills>system-prompt section). This PR delivers the eviction-state sync plus the user-only half of mechanism 1; the remaining mechanisms stay tracked on #6762 for follow-up PRs.中文说明
这个 PR 做了什么
让"已加载 skill"的簿记在会话历史被改写、skill 正文被移除时保持同步，并新增面向用户的
/unskill <name>命令，可按需把已加载 skill 的正文从上下文中释放。具体来说：微压缩现在会上报它清掉了哪些 skill 正文（以及有多少无法解析）；所有改写历史的路径——发送前微压缩、
/compress-fast、内存压力的compact_history步骤、LLM/compress——都会消费这些信息来取消对应 skill 的"已加载"标记；/unskill会把已加载 skill 的正文替换为一条简短的占位符、下调会话 token 估算、并取消该 skill 的标记，使其在下一次调用时完整重载。skill 的列表完全不受影响——只有上下文中的正文被驱逐；重载是零成本的：下一次Skill调用会返回完整正文，因为去重守卫被重新激活。降级方向是刻意选择的：当被清空的 skill 正文无法解析出名字时，整体清空标记而不是部分清理——多清的代价只是下次调用时重复附加一份正文，而少清会让 skill 永久无法重载。
为什么需要
两个相关问题，均已在当前发布版上复现：
/context中仍显示active，但模型已经看不到正文。更糟的是，去重守卫此后对每次重新调用都返回 "already loaded in context"，该 skill 在本会话中永久失效——模型持续引用它看不到的指令，除了开新会话别无恢复手段。评审者测试计划
如何验证
此改动涉及用户可见行为，最有意义的检查是拿构建产物做一小段交互式运行（
npm run build && npm run bundle，然后node dist/cli.js），并准备任意测试 skill（例如在~/.qwen/skills/下放一个小型demo-poemskill）。驱逐同步（bug 修复）：
QWEN_MC_KEEP_RECENT=1启动 CLI（keep-recent 预算被钳位到 ≥1，因此需要一条更新的工具结果才能把 skill 正文挤出保护窗口）。/context detail显示其为active并带body loadedtoken 行。echo hi），让更新的工具结果存在。/compress-fast。预期：skill 正文在被清空的条目之中。/context detail。预期（修复后行为）：skill 不再显示active。（本 PR 之前，它会一直显示active——幽灵状态。）Skill工具返回完整正文，/context重新显示active。（本 PR 之前，工具永远返回 "already loaded in context"。）/unskill（新命令）：/context detail中确认active并记下body loadedtoken 行。/unskill <name>。预期：info 消息Unloaded skill "<name>" (~N tokens freed). Invoke it again to reload./context detail不再标记该 skill 为 active。active恢复——状态闭环。/unskill nonexistent打印Skill "nonexistent" is not loaded in context.；不带参数的/unskill打印用法；tab 补全只列出当前已加载的 skill 名。验证时注意：不要让模型在驱逐前复述 skill 正文——复述会落在压缩不会清空的模型消息里，使"模型是否还能看到正文"的探测失效。应断言
/context输出和重新调用的原始工具结果。单测层面：新增测试覆盖驱逐上报（可解析与不可解析的 skill 名）、四个调用点上的同步消费者（定向卸载 vs 整体清空）、
/unskill的核心操作（占位符替换、token 调整、未知名字的 no-op）以及命令本身（未加载提示、正常路径、补全）。chat、client、内存压力监控、微压缩、skill 工具的回归套件全部原样通过。证据（前后对比）
修复前（发布版构建，tmux 实录）：
/compress-fast清空 skill 正文后，/context detail仍显示demo-poem active，模型已看不到正文，且重新调用 skill 返回Skill "demo-poem" is already loaded in context.——死路一条。修复后（本分支，tmux 实录）：
修复后的驱逐路径：
/compress-fast释放了约 299 tokens（含 skill 正文）；/context detail立即停止显示active；重新调用返回完整正文而非 "already loaded in context"。测试平台
环境（可选）
构建产物（
npm run build && npm run bundle，node dist/cli.js）在 tmux 会话中实测：四步驱逐序列、/unskill生命周期及边界用例均已现场验证。单测与回归套件通过 vitest 运行。风险与范围
--all/--list标志；resume 时不恢复已加载状态；子代理的 skill 加载不受影响。关联 Issue
Refs #6762 —— 有意不自动关闭：#6762 是一个五机制的组合需求（模型可调用的 unskill 工具、COMPACTABLE_TOOLS——已作为 #6788 合入、TTL/轮次自动驱逐、
lifecycle: one-shotfrontmatter、<loaded_skills>系统提示段）。本 PR 交付的是驱逐状态同步 + 机制 1 的用户侧部分；其余机制继续由 #6762 跟踪，留待后续 PR。