From f334f277ddea3c52599b521ac728aceeeaa9a9dc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 13 Sep 2026 22:17:48 -0400 Subject: [PATCH] fix(#4324): stop the retired /gsd: prefix reaching users (#4712) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#4324): prove colon tokens the installer cannot convert leak Failing-first regression coverage for #4324. The install rewrite (transformContentToHyphen) is gated on an exact match against the commands/gsd stem list, so any /gsd: whose token is not a registered stem survives the install and reaches the user as the deprecated colon form. The gate is load-bearing -- it is the only thing protecting the workflow DSL marker family (gsd:section, gsd:protected, gsd:loop-host, gsd:guard, gsd:dispatch, gsd:plan-revision-conflicts), which workflow-fragments parses as a literal. So this suite asserts the shipped text is convertible rather than asserting the transform is broad, and pins the marker family as explicit negative space. Co-Authored-By: Claude Opus 5 * fix(#4324): stop unconvertible colon tokens reaching the user The install rewrite is gated on an exact match against the commands/gsd stem list, so a /gsd: whose token is not a registered stem survives the install and reaches the user as the deprecated colon form. That gate is load-bearing -- it protects the gsd:section / gsd:protected / gsd:loop-host marker family -- so the fix is in the shipped text, and the source stays colon per CONTEXT.md's two-tier rule. - quick-batch command + skill description: close the command token at a boundary so `/gsd:quick`-shaped converts instead of being skipped. - gsd-code-fixer (both variants): execute-plan and diagnose-issues are workflows, not commands, so they never converted and rendered beside two hyphenated siblings on the same line. Name them as workflows. - help topic-mode: the extraction rule hard-coded a colon prefix that the converted full.md never ships, so --brief could never match a signature line and silently fell back on every topic. Describe the signature line without a literal prefix. - update.md: drop the prefix from prose describing a stale command. Co-Authored-By: Claude Opus 5 * chore(#4324): add changeset fragment pr:0 placeholder is backfilled with the real number once the PR exists. Co-Authored-By: Claude Opus 5 * fix(#4324): locate the help summary per reference variant Adversarial review finding. Restoring the signature-line match (the #4324 fix) activated a latent defect in the clause next to it: compact scope emitted "the single non-blank line immediately after" the signature, and that clause is only correct for full.md. full.compact.md puts the summary on the signature line itself, after an em-dash, and its next non-blank line is an unrelated "Usage:" line. Both variants ship and both are served, so before this commit the compact variant would have emitted the wrong line as the summary. It was masked until now only because the stale colon prefix meant no signature line ever matched at all. Name the two placements and pick per line, and say explicitly that a Usage: line is never a summary. Co-Authored-By: Claude Opus 5 * test(#4324): de-vacuum the help parity check, narrow the marker waiver Two adversarial review findings against the #4324 coverage. The help-parity assertion went vacuous the moment the fix landed: once topic.md stops spelling a literal prefix, the matched set is empty and the assertion holds for any rewording, correct or not. It now also asserts across BOTH served reference variants that each ships signature lines under the hyphen prefix, that the two genuinely disagree about where the summary sits, and that topic.md still names both placements and the Usage: guard. The marker waiver keyed on "sits inside an HTML comment", which waves through a real broken reference that happens to be commented out -- `` scored clean. Enumerate the six marker families instead. Verified the narrowed rule catches that probe and still passes over the tree; it also surfaced a seventh family, write-continue, that the broad rule was hiding. Co-Authored-By: Claude Opus 5 * fix(#4324): normalize the namespace in skill descriptions Both hyphen-namespace skill converters ran the hyphen transform over the body but rebuilt the frontmatter description from the raw field, so a /gsd: mention in a command description survived into the installed SKILL.md -- the exact field the host's skill picker renders, which is the surface this issue was filed about. The local flat-command path was already correct because it rewrites the whole file; only the skills path, used by a global install, was affected. Confirmed by installing into a fake HOME before and after. Fixed in both copies: bin/install.js and the src/ source of truth that compiles into gsd-core/bin/lib. Co-Authored-By: Claude Opus 5 * test(#4324): assert descriptions through the real converters The previous version of this check called transformContentToHyphen on the description line itself and passed, while a real install still shipped the colon form -- the converter never calls that transform on the description. It asserted a proxy for the behaviour instead of the behaviour. Drive convertClaudeCommandToClaudeSkill and convertClaudeCommandToClineSkill over every registered command and assert on the emitted description. Verified it fails against the pre-fix converters and passes against the fixed ones. Co-Authored-By: Claude Opus 5 * chore(#4324): regenerate skills after the description change skills//SKILL.md is generated by gen-plugin-skills, not hand-maintained, and lint:generated-sync caught the hand edit. The regenerated file emits the hyphen form, which also corrects the assumption behind the scan comment in the namespace test: skills/ is runtime-emitter output, not colon source. Co-Authored-By: Claude Opus 5 * test(#4324): re-sanction normalizeKimiSkillName's real end line The description-normalisation fix inserted five lines above normalizeKimiSkillName in src/runtime-artifact-conversion.cts, moving its closing brace from 635 to 640. MAJOR-1 pins that line deliberately, so the planted violation landed INSIDE the exempted body and went unflagged -- 0 !== 1. Re-sanction the value rather than derive it: the array is named sanctionedRealEndLines, and a pinned line that fails loudly on drift is the design. Deriving it would remove the human check the name asks for. Verified by executing all four MAJOR-1 rows against the real tree: each planted violation is flagged at realEndLine+1 and each unmodified file stays exempt. Emitted-Drift-Ack-Growth: gsd-code-fixer.md — names execute-plan and diagnose-issues as workflows rather than as slash commands that do not exist Emitted-Drift-Ack-Growth: gsd-code-fixer.compact.md — same rewording as its full sibling, kept byte-consistent with it Co-Authored-By: Claude Opus 5 * chore(#4324): backfill the changeset PR number Co-Authored-By: Claude Opus 5 --------- Co-authored-by: sim Co-authored-by: Claude Opus 5 --- .changeset/calm-lemurs-sprint.md | 5 + agents/gsd-code-fixer.compact.md | 2 +- agents/gsd-code-fixer.md | 4 +- bin/install.js | 9 +- commands/gsd/quick-batch.md | 2 +- gsd-core/workflows/help/modes/topic.md | 20 +- gsd-core/workflows/update.md | 2 +- skills/gsd-quick-batch/SKILL.md | 2 +- src/runtime-artifact-conversion.cts | 9 +- tests/slash-command-namespace.test.cjs | 372 +++++++++++++++++++++ tests/slug-derivation-drift-guard.test.cjs | 2 +- 11 files changed, 415 insertions(+), 14 deletions(-) create mode 100644 .changeset/calm-lemurs-sprint.md diff --git a/.changeset/calm-lemurs-sprint.md b/.changeset/calm-lemurs-sprint.md new file mode 100644 index 000000000..8aa680f90 --- /dev/null +++ b/.changeset/calm-lemurs-sprint.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4712 +--- +**Slash-command suggestions no longer show the retired `/gsd:` form** — a handful of shipped command descriptions, agent bodies and workflow instructions used `/gsd:` tokens the installer could not convert, so they reached you as the deprecated colon form after a fresh install. One consequence was functional, not cosmetic: `/gsd-help --brief ` looked for a signature line that the installed reference never renders, so every topic silently fell back to its first paragraph. (#4324) diff --git a/agents/gsd-code-fixer.compact.md b/agents/gsd-code-fixer.compact.md index af6caeb72..2352e63a4 100644 --- a/agents/gsd-code-fixer.compact.md +++ b/agents/gsd-code-fixer.compact.md @@ -120,7 +120,7 @@ After applying each fix: **Isolation: create a dedicated git worktree BEFORE touching any files.** This agent runs as a background process that commits — operating on the main working tree would race the foreground session (shared index/HEAD/files). Every instance runs in its own isolated worktree. -**Honor `workflow.use_worktrees` (the documented opt-out; the same flag the sibling writer workflows `/gsd:execute-phase`, `/gsd:execute-plan`, `/gsd:quick`, `/gsd:diagnose-issues` all honor — this is the only writer that hand-rolls its own worktree).** Read it directly via `node` from `.planning/config.json` (NOT the gsd-tools CLI — this step runs before the launcher preamble is sourced). When `false`: edit/commit in the main checkout directly — `wt="."`, `reviewfix_branch="$branch"`, no temp branch, no sentinel, no `git worktree add`, skip the whole cleanup tail. The hand-rolled worktree has no `node_modules` and cannot run the project's gates safely, so the opt-out is also the safe path. +**Honor `workflow.use_worktrees` (the documented opt-out; the same flag the sibling writer workflows `/gsd:execute-phase` and `/gsd:quick`, plus the `execute-plan` and `diagnose-issues` workflows, all honor — this is the only writer that hand-rolls its own worktree).** Read it directly via `node` from `.planning/config.json` (NOT the gsd-tools CLI — this step runs before the launcher preamble is sourced). When `false`: edit/commit in the main checkout directly — `wt="."`, `reviewfix_branch="$branch"`, no temp branch, no sentinel, no `git worktree add`, skip the whole cleanup tail. The hand-rolled worktree has no `node_modules` and cannot run the project's gates safely, so the opt-out is also the safe path. ```bash USE_WORKTREES=$(node -e ' diff --git a/agents/gsd-code-fixer.md b/agents/gsd-code-fixer.md index ae3f2ea14..d8377ad93 100644 --- a/agents/gsd-code-fixer.md +++ b/agents/gsd-code-fixer.md @@ -217,8 +217,8 @@ If a finding references multiple files (in Fix section or Issue section): This agent runs as a background process that makes commits. Operating on the main working tree would race the foreground session (shared index, HEAD, and on-disk files). Instead, every instance runs in its own isolated worktree. **#2825: honor `workflow.use_worktrees`.** This is the ONLY writer that hand-rolls a git worktree -inside the agent prompt; every other writer path (`/gsd:execute-phase`, `/gsd:execute-plan`, -`/gsd:quick`, `/gsd:diagnose-issues`) reads `workflow.use_worktrees` and skips isolation when it is +inside the agent prompt; every other writer path (`/gsd:execute-phase`, `/gsd:quick`, and the +`execute-plan` / `diagnose-issues` workflows) reads `workflow.use_worktrees` and skips isolation when it is `false`. Read the same flag here and, when it is `false`, edit and commit in the main checkout directly (set `wt="."`, no `reviewfix_branch`, no recovery sentinel, no `git worktree add`, and skip the cleanup tail — there is no worktree to remove). When the flag is not `false`, the transactional diff --git a/bin/install.js b/bin/install.js index c3f5473d1..d695f9398 100755 --- a/bin/install.js +++ b/bin/install.js @@ -2023,7 +2023,12 @@ function convertClaudeCommandToClaudeSkill(content, skillName, runtime = null, c const names = cmdNames || readGsdCommandNames(); const normalizedBody = transformContentToHyphen(body, names); - const description = extractFrontmatterField(frontmatter, 'description') || ''; + // #4324: the description is the text the host's skill picker renders, so it + // needs the same hyphen normalisation the body gets — otherwise a `/gsd:` + // mention in a command description ships the retired colon form to the user. + const description = transformContentToHyphen( + extractFrontmatterField(frontmatter, 'description') || '', names, + ); const argumentHint = extractFrontmatterField(frontmatter, 'argument-hint'); const agent = extractFrontmatterField(frontmatter, 'agent'); // #769: preserve context: from source command files so it is emitted into @@ -2947,6 +2952,8 @@ function convertClaudeCommandToClineSkill(content, skillName, runtime = null, cm let description = extractFrontmatterField(frontmatter, 'description'); if (!description) description = `Run GSD workflow ${skillName}.`; description = toSingleLine(description); + // #4324: same reason as the Claude skill converter above. + description = transformContentToHyphen(description, names); // Cline documented max is 1024 code points (not UTF-16 code units). // Use Array.from to iterate by code point so that multibyte characters // (e.g. emoji, astral-plane chars) are never split, which would produce diff --git a/commands/gsd/quick-batch.md b/commands/gsd/quick-batch.md index 33a58ca86..f54554596 100644 --- a/commands/gsd/quick-batch.md +++ b/commands/gsd/quick-batch.md @@ -1,6 +1,6 @@ --- name: gsd:quick-batch -description: Batch several /gsd:quick-shaped tasks together — planned, dispatched, and merged as one run +description: Batch several `/gsd:quick`-shaped tasks together — planned, dispatched, and merged as one run argument-hint: "[--file ] [--jobs auto|N] [--validate] [--research] [--resume ] [task list]" allowed-tools: - Read diff --git a/gsd-core/workflows/help/modes/topic.md b/gsd-core/workflows/help/modes/topic.md index 07761879c..32e8b151e 100644 --- a/gsd-core/workflows/help/modes/topic.md +++ b/gsd-core/workflows/help/modes/topic.md @@ -53,20 +53,30 @@ Emit a section from the full reference for the topic in `$ARGUMENTS`. Read `work Use the canonical alias from the leftmost column. Use the literal heading text from the matched cell. State the scope you are about to emit. -5. Read `workflows/help/modes/full.md`. Strip `` / `` wrapper tags — never emit them. Apply the extraction rule for the matched table cell, modulated by scope: +5. Read `workflows/help/modes/full.md` — when `workflow.compact_content` is on, the installed reference is its `full.compact.md` sibling instead, and the two differ in where a command's one-line summary sits (see *Locating the summary* below). Strip `` / `` wrapper tags — never emit them. Apply the extraction rule for the matched table cell, modulated by scope: 5a. **Single section** (cell contains a single `` `## Heading` `` or `` `### Heading` ``): - *Full scope:* emit from that heading up to (but not including) the next sibling or higher-level heading. - - *Compact scope:* emit the heading, then the first `` **`/gsd:...`** `` bold line within the section (the signature) and the single non-blank line immediately after it (the one-line summary). If the section has no `` **`/gsd:...`** `` bold line, emit the heading and the first paragraph. + - *Compact scope:* emit the heading, then the first **command-signature** bold line within the section (a `` **` [args]`** `` line, whatever prefix the installed reference uses) together with its one-line summary, located per *Locating the summary* below. If the section has no command-signature bold line, emit the heading and the first paragraph. 5b. **Multiple sections joined by "plus"**: apply rule 5a to each listed section in document order and emit them sequentially with no gap between them. - 5c. **Sub-block** (cell says `the /gsd:X block under ### Heading` or `the /gsd:X ... blocks under ### Heading`): within the named heading's section, start at each `` **`/gsd:X ...`** `` bold line. - - *Full scope:* stop immediately before the next `` **`/gsd:...`** `` bold line or the next heading, whichever comes first. - - *Compact scope:* emit the bold line and the single non-blank line immediately after it (the one-line summary). + 5c. **Sub-block** (cell says `the block under ### Heading` or `the ... blocks under ### Heading`): within the named heading's section, start at each command-signature bold line for that command. + - *Full scope:* stop immediately before the next command-signature bold line or the next heading, whichever comes first. + - *Compact scope:* emit the bold line together with its one-line summary, located per *Locating the summary* below. For cells listing multiple sub-blocks, emit them sequentially. + **Locating the summary.** The summary sits in one of two places, and which one depends on which + reference variant is installed — decide per signature line, by looking at the line itself: + - If the signature line continues past its closing `**` with an em-dash followed by prose, that + trailing prose **is** the summary. Emit that one line and stop; do not also emit the line + after it. + - Otherwise the summary is the single non-blank line immediately after the signature line. Emit + both lines. + A `Usage:` line is never a summary. If applying the second case would emit one, emit the + signature line alone. + 6. After the section content, emit a single closing line: ```text diff --git a/gsd-core/workflows/update.md b/gsd-core/workflows/update.md index 18da798b1..83601d895 100644 --- a/gsd-core/workflows/update.md +++ b/gsd-core/workflows/update.md @@ -526,7 +526,7 @@ already empty). Say nothing and continue — the update flow is unchanged. Otherwise, render the report. Each entry carries `path`, `outcome`, and a `warnings` array of `{code, detail}` produced by a compatibility pass against -the just-installed release — a renamed workflow it `@`-references, a `/gsd:` +the just-installed release — a renamed workflow it `@`-references, a slash command that no longer exists, missing skill frontmatter. Render each entry's warnings under its path. Entries whose `outcome` starts with `skipped_` will **not** be restored; list them separately, with their reason, so the user knows diff --git a/skills/gsd-quick-batch/SKILL.md b/skills/gsd-quick-batch/SKILL.md index d646c710c..73b3309dc 100644 --- a/skills/gsd-quick-batch/SKILL.md +++ b/skills/gsd-quick-batch/SKILL.md @@ -1,6 +1,6 @@ --- name: gsd-quick-batch -description: "Batch several /gsd:quick-shaped tasks together — planned, dispatched, and merged as one run" +description: "Batch several `/gsd-quick`-shaped tasks together — planned, dispatched, and merged as one run" argument-hint: "[--file ] [--jobs auto|N] [--validate] [--research] [--resume ] [task list]" allowed-tools: - Read diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index df46a4a5c..5af48bb77 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -481,7 +481,12 @@ function convertClaudeCommandToClaudeSkill(content, skillName, runtime = null, c const names = cmdNames || readGsdCommandNames(); const normalizedBody = transformContentToHyphen(body, names); - const description = extractFrontmatterField(frontmatter, 'description') || ''; + // #4324: the description is the text the host's skill picker renders, so it + // needs the same hyphen normalisation the body gets — otherwise a `/gsd:` + // mention in a command description ships the retired colon form to the user. + const description = transformContentToHyphen( + extractFrontmatterField(frontmatter, 'description') || '', names, + ); const argumentHint = extractFrontmatterField(frontmatter, 'argument-hint'); const agent = extractFrontmatterField(frontmatter, 'agent'); // #769: preserve context: from source command files so it is emitted into @@ -1769,6 +1774,8 @@ function convertClaudeCommandToClineSkill(content, skillName, _runtime = null, c let description = extractFrontmatterField(frontmatter, 'description'); if (!description) description = `Run GSD workflow ${skillName}.`; description = toSingleLine(description); + // #4324: same reason as the Claude skill converter above. + description = transformContentToHyphen(description, names); // Cline documented max is 1024 code points (not UTF-16 code units). // Use Array.from to iterate by code point so that multibyte characters // (e.g. emoji, astral-plane chars) are never split, which would produce diff --git a/tests/slash-command-namespace.test.cjs b/tests/slash-command-namespace.test.cjs index 8a200275d..265941d93 100644 --- a/tests/slash-command-namespace.test.cjs +++ b/tests/slash-command-namespace.test.cjs @@ -1099,3 +1099,375 @@ describe('bug #3683 — workflow/reference colon-namespace leak (Claude local in }); }); } + +// ──────────────────────────────────────────────────────────────────────── +// #4324 — colon tokens the install transform CANNOT convert leak to users +// ──────────────────────────────────────────────────────────────────────── +// +// Companion to the `#3443` invariant above, and deliberately its mirror image. +// That one asserts the source stays COLON. This one asserts every colon token +// in the source is one the installer can actually turn into hyphen form. +// +// The install rewrite (`transformContentToHyphen`) is gated on an exact match +// against the `commands/gsd/*.md` stem list, so a `/gsd:` whose token is +// not a registered stem survives the install untouched and reaches the user as +// the deprecated colon form. #4324 reported this as "all auto-suggestions are +// still using the outdated /gsd:". +// +// That gate is load-bearing and must NOT be widened: it is the only thing +// protecting the workflow DSL marker family (`gsd:section`, `gsd:protected`, +// `gsd:loop-host`, `gsd:guard`, `gsd:dispatch`, `gsd:plan-revision-conflicts`), +// which is parsed as a literal — `src/workflow-fragments.cts` pins +// `CLOSE_TAG = '/gsd:section'`. The fix therefore belongs in the shipped text, +// and this suite is what keeps it there. +{ + const { describe, test } = require('node:test'); + const assert = require('node:assert/strict'); + const fs = require('node:fs'); + const path = require('node:path'); + + const ROOT = path.join(__dirname, '..'); + const FIXER = path.join(ROOT, 'scripts', 'fix-slash-commands.cjs'); + // Drive the REAL production transform with the REAL roster. A roster invented + // here could only confirm what this test's author already believed about the + // gate (fixture-provenance rule, #2371). + const { + transformContentToHyphen, + buildColonPattern, + readCmdNames, + SKIP_DIRS, + } = require(FIXER); + + const cmdNames = readCmdNames(); + + // Shipped surfaces whose text the runtime loads and shows the user. + // + // `skills/` is included even though the #3443 colon-invariant scan omits it — + // but not for the reason that scan omits it. skills//SKILL.md is + // GENERATED by scripts/gen-plugin-skills.cjs and is already emitted in hyphen + // form, so it is runtime-emitter output rather than colon source. Scanning it + // is a cheap belt-and-braces check that the generator never emits a colon + // token the installer could not convert; it currently contributes zero + // tokens, and that is the expected steady state. + const SCAN_DIRS = [ + path.join(ROOT, 'commands', 'gsd'), + path.join(ROOT, 'agents'), + path.join(ROOT, 'gsd-core', 'workflows'), + path.join(ROOT, 'gsd-core', 'references'), + path.join(ROOT, 'gsd-core', 'templates'), + path.join(ROOT, 'skills'), + ]; + + // Structural markers that legitimately use `gsd:` and are NOT slash commands. + // Enumerated BY FAMILY, never by "it sits in a comment": a blanket + // comment-context waiver would also wave through a genuinely broken command + // reference that merely happens to be commented out, e.g. + // ``, which is exactly the leak this guard exists + // to catch. + const COMMENT_MARKER_TOKENS = new Set([ + 'section', // / + 'protected', // / :end + 'loop-host', // + 'plan-revision-conflicts', // / :end + 'live-dom-families', // + 'write-continue', // + ]); + const BARE_MARKER_TOKENS = new Set([ + 'guard', // `# gsd:guard=orchestrator-cwd-drift` + 'dispatch', // `[gsd:dispatch phase="…" plan="…"]` + ]); + const IN_HTML_COMMENT = /` — the compact-content disclosure + // banner. Its token is empty, so it is matched by shape rather than by name. + const COMPACT_DISCLOSURE = /', + '', + '', + '', + '', + '', + '# gsd:guard=orchestrator-cwd-drift', + '[gsd:dispatch phase="{phase_number}" plan="{plan_id}"]', + '', + ]; + for (const marker of markers) { + assert.equal( + transformContentToHyphen(marker, cmdNames), + marker, + `structural marker must survive byte-identical: ${marker}`, + ); + // CRLF variant — same verdict. + assert.equal( + transformContentToHyphen(`${marker}\r\n`, cmdNames), + `${marker}\r\n`, + `structural marker must survive byte-identical under CRLF: ${marker}`, + ); + } + }); + }); +} diff --git a/tests/slug-derivation-drift-guard.test.cjs b/tests/slug-derivation-drift-guard.test.cjs index 80048c1c1..5d54110d7 100644 --- a/tests/slug-derivation-drift-guard.test.cjs +++ b/tests/slug-derivation-drift-guard.test.cjs @@ -130,7 +130,7 @@ describe('findSlugDerivationDrift — MAJOR-1: allowlist exemption is scoped to const sanctionedRealEndLines = [ { file: path.join('src', 'core-utils.cts'), fn: 'generateSlugInternal', realEndLine: 199 }, { file: path.join('src', 'gsd2-import.cts'), fn: 'slugify', realEndLine: 103 }, - { file: path.join('src', 'runtime-artifact-conversion.cts'), fn: 'normalizeKimiSkillName', realEndLine: 635 }, + { file: path.join('src', 'runtime-artifact-conversion.cts'), fn: 'normalizeKimiSkillName', realEndLine: 640 }, { file: path.join('scripts', 'generate-package-identity.cjs'), fn: 'slugifyPackageName', realEndLine: 42 }, ];