feat(skills): enforce auto-skill- directory prefix for auto-generated skills#4839
Merged
Conversation
… skills Auto-generated skills created by the managed-skill-extractor agent now use a mandatory `auto-skill-` directory prefix, and `.gitignore` re-ignores `.qwen/skills/auto-skill-*/` so these transient, session-specific directories no longer surface as untracked files in `git status`. - .gitignore: add `.qwen/skills/auto-skill-*/` after the `!.qwen/skills/**` unignore rule. Git's last-rule-wins semantics re-ignore only the prefixed directories while hand-authored project skills stay tracked. Existing auto-generated skills are not renamed. - skillReviewAgentPlanner.ts: add an AUTO_SKILL_DIR_PREFIX constant and require the prefix in SKILL_REVIEW_SYSTEM_PROMPT and buildTaskPrompt(). The frontmatter `name:` stays the natural name so skills keep their normal invocation. This is a prompt-level convention only — SkillManager discovery is prefix-agnostic and `source: auto-skill` remains the file-level edit-protection marker. - add a test asserting buildTaskPrompt() emits the prefix instruction. Closes #4837
Collaborator
|
@qwen-code /review |
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running. — qwen3.7-max via Qwen Code /review
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
✅ Independent Verification Report — PR #4839Reproduced the Reviewer Test Plan end-to-end in a real local environment. Both documented checks pass, plus a rendered-prompt cross-check. Verdict: LGTM — safe to merge. Environment
Test 1 —
|
| Path | Expect | Result | Decided by |
|---|---|---|---|
.qwen/skills/auto-skill-foo/SKILL.md |
ignored | ✅ ignored | :41 .qwen/skills/auto-skill-*/ |
.qwen/skills/auto-skill-bar/scripts/run.sh (nested) |
ignored | ✅ ignored | :41 …auto-skill-*/ |
.qwen/skills/auto-skill-baz/references/notes.md (nested) |
ignored | ✅ ignored | :41 …auto-skill-*/ |
.qwen/skills/bugfix/SKILL.md (hand-authored) |
tracked | ✅ tracked | :36 !.qwen/skills/** |
.qwen/skills/triage/SKILL.md (hand-authored) |
tracked | ✅ tracked | :36 !…/** |
.qwen/skills/qwen-code-session-data-extraction/… (legacy auto) |
tracked | ✅ tracked | :36 !…/** |
.qwen/skills/qwen-code-token-bridge-server/… (legacy auto) |
tracked | ✅ tracked | :36 !…/** |
.qwen/skills/my-auto-skill-thing/SKILL.md (substring) |
tracked | ✅ tracked | :36 !…/** |
git status ground truth — only the three auto-skill-* dirs are ignored:
Staged (tracked): bugfix/ triage/ qwen-code-session-data-extraction/ qwen-code-token-bridge-server/ my-auto-skill-thing/
Ignored : auto-skill-foo/ auto-skill-bar/ auto-skill-baz/
Notable correctness points confirmed:
- Nested files under an
auto-skill-*dir are ignored too (dir-level match), not justSKILL.md. - Prefix anchors at the directory-segment start:
my-auto-skill-thing/(whereauto-skill-is a substring, not a prefix) is not matched → stays tracked. No over-matching. - Legacy auto-skills without the prefix (
qwen-code-session-data-extraction,qwen-code-token-bridge-server) stay tracked — consistent with the PR's "no renaming of existing skills" scope note.
The shipped .gitignore in the actual PR worktree (full 40+ rule context) was also spot-checked and behaves identically.
Test 2 — prompt-layer unit test ✅ 18 / 18
$ npx vitest run packages/core/src/memory/skillReviewAgentPlanner.test.ts
✓ |@qwen-code/qwen-code-core| src/memory/skillReviewAgentPlanner.test.ts (18 tests) 21ms
Test Files 1 passed (1)
Tests 18 passed (18)
Matches the PR's expected 18 passed. The new test is present and green (verbose reporter):
✓ buildTaskPrompt > instructs the agent to use the auto-skill- directory prefix (#4837)
Bonus — actual rendered prompt
Calling buildTaskPrompt() against the PR source emits the real instruction the review agent receives (prefix substituted, not just an asserted substring):
New skills you create MUST live at
.qwen/skills/auto-skill-<name>/SKILL.md— theauto-skill-directory prefix is mandatory so the project's .gitignore keeps auto-generated skills out of version control. Keep the frontmattername:as the natural<name>(no prefix). The frontmatter MUST includesource: auto-skill:
The mirror instruction in SKILL_REVIEW_SYSTEM_PROMPT reinforces the same rule at the system-prompt layer. Substitution, the mandatory wording, and the "keep frontmatter name: natural" guidance all render correctly.
Observations (non-blocking)
- Prompt-layer soft guard, by design. Enforcement is in the prompt, not in
evaluateScopedDecision. If the agent ignores the instruction, the only fallout is a new dir appearing ingit status— the pre-existing behavior — so no correctness/security boundary is at stake. Matches the PR's stated scope; reasonable. - System-prompt line isn't directly asserted. Only the task prompt (via
buildTaskPrompt) has a unit test; the parallel line inSKILL_REVIEW_SYSTEM_PROMPTis covered only by inspection. It's a static string and low-risk — a one-line assertion would fully close the gap, but it's optional.
Recommendation: ✅ verified — both Reviewer-Test-Plan items reproduce cleanly on Linux; recommend merge.
中文小结
在本地真实环境（🐧 Linux，Node v22.22.2）用 tmux 驱动复现了 PR 的 Reviewer 测试计划，两项均通过：
.gitignore语义（后规则优先）8/8 通过 —auto-skill-*目录（含其下嵌套文件）被忽略；手写 skill（bugfix/triage）、两个 legacy auto-skill、以及my-auto-skill-thing（auto-skill-只是子串而非前缀）都保持被 git 跟踪。前缀正确锚定在目录名开头，无误匹配。判定以git check-ignore -q退出码为准，并用git status --ignored交叉验证。- prompt 层单元测试 18/18 通过 — 含新增用例
instructs the agent to use the auto-skill- directory prefix (#4837)。 - 额外：实际渲染
buildTaskPrompt()输出确认前缀替换、mandatory措辞、以及「frontmattername:保持自然名」指令均正确。
注（非阻塞）：前缀仅在 prompt 层软约束（符合 PR scope）；SKILL_REVIEW_SYSTEM_PROMPT 中的对应行未被单测直接断言，建议（可选）补一行断言。
结论：✅ 验证通过，建议合并。
Collaborator
✅ Verification Report — PR #4839 (
|
Fixture under .qwen/skills/ |
BEFORE (main) |
AFTER (PR #4839) | Intended |
|---|---|---|---|
auto-skill-foo/SKILL.md (auto-gen) |
visible ?? |
IGNORED by :41 .qwen/skills/auto-skill-*/ |
✅ |
auto-skill-bar/nested/deep.md (nested) |
visible ?? |
IGNORED (whole subtree) | ✅ |
myhandskill/SKILL.md (hand-authored) |
visible | visible / trackable | ✅ |
qwen-code-session-data-extraction/SKILL.md (legacy auto-skill) |
visible | visible / trackable | ✅ (PR: out of scope, not renamed) |
git status --porcelain confirms it — on the PR branch auto-skill-foo/ and auto-skill-bar/ disappear from untracked output, while myhandskill/ and the legacy dir remain:
# AFTER (PR 4839) — git status --porcelain .qwen/skills
?? .qwen/skills/Auto-Skill-Caps/
?? .qwen/skills/auto-skill-baz
?? .qwen/skills/myhandskill/
?? .qwen/skills/qwen-code-session-data-extraction/
git check-ignore -v attributes the decision to the new rule, proving last-match-wins over the earlier un-ignore:
.qwen/skills/auto-skill-foo/SKILL.md -> .gitignore:41:.qwen/skills/auto-skill-*/
2. ✅ Prompt ↔ gitignore consistency. Both prompt strings are inside .join('\n') arrays (lines 257 & 353), so they render into the real system/task prompts. With the actual constant AUTO_SKILL_DIR_PREFIX = 'auto-skill-' they produce .qwen/skills/auto-skill-<name>/SKILL.md and contain "mandatory" — and auto-skill-<name>/ is exactly the directory shape step 1 proved gets ignored. The two halves of the PR are aligned.
3. 🔍 Graceful-degradation of the soft guard. If the agent ever omits the prefix, the new skill simply shows up in git status — identical to today's behavior. Directly observed: the legacy qwen-code-session-data-extraction/ (no prefix) stays visible on the PR branch. So the worst case of a prompt miss is the verified-safe status quo, never data loss.
Findings (edge probes)
⚠️ Reserved-namespace footgun. A hand-authored skill literally namedauto-skill-*(probe:auto-skill-collide/) is silently ignored — no warning. Acceptable since the prefix is reserved for auto-gen, but worth a one-line note in the prompt/docs that users shouldn't hand-author skills with this prefix.- 🔍 Directory-only (correct). A plain file
auto-skill-baz(not a directory) is not ignored — the rule's trailing/scopes it to directories. Fine, since real skills are always directories. - 🔍 Case-sensitivity (Linux now covered).
Auto-Skill-Caps/is not ignored on Linux (git is case-sensitive); the prompt mandates lowercaseauto-skill-, so this is fine in practice. Note for the macOS-only-tested PR: on a case-insensitive FS (core.ignorecase=true) a capitalized variant could match — harmless given the lowercase mandate. Linux ignore behavior is confirmed ✅ here (PR table had it as⚠️ ). - ℹ️ Standard git caveat (not a defect):
.gitignoredoes not retroactively untrack anauto-skill-*dir that was already committed. Not applicable to current state; relevant only if such a dir were ever committed before this rule landed.
Bottom line
The enforcing mechanism (the .gitignore rule) does exactly what it claims at its real runtime surface, with hand-authored and legacy skills correctly untouched; the prompt change is consistent with it and fails safe. Recommend merge. The one cheap follow-up worth considering is a half-sentence reserving auto-skill- as an auto-gen-only directory namespace so a future hand-authored collision isn't silently un-tracked.
Verified locally on Linux via tmux + real-repo worktrees (PR head 4b6cc15c vs main ce53283a). No CI rerun; runtime git observation only.
…pace Follow-up to reviewer verification on #4839. - Export SKILL_REVIEW_SYSTEM_PROMPT and add a unit test asserting it carries the `auto-skill-` prefix instruction. The prefix mandate lives on two independent string arrays (SKILL_REVIEW_SYSTEM_PROMPT and buildTaskPrompt); only the latter was guarded, so an edit could silently drop the mandate from the system-prompt layer. The new test closes that gap. - Extend the .gitignore comment to reserve `auto-skill-` as an auto-generated namespace, noting that a hand-authored skill using this prefix would be ignored — preventing a silent-untrack surprise.
Collaborator
Author
|
Both follow-ups from your verification reports are in
The remaining items match my read and need no change: the soft guard is prompt-only by design (a missed prefix degrades to the existing "shows in Thanks for the thorough before/after and edge-probe verification on Linux. 中文：两点都已在 |
wenshao
approved these changes
Jun 8, 2026
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running. — qwen3.7-max via Qwen Code /review
tanzhenxin
approved these changes
Jun 8, 2026
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. CI green across Lint/Tests/CodeQL, clean and well-tested change enforcing the auto-skill- directory prefix. Thanks!
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
Auto-generated skills — the ones the
managed-skill-extractorforked agent writes under.qwen/skills/— now use a mandatoryauto-skill-directory prefix, and the project.gitignorere-ignores.qwen/skills/auto-skill-*/. Hand-authored project skills (e.g.bugfix,triage) keep their natural directory names and stay tracked; only auto-generated directories are excluded from version control. The change is two parts: a.gitignorerule, and a prompt-layer instruction that steers the review agent to create new skill directories with the prefix. The skill's frontmattername:stays the natural name, so a skill created at.qwen/skills/auto-skill-foo/is still invoked as/foo.Why it's needed
When auto-skill generation creates skills under
.qwen/skills/, they show up as untracked files ingit statusbecause the current.gitignoreexplicitly unignores the entire.qwen/skills/directory. This is noise for developers and risks accidentally committing transient, session-specific skills. A directory-name prefix solves it cleanly: git's last-rule-wins semantics re-ignore onlyauto-skill--prefixed directories while keeping hand-authored skills tracked. The existingsource: auto-skillfrontmatter marker only helps at file-read time — it can't drive.gitignoreor a directory listing, which is what a name prefix does.Reviewer Test Plan
How to verify
.gitignoresemantics (last-rule-wins) — in a scratch repo with the updated rules:Expected: only the
auto-skill--prefixed directory is ignored; hand-authored and pre-existing auto-skills remain tracked.Prompt-layer instruction — unit test:
Expected: 18 passed, including
instructs the agent to use the auto-skill- directory prefix (#4837), which assertsbuildTaskPrompt()emits.qwen/skills/auto-skill-<name>/and themandatorywording.Evidence (Before & After)
N/A — non-user-visible (
.gitignorerule + prompt text). The behavioral change is the agent's chosen directory name and git's ignore status, both covered by the verification steps above.Tested on
Environment (optional)
Unit tests only (
npx vitest run);.gitignoresemantics verified withgit check-ignorein a scratch repo.Risk & Scope
evaluateScopedDecision). If the agent ignores the instruction, the sole consequence is that the new skill appears ingit status— the pre-existing behavior — so there is no security or correctness boundary at stake. Keeping it prompt-only matches the issue scope.SkillManagerdiscovery (it is prefix-agnostic by design) and no renaming of existing auto-generated skills (qwen-code-session-data-extraction,qwen-code-token-bridge-server).Linked Issues
Closes #4837
中文说明
这个 PR 做了什么
自动生成的 skill（由
managed-skill-extractorforked agent 写入.qwen/skills/的那些）现在强制使用auto-skill-目录前缀，项目.gitignore重新忽略.qwen/skills/auto-skill-*/。手写的项目 skill（如bugfix、triage）保持自然目录名并继续被 git 跟踪，只有自动生成的目录被排除出版本控制。改动分两部分：一条.gitignore规则，以及一条 prompt 层指令，引导 review agent 用带前缀的目录名创建新 skill。skill 的 frontmattername:保持自然名，所以创建在.qwen/skills/auto-skill-foo/的 skill 仍然用/foo调用。为什么需要
auto-skill 在
.qwen/skills/下创建后，由于当前.gitignore显式 unignore 了整个目录，它们会出现在git status的 untracked files 中，造成噪音并有误提交风险。目录前缀干净解决：git 的"后规则优先"语义只重新忽略带auto-skill-前缀的目录，手写 skill 保持跟踪。已有的source: auto-skillfrontmatter 标记只在读文件时有效，无法驱动.gitignore或目录列表——而前缀可以。Reviewer 测试计划
如何验证
.gitignore语义（后规则优先）——在 scratch 仓库用更新后的规则：git check-ignore -v验证auto-skill-foo/SKILL.md被忽略，而bugfix/SKILL.md和 legacy 的qwen-code-session-data-extraction/SKILL.md不被忽略。npx vitest run packages/core/src/memory/skillReviewAgentPlanner.test.ts，预期 18 passed，含新测试instructs the agent to use the auto-skill- directory prefix (#4837)。证据（Before & After）
N/A——非用户可见改动（
.gitignore规则 + prompt 文本）。测试平台
仅 macOS 本地验证；Windows / Linux 由 CI 覆盖。
风险与范围
evaluateScopedDecision）。即使 agent 忽略指令，唯一后果是新 skill 出现在git status（即当前已有行为），无安全或正确性边界风险。只做 prompt 层符合 issue scope。SkillManagerdiscovery（设计上就 prefix-agnostic），不重命名已有自动生成 skill。关联 Issue
Closes #4837