mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-24 05:26:21 +00:00
d2ece83726
* feat(skills): support priority field in SKILL.md for sorting skill display order
Closes #4136
* fix(skills): make /skills respect priority and treat unset as 0
- /skills was re-sorting alphabetically after listSkills(), masking the
new priority order. Drop the redundant sort and reuse the manager's
output directly.
- Treat missing priority as 0 instead of -Infinity so an explicit
negative priority (e.g. -1) sorts below unset skills, which matches
user intent.
* fix(skills): harden priority parsing and ordering
* fix(skills): warn when extension supplies invalid priority
Extension-provided skills bypass parseSkillContent / validateConfig, so a
non-number `priority` was silently normalized to 0 in the sort with zero
diagnostic. Match the SKILL.md author signal: warn at load time so the
extension author can see and fix the typo.
Addresses PR #4155 review (the extension-bypass-validation point).
* test(skills): direct unit tests for parsePriorityField and normalizeSkillPriority
Both helpers are exported but previously had no direct tests — coverage
came only via parseSkillContent and listSkills. Adds inputs the
integration paths can't surface cleanly: -0 / NaN / Infinity, numeric
strings, objects, arrays, and the boolean coercion regression that
motivated the strict typecheck.
Also adds a NOTE on parsePriorityField warning future contributors that
SKILL.md frontmatter parsing lives in two places (parseSkillContent here
and SkillManager.parseSkillContent), so any new field must be wired into
both — the same regression that previously hit whenToUse,
disable-model-invocation, paths, and priority. Full dedup of the two
parseSkillContent bodies is left as a follow-up refactor.
Addresses the remaining two [Suggestion] items from PR #4155 review.
* fix(skills): scope priority to /skills listing only
Earlier in this PR, `skill.priority` was mapped into `SlashCommand.completionPriority`
on both bundled and non-bundled skill loaders, so a high-priority skill
also bubbled up in the slash-completion menu and the `/help` custom-commands
tab. That was broader than intended — the design goal is for `priority:`
to control the `/skills` listing only, with everything else (typing `/`,
mid-input completion, `/help`) staying purely alphabetical so a skill
can't reorder built-in commands.
Changes:
- BundledSkillLoader / SkillCommandLoader: drop the
`completionPriority: skill.priority` mapping. Skill commands now have
no `completionPriority`, falling back to alphabetical+recency in the
shared completion comparator.
- Help.tsx: revert the per-group sort to `localeCompare` and remove the
`compareCommandsForHelp` helper. `/help` is again purely alphabetical
within each group.
- Tests:
- Both loader tests assert `completionPriority` is `undefined` when
a skill has a `priority` set, locking the non-leakage in.
- Help.test.tsx's "orders by completionPriority" case is replaced
with "orders alphabetically regardless of completionPriority", so a
future change that re-introduces the leak fails the test.
- Extension-skill validation also normalizes `skill.priority` to 0 (in
addition to the existing sort-time normalization) so downstream
consumers see a clean value matching the emitted warning.
Validation:
- 177/177 unit tests pass across the 5 affected test files
- core typecheck clean
- bundled CLI built (`npm run bundle`) and exercised via tmux E2E:
E1 /skills sorted by priority, E2 / completion menu unaffected,
E3 mid-input alphabetical, E4 invalid priority warns + skill loads,
E5 order stable across restart — all 5 pass.
* fix(skills): tag priority warning with calling module's namespace
`parsePriorityField` previously hardcoded `debugLogger.warn` from
skill-load, so a warning emitted from `SkillManager.parseSkillContent`
(project / user / bundled skills) was tagged `[SKILL_LOAD]` instead of
`[SKILL_MANAGER]`. Annoying for log filtering and slightly misleading
about which parse path actually surfaced the bad priority.
Added an optional `warn` callback parameter; the existing extension
call site keeps the default skill-load logger, while skill-manager
passes its own. Behavior is otherwise unchanged.
* docs(skills): correct priority scope description
Earlier doc said priority sorts "in /skills, slash-command completion,
and the /help custom commands view." After the scope-narrowing in
|
||
|---|---|---|
| .. | ||
| channels | ||
| _meta.ts | ||
| approval-mode.md | ||
| arena.md | ||
| auto-mode.md | ||
| checkpointing.md | ||
| code-review.md | ||
| commands.md | ||
| dual-output.md | ||
| followup-suggestions.md | ||
| headless.md | ||
| hooks.md | ||
| language.md | ||
| lsp.md | ||
| markdown-rendering.md | ||
| mcp.md | ||
| memory.md | ||
| sandbox.md | ||
| scheduled-tasks.md | ||
| skills.md | ||
| status-line.md | ||
| structured-output.md | ||
| sub-agents.md | ||
| tips.md | ||
| token-caching.md | ||
| tool-use-summaries.md | ||