Follow-up work left by #10841 (skill naming)

Follow-up work left by #10841

#10841 merged as fdb33117b4 and closed #9408. Its review rounds deferred a set of findings rather than resolving them, and one Critical was filed against the merged head and merged anyway. This issue collects that work so each piece can land as its own small PR.

Everything below was recorded during review of #10841. Item 1 has been re-verified against main at 05a54fc32a. Items 2 through 8 are described as the reviewers filed them and have not been re-verified against current main.

Line numbers are given at 05a54fc32a and will drift. Symbol names are the stable reference.

1. The daemon skill toggle guard infers extension identity from punctuation

skillToggleBlockForName in packages/cli/src/config/skill-settings.ts (line 294) derives the authored spelling of a requested skill name by splitting at its first colon:

const prefixEnd = skillName.indexOf(':');
return buildHigherDisabled(settings).blockIn({
  name: skillName,
  ...(prefixEnd > 0 ? { authoredName: skillName.slice(prefixEnd + 1) } : {}),
});

The doc comment on qualifySkillName in packages/core/src/skills/types.ts (line 334) states the opposite invariant for the same feature:

Built only by concatenation from the manifest that owns the skill, never parsed back apart — SKILL_NAME_PATTERN admits :, so an author may write rust:chat inside rust and that skill's authored name is rust:chat, not chat.

Consequence as filed in review 5177458504: a project skill whose authored name contains a colon, with a trusted workspace skills.disabled holding that same name plus an unrelated entry matching the invented alias, gets its enable refused. Correct behaviour removes the exact entry and succeeds. user, project and bundled skills are never prefixed, so they keep their complete name and have no authored alias to invent.

Both daemon persistence paths reach this helper, in packages/cli/src/serve/run-qwen-serve.ts at line 5415 (single toggle) and line 5494 (batch toggle).

The authoritative identity is already available to the daemon and is dropped on the way out. packages/cli/src/serve/workspace-skills-status.ts line 162 calls skillManager.listSkills(), which returns SkillConfig carrying authoredName (packages/core/src/skills/types.ts line 119). mapSkillConfigToStatus in packages/cli/src/runtime/workspace-skills-mapping.ts (line 21) maps fourteen fields into the status entry. name, level and extensionName are three of them and authoredName is not, so the authored spelling never reaches the toggle route. ServeWorkspaceSkillStatus in packages/acp-bridge/src/status.ts (line 518) declares no authoredName field either.

The two fields that are already present identify the skill, so the fix needs no new field. SkillManager prefixes only extension skills, at packages/core/src/skills/skill-manager.ts line 1036, and it records extensionName on each of them. A skill whose level is not extension therefore has one spelling and no alias to infer. For an extension skill the registry name is extensionName, then :, then the authored name, so removing that known prefix returns the authored name correctly even when the authored name itself contains a colon.

The delete path already resolves a requested name against that authoritative list, in packages/cli/src/serve/workspace-service/index.ts at lines 1087 and 1094, with a level filter and an exact-name preference.

Suggested direction, as named in review 5177458504: use registry or manifest identity for the target and never infer extension ownership from punctuation. The helpers skillRestrictionNames (line 359) and authoredSkillName (line 382) in packages/core/src/skills/types.ts already take a skill object rather than a string.

2. Batch toggle outcomes depend on item order

With a user skill pdf plus a workspace bare disablement, the batch [pdf, rust:pdf] enables both while [rust:pdf, pdf] refuses rust:pdf, even though the same batch removes its blocker. Batch outcomes carry no block reason, so a caller cannot tell why. Raised in round 7 of #10841.

Named fixes: re-check refused items against the batch's final lists, or return the block per item. Test gap: neither the batch refusal nor the in-batch exception is covered, so removing either survives.

This shares both call sites with item 1, so the two belong in one PR, and item 1 must be fixed first.

3. A default-off qualified skill can be left off with no diagnostic

When a user-level skill of the same authored name exists, the group loop skips the whole group, so a defaultDisabled entry plus an identical enabled entry leaves a genuinely default-off qualified skill off with nothing reported anywhere. Filed as N-3 in round 5, carried through rounds 6 and 7. Changes no reachable state, only the silence around one.

4. The bare-entry warning's advice is unexecutable from system settings

Following the warning's advice verbatim in the only file the user can edit leaves the warning and the state identical across two boots, while the picker on that same boot already names the lock with entry and scope. The advice must name the scope it cannot reach, or point at the lock. Filed as N-6 in round 5, carried through rounds 6 and 7.

5. The pair warning's advice silently re-enables a sibling

"Remove that entry too", applied in one edit, re-enables a sibling the warning never names, and the next boot prints no warning at all because the pair branch lists only its still-off members. Named fix: when the bare hard entry is present, append the sister generator's existing "also re-enables ... add ... to keep it blocked" sentence for the group members outside the still-off list. Filed as N-9 in round 6, carried in round 7.

Two wording slips ride along: the bare note says "blocks them" right after naming one skill, and the qualified note uses a plural subject with a singular verb.

6. The web shell reports a refused enable as an unchanged setting

After the toggle guard, a refused enable writes nothing, which is correct, but the web shell skills page maps every unchanged response to "Skill already has the requested workspace setting; no setting was changed". That is false, because the skill is still disabled. The SDK's daemon toggle result type has no block field for the page to read the reason.

Lives in the web-shell and sdk-typescript packages. Carries an i18n tail because the honest notice is a new string.

7. Warning text composition lives in core config

The two warning generators bareEnabledGrantWarnings (lines 1749 to 1863) and bareDisablementBlocksQualifiedGrantWarnings (lines 1873 to 1949) hold 192 lines of English warning text composition inside packages/core/src/config/config.ts, a file of 10843 lines. Review 5170856226 recorded the figure as about 220 added core lines; 192 is the measured span of those two functions at 05a54fc32a. Message text living in core is what kept re-triggering documentation and schema wording findings during #10841, with the same sentence required in four places: the core warning, the settings doc, the skills doc, and en.js.

PR D takes a narrow slice of this. Move the warning strings out of those two generators into en.js, with the zh and zh-TW coverage that check-i18n requires. Leave the settings doc and the JSON schema wording untouched. That half of the finding stays open here and belongs to no PR in this split.

8. Warning wiring is pinned by presence rather than behaviour

R2-2, R3-5 and R3-6 from rounds 2 and 3 remain open at Suggestion severity. Test-strengthening only, no behaviour change.

Proposed split and ordering

A and B are disjoint and can proceed in parallel. B and D both rewrite the same warning generators in core config, so D must follow B. The four PRs stay separate. C may depend on the toggle response shape produced by A, since item 6 needs a block reason for the page to read.

Item 9 from the #10841 follow-up list, a carried observation about a divergent sister budget for the same concept elsewhere in the tree, was recorded non-blocking and never turned into an actionable finding by any reviewer. It is not included here.

Start with the proposed PR split and item 1, reading skillToggleBlockForName in packages/cli/src/config/skill-settings.ts alongside the skill identity types, status mapping, and workspace service paths named in the issue. Trace the existing toggle and warning tests, then add regression coverage for the selected item; done means the relevant behavior is corrected without disturbing the other split dependencies.