From 4d394a249d5113832ecaa4d544b4211687ab7862 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Wed, 29 Apr 2026 21:56:59 -0500 Subject: [PATCH] fix(commands): normalize gsd slash namespace drift (#2858) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(commands): normalize gsd slash namespace drift * fix(#2855): address CodeRabbit findings on namespace drift PR Three CR findings, all valid: 1. autonomous.md line 783 still had `gsd:discuss-phase` (the PR's own normalization missed this line). Switched to `gsd-discuss-phase` and updated the matching test in autonomous-interactive.test.cjs that was asserting the now-retired colon form. 2. tests/bug-2543-gsd-slash-namespace.test.cjs source-grepped the fix-slash-commands.cjs script with .includes() rather than driving its transform behaviour. Refactored fix-slash-commands.cjs to export a pure transformContent(src, cmdNames) function, kept the CLI behaviour unchanged via require.main, and replaced the source-grep block with five behavioural cases: rewrite, multi-occurrence, idempotence on canonical input, no-op on gsd-sdk/gsd-tools, and word-boundary safety. 3. tests/bug-2808-skill-hyphen-name.test.cjs matched `name:` anywhere in SKILL.md; a stray name: in the body could satisfy the assertion. Scoped the lookup to the YAML frontmatter block via the suggested diff (parse the leading --- ... --- region first, then find name: inside it). Full suite: 5854/5854 passing. * fix(#2855): address remaining CodeRabbit findings on PR #2858 Three structural concerns flagged on the namespace-drift fix PR: 1. scripts/fix-slash-commands.cjs:24 — `buildPattern([])` compiled `/gsd:()(?=[^a-zA-Z0-9_-]|$)/g`. The empty capture group still matches any `/gsd:` token followed by a non-word boundary (whitespace, EOL, punctuation), rewriting it to a stray `/gsd-`. Verified live: `transformContent("/gsd:", [])` → `"/gsd-"`. Added a guard returning null from `buildPattern` on empty input and updated `transformContent` and `processDir` to no-op when the pattern is null. 2. tests/autonomous-interactive.test.cjs:44-47 — assertion was `content.includes('gsd-discuss-phase') && content.includes('INTERACTIVE')`, which would false-pass on any unrelated co-occurrence (e.g. `INTERACTIVE=""` initialization plus a stray `gsd-discuss-phase` prose mention). Replaced with a structural extraction: locate the `**If \`INTERACTIVE\` is set:**` branch, bound it by the next `**If` / `` boundary, and assert the `Skill(skill="gsd-discuss-phase", ...)` invocation lives inside that region. Tolerates whitespace around `(`, `skill`, and `=`. 3. tests/bug-2808-skill-hyphen-name.test.cjs:104 — colon-call regex was `Skill\(skill=...` and missed valid formatting like `Skill(skill = "gsd:cmd")` or `Skill( skill = ...)`. Loosened to `Skill\(\s*skill\s*=\s*...` so reformatting drift can't slip past the namespace guard. Verification: 5854/5854 pass on `npm test` from the rebased branch. * fix(#2855): drop pre-validation filter that hid namespace drift CR finding on tests/bug-2808-skill-hyphen-name.test.cjs:128: the test collected generated skill directories with `.filter(entry => entry.isDirectory() && entry.name.startsWith('gsd-'))`, then validated namespace invariants over that filtered list. Anything that violated the prefix invariant — `gsd:extract-learnings` (colon form), `extract_learnings` without prefix, `Gsd-foo` mis-cased — would silently disappear from the iteration and the test would falsely pass. Drop the `startsWith('gsd-')` filter so every generated directory shows up. Add explicit assertions before the existing per-skill loop: - directory list is non-empty (catches a broken converter that produces nothing) - every directory begins with `gsd-` - every directory contains no `:` - every directory contains no `_` Re-audited the full PR diff for the same anti-pattern: only this one site filtered before validating the namespace; bug-2643 and commands-doc-parity also use `readdirSync().filter()` but only by file extension, which is correct. 5854/5854 on `npm test`. * fix(#2855): address remaining CR findings (1 active + 2 nitpicks) Three findings on PR #2858, all the same root cause: input narrowing before validation lets drift slip past the guards. 1. tests/bug-2808-...:104 (active) — `colonCallRe` captured local names with `[a-z0-9-]+`, which excluded the underscore. A drift like `Skill(skill="gsd:extract_learnings")` (deprecated colon syntax with the old underscore filename) silently slid through. Broadened the capture to `[^'"\s)]+` so any malformed local name is surfaced; surrounding pattern (whitespace tolerance, escape support, flags) unchanged. 2. tests/bug-2643-...:43 (nitpick) — `extractSkillNamesHyphen` and `extractSkillNamesColon` had the same over-strict capture plus relied on a single regex over raw bytes, which the project test- rigor memory bans (`feedback_no_source_grep_tests.md`). Replaced with `extractSkillCalls(content)` — a small structural extractor that walks `Skill(` openers, locates each call's matching `)`, parses the body's `skill = "..."` keyword argument with permissive whitespace + quoting + escape handling, and returns `{ name, raw }` records. The two namespace-form helpers become thin filters over the structured output. Tightened the body class to `[^'"\\]+` so a trailing escape `\` before the closing quote (as in `Skill(skill=\"gsd-foo\", …)` written inside another string context) doesn't get included in the captured name. 3. tests/bug-2543-...:44 (nitpick) — `DOC_SEARCH_FILES` was a hand- curated 7-entry array. Every doc added in the future would silently weaken drift detection until someone remembered to extend the list. Replaced with `discoverDocSearchFiles(ROOT)`: globs every `.md` under `docs/` and adds `README.md` if present. New docs are picked up automatically. Re-audited the diff surface for similar narrowings; no other sites filter or constrain before validating namespace invariants. 5854/5854 on `npm test`. * fix(#2855): recurse docs/ tree so localized translations are scanned too CR finding: discoverDocSearchFiles() stopped at docs/*.md, leaving localized translation trees (docs/ja-JP/, docs/zh-CN/, docs/ko-KR/, docs/pt-BR/) and other nested doc collections (docs/skills/, docs/superpowers/) invisible to the namespace-drift invariant. Verified the gap: docs/ has 6 nested directories with ~30 .md files that the previous top-level-only scan was skipping. None contain /gsd: references today, but a future translation update or new doc subdir could leak drift. Switch to an iterative stack walk so every .md under docs/ is scanned regardless of depth. Stack form (rather than recursion) avoids the risk of running into the call-stack limit on deep doc trees. 5854/5854 on `npm test`. --------- Co-authored-by: Tom Boucher --- CHANGELOG.md | 1 + bin/install.js | 4 +- ...ract_learnings.md => extract-learnings.md} | 0 docs/AGENTS.md | 2 +- docs/ARCHITECTURE.md | 8 +- docs/CLI-TOOLS.md | 2 +- docs/FEATURES.md | 4 +- docs/INVENTORY-MANIFEST.json | 2 +- docs/INVENTORY.md | 2 +- docs/USER-GUIDE.md | 9 +- get-shit-done/workflows/autonomous.md | 6 +- scripts/fix-slash-commands.cjs | 61 ++++++++--- tests/autonomous-interactive.test.cjs | 34 +++++- tests/bug-2543-gsd-slash-namespace.test.cjs | 100 +++++++++++++++++- .../bug-2643-skill-frontmatter-name.test.cjs | 63 +++++++++-- tests/bug-2808-skill-hyphen-name.test.cjs | 67 +++++++++++- tests/commands-doc-parity.test.cjs | 4 +- tests/extract-learnings.test.cjs | 4 +- 18 files changed, 314 insertions(+), 59 deletions(-) rename commands/gsd/{extract_learnings.md => extract-learnings.md} (100%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c25d7cfa..c06599ca6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). (#2789) ### Fixed +- **GSD slash command namespace drift cleaned up across docs, workflows, and autocomplete** — remaining active `/gsd:` references now use canonical `/gsd-`, escaped workflow `Skill(skill=\"gsd:...\")` prompts now use hyphenated skill names, `scripts/fix-slash-commands.cjs` rewrites retired colon syntax to hyphen syntax, and the extract-learnings command file now uses `extract-learnings.md` so generated Claude/Qwen skill autocomplete exposes `gsd-extract-learnings` instead of `gsd-extract_learnings`. (#2855) - **`extractCurrentMilestone` no longer truncates ROADMAP.md at heading-like lines inside fenced code blocks** — the milestone-end search now scans line-by-line while tracking ` ``` ` / `~~~` fence state, so a line like `# Ops runbook (v1.0 compat)` inside a code block no longer acts as a milestone boundary. Previously, any phase defined after such a block was invisible to `roadmap analyze`, `roadmap get-phase`, `/gsd-autonomous`, and all phase-number commands. (#2787) - **Codex install no longer corrupts existing `~/.codex/config.toml`** — the installer now defensively strips legacy `[agents]` (single-bracket) and `[[agents]]` (sequence) diff --git a/bin/install.js b/bin/install.js index 6e9035270..d05fc6b02 100755 --- a/bin/install.js +++ b/bin/install.js @@ -1169,8 +1169,8 @@ function skillFrontmatterName(skillDirName) { * Convert a Claude command (.md) to a Claude skill (SKILL.md). * Claude Code is the native format, so minimal conversion needed — * preserve allowed-tools as YAML multiline list, preserve argument-hint. - * Emits `name: gsd:` (colon) so Skill(skill="gsd:") calls in - * workflows resolve on flat-skills installs — see #2643. + * Emits `name: gsd-` (hyphen) so Skill(skill="gsd-") calls and + * tab autocomplete use the canonical command namespace. */ function convertClaudeCommandToClaudeSkill(content, skillName) { const { frontmatter, body } = extractFrontmatterAndBody(content); diff --git a/commands/gsd/extract_learnings.md b/commands/gsd/extract-learnings.md similarity index 100% rename from commands/gsd/extract_learnings.md rename to commands/gsd/extract-learnings.md diff --git a/docs/AGENTS.md b/docs/AGENTS.md index e081b02e2..b877b199d 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -343,7 +343,7 @@ GSD uses a multi-agent architecture where thin orchestrators (workflow files) sp | Property | Value | |----------|-------| -| **Spawned by** | `/gsd-map-codebase`, post-execute drift gate in `/gsd:execute-phase` | +| **Spawned by** | `/gsd-map-codebase`, post-execute drift gate in `/gsd-execute-phase` | | **Parallelism** | 4 instances (tech, architecture, quality, concerns) | | **Tools** | Read, Bash, Grep, Glob, Write | | **Model (balanced)** | Haiku | diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ce95e968c..f69b189d9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -134,7 +134,7 @@ Orchestration logic that commands reference. Contains the step-by-step process i #### Progressive disclosure for workflows Workflow files are loaded verbatim into Claude's context every time the -corresponding `/gsd:*` command is invoked. To keep that cost bounded, the +corresponding `/gsd-*` command is invoked. To keep that cost bounded, the workflow size budget enforced by `tests/workflow-size-budget.test.cjs` mirrors the agent budget from #2361: @@ -534,7 +534,7 @@ Equivalent paths for other runtimes: ### Post-Execute Codebase Drift Gate (#2003) -After the last wave of `/gsd:execute-phase` commits, the workflow runs a +After the last wave of `/gsd-execute-phase` commits, the workflow runs a non-blocking `codebase_drift_gate` step (between `schema_drift_gate` and `verify_phase_goal`). It compares the diff `last_mapped_commit..HEAD` against `.planning/codebase/STRUCTURE.md` and counts four kinds of @@ -546,7 +546,7 @@ structural elements: 4. New route modules under `routes/` or `api/` If the count meets `workflow.drift_threshold` (default 3), the gate either -**warns** (default) with the suggested `/gsd:map-codebase --paths …` command, +**warns** (default) with the suggested `/gsd-map-codebase --paths …` command, or **auto-remaps** (`workflow.drift_action = auto-remap`) by spawning `gsd-codebase-mapper` scoped to the affected paths. Any error in detection or remap is logged and the phase continues — drift detection cannot fail @@ -675,4 +675,4 @@ GSD supports multiple AI coding runtimes through a unified command/workflow arch 4. **Path conventions** — Each runtime stores config in different directories 5. **Model references** — `inherit` profile lets GSD defer to runtime's model selection -The installer handles all translation at install time. Workflows and agents are written in Claude Code's native format and transformed during deployment. \ No newline at end of file +The installer handles all translation at install time. Workflows and agents are written in Claude Code's native format and transformed during deployment. diff --git a/docs/CLI-TOOLS.md b/docs/CLI-TOOLS.md index 3bba936f0..5e5bbb064 100644 --- a/docs/CLI-TOOLS.md +++ b/docs/CLI-TOOLS.md @@ -498,4 +498,4 @@ API keys configured via `/gsd-settings-integrations` (`brave_search`, `firecrawl - [sdk/src/query/QUERY-HANDLERS.md](../sdk/src/query/QUERY-HANDLERS.md) — registry matrix, routing, golden parity, intentional CJS differences - [Architecture](ARCHITECTURE.md) — where `gsd-sdk query` fits in orchestration -- [Command Reference](COMMANDS.md) — user-facing `/gsd:` commands +- [Command Reference](COMMANDS.md) — user-facing `/gsd-` commands diff --git a/docs/FEATURES.md b/docs/FEATURES.md index f832e3e8c..4a154bc05 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -813,7 +813,7 @@ against the mapping point, not HEAD. ### 27a. Post-Execute Codebase Drift Detection **Introduced by:** #2003 -**Trigger:** Runs automatically at the end of every `/gsd:execute-phase` +**Trigger:** Runs automatically at the end of every `/gsd-execute-phase` **Configuration:** - `workflow.drift_threshold` (integer, default `3`) — minimum new structural elements before the gate acts. @@ -837,7 +837,7 @@ continues. Drift detection cannot fail verification. - REQ-DRIFT-02: Action fires only when element count ≥ `workflow.drift_threshold` - REQ-DRIFT-03: `warn` action MUST NOT spawn any agent - REQ-DRIFT-04: `auto-remap` action MUST pass sanitized `--paths` to the mapper -- REQ-DRIFT-05: Detection/remap failure MUST be non-blocking for `/gsd:execute-phase` +- REQ-DRIFT-05: Detection/remap failure MUST be non-blocking for `/gsd-execute-phase` - REQ-DRIFT-06: `last_mapped_commit` round-trip through YAML frontmatter on each `.planning/codebase/*.md` file diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 192c9b63a..d3b001e94 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -60,7 +60,7 @@ "/gsd-eval-review", "/gsd-execute-phase", "/gsd-explore", - "/gsd-extract_learnings", + "/gsd-extract-learnings", "/gsd-fast", "/gsd-forensics", "/gsd-from-gsd2", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index b4859caab..f95eb23c8 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -140,7 +140,7 @@ Full roster at `commands/gsd/*.md`. The groupings below mirror `docs/COMMANDS.md | `/gsd-scan` | Rapid codebase assessment — lightweight alternative to `/gsd-map-codebase`. | [commands/gsd/scan.md](../commands/gsd/scan.md) | | `/gsd-intel` | Query, inspect, or refresh codebase intelligence files in `.planning/intel/`. | [commands/gsd/intel.md](../commands/gsd/intel.md) | | `/gsd-graphify` | Build, query, and inspect the project knowledge graph in `.planning/graphs/`. | [commands/gsd/graphify.md](../commands/gsd/graphify.md) | -| `/gsd-extract-learnings` | Extract decisions, lessons, patterns, and surprises from completed phase artifacts. | [commands/gsd/extract_learnings.md](../commands/gsd/extract_learnings.md) | +| `/gsd-extract-learnings` | Extract decisions, lessons, patterns, and surprises from completed phase artifacts. | [commands/gsd/extract-learnings.md](../commands/gsd/extract-learnings.md) | ### Review, Debug & Recovery diff --git a/docs/USER-GUIDE.md b/docs/USER-GUIDE.md index ad3d6ae36..3f3648fc7 100644 --- a/docs/USER-GUIDE.md +++ b/docs/USER-GUIDE.md @@ -862,16 +862,16 @@ claude --dangerously-skip-permissions # (normal phase workflow from here) ``` -**Post-execute drift detection (#2003).** After every `/gsd:execute-phase`, +**Post-execute drift detection (#2003).** After every `/gsd-execute-phase`, GSD checks whether the phase introduced enough structural change (new directories, barrel exports, migrations, or route modules) to make `.planning/codebase/STRUCTURE.md` stale. If it did, the default behavior is -to print a one-shot warning suggesting the exact `/gsd:map-codebase --paths …` +to print a one-shot warning suggesting the exact `/gsd-map-codebase --paths …` invocation to refresh just the affected subtrees. Flip the behavior with: ```bash -/gsd:settings workflow.drift_action auto-remap # remap automatically -/gsd:settings workflow.drift_threshold 5 # tune sensitivity +/gsd-settings workflow.drift_action auto-remap # remap automatically +/gsd-settings workflow.drift_threshold 5 # tune sensitivity ``` The gate is non-blocking: any internal failure logs and the phase continues. @@ -1297,4 +1297,3 @@ For reference, here is what GSD creates in your project: XX-UI-REVIEW.md # Visual audit scores (from /gsd-ui-review) ui-reviews/ # Screenshots from /gsd-ui-review (gitignored) ``` - diff --git a/get-shit-done/workflows/autonomous.md b/get-shit-done/workflows/autonomous.md index 4b1fc953d..a7c5365a8 100644 --- a/get-shit-done/workflows/autonomous.md +++ b/get-shit-done/workflows/autonomous.md @@ -322,7 +322,7 @@ UI_SPEC_FILE=$(ls "${PHASE_DIR}"/*-UI-SPEC.md 2>/dev/null | head -1) Agent( description="Plan phase ${PHASE_NUM}: ${PHASE_NAME}", run_in_background=true, - prompt="Run plan-phase for phase ${PHASE_NUM}: Skill(skill=\"gsd:plan-phase\", args=\"${PHASE_NUM}\")" + prompt="Run plan-phase for phase ${PHASE_NUM}: Skill(skill=\"gsd-plan-phase\", args=\"${PHASE_NUM}\")" ) ``` @@ -344,7 +344,7 @@ Verify plan produced output — re-run `init phase-op` and check `has_plans`. If Agent( description="Execute phase ${PHASE_NUM}: ${PHASE_NAME}", run_in_background=true, - prompt="Run execute-phase for phase ${PHASE_NUM}: Skill(skill=\"gsd:execute-phase\", args=\"${PHASE_NUM} --no-transition\")" + prompt="Run execute-phase for phase ${PHASE_NUM}: Skill(skill=\"gsd-execute-phase\", args=\"${PHASE_NUM} --no-transition\")" ) ``` @@ -780,7 +780,7 @@ When any phase operation fails or a blocker is detected, present 3 options via A - [ ] `--to N` compatible with `--from N` (run phases from M to N) - [ ] `--to N` handle_blocker resume message preserves --to flag - [ ] `--to N` skips lifecycle when not all milestone phases complete -- [ ] `--interactive` runs discuss inline via gsd:discuss-phase (asks questions, waits for user) +- [ ] `--interactive` runs discuss inline via gsd-discuss-phase (asks questions, waits for user) - [ ] `--interactive` dispatches plan and execute as background agents (context isolation) - [ ] `--interactive` enables pipeline parallelism: discuss Phase N+1 while Phase N builds - [ ] `--interactive` main context only accumulates discuss conversations (lean) diff --git a/scripts/fix-slash-commands.cjs b/scripts/fix-slash-commands.cjs index cabf09531..4e8abae83 100644 --- a/scripts/fix-slash-commands.cjs +++ b/scripts/fix-slash-commands.cjs @@ -1,21 +1,16 @@ 'use strict'; /** - * One-shot script: replace /gsd- with /gsd: for known command names. + * One-shot script: replace retired /gsd: with /gsd- for known command names. * Only replaces when followed by a word boundary (space, newline, quote, backtick, ), end). + * + * The transform is exported as a pure function so it can be unit-tested directly + * (see tests/bug-2543-gsd-slash-namespace.test.cjs) without needing fixture files. */ const fs = require('node:fs'); const path = require('node:path'); const COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); -const cmdNames = fs.readdirSync(COMMANDS_DIR) - .filter(f => f.endsWith('.md')) - .map(f => f.replace(/\.md$/, '')) - .sort((a, b) => b.length - a.length); // longest first to avoid partial matches - -// Build regex: /gsd-(cmd1|cmd2|...) followed by non-word-char or end -const pattern = new RegExp(`/gsd-(${cmdNames.join('|')})(?=[^a-zA-Z0-9_-]|$)`, 'g'); - const SEARCH_DIRS = [ path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib'), path.join(__dirname, '..', 'get-shit-done', 'workflows'), @@ -26,16 +21,46 @@ const SEARCH_DIRS = [ ]; const EXTENSIONS = new Set(['.md', '.cjs', '.js']); -function processDir(dir) { +function buildPattern(cmdNames) { + // Empty input would compile `/gsd:()(?=[^a-zA-Z0-9_-]|$)/g`, which the regex + // engine still matches at any `/gsd:` token followed by a non-word boundary + // (e.g. EOL, whitespace, punctuation) — rewriting it to a stray `/gsd-`. + // Short-circuit so the caller can no-op on a missing/empty registry rather + // than perform an unintended broad rewrite. + if (!Array.isArray(cmdNames) || cmdNames.length === 0) return null; + const sorted = [...cmdNames].sort((a, b) => b.length - a.length); // longest first to avoid partial matches + return new RegExp(`/gsd:(${sorted.join('|')})(?=[^a-zA-Z0-9_-]|$)`, 'g'); +} + +/** + * Pure transform: rewrite retired `/gsd:` to `/gsd-` for the given command names. + * Returns the rewritten string. Identifiers not in `cmdNames` (e.g. `/gsd:sdk`, + * `/gsd:tools`) are left untouched. + */ +function transformContent(src, cmdNames) { + const pattern = buildPattern(cmdNames); + if (!pattern) return src; + return src.replace(pattern, (_, cmd) => `/gsd-${cmd}`); +} + +function readCmdNames() { + return fs.readdirSync(COMMANDS_DIR) + .filter(f => f.endsWith('.md')) + .map(f => f.replace(/\.md$/, '')); +} + +function processDir(dir, cmdNames) { + const pattern = buildPattern(cmdNames); + if (!pattern) return; let entries; try { entries = fs.readdirSync(dir, { withFileTypes: true }); } catch { return; } for (const e of entries) { const full = path.join(dir, e.name); if (e.isDirectory()) { - processDir(full); + processDir(full, cmdNames); } else if (EXTENSIONS.has(path.extname(e.name))) { const src = fs.readFileSync(full, 'utf-8'); - const replaced = src.replace(pattern, (_, cmd) => `/gsd:${cmd}`); + const replaced = transformContent(src, cmdNames); if (replaced !== src) { fs.writeFileSync(full, replaced, 'utf-8'); const count = (src.match(pattern) || []).length; @@ -45,8 +70,12 @@ function processDir(dir) { } } -let totalFiles = 0; -for (const dir of SEARCH_DIRS) { - processDir(dir); +if (require.main === module) { + const cmdNames = readCmdNames(); + for (const dir of SEARCH_DIRS) { + processDir(dir, cmdNames); + } + console.log('Done.'); } -console.log('Done.'); + +module.exports = { transformContent, buildPattern }; diff --git a/tests/autonomous-interactive.test.cjs b/tests/autonomous-interactive.test.cjs index f111895a0..4873ecd99 100644 --- a/tests/autonomous-interactive.test.cjs +++ b/tests/autonomous-interactive.test.cjs @@ -38,10 +38,40 @@ describe('autonomous --interactive flag (#1413)', () => { }); test('workflow uses discuss-phase skill in interactive mode', () => { + // Per #2697 the user-facing form is the hyphen invariant gsd-discuss-phase; + // the colon form was retired and is enforced absent by bug-2543 tests. + // + // Don't `.includes()` against the full file — both tokens could appear in + // unrelated sections (e.g. INTERACTIVE="" initialization + a stray + // gsd-discuss-phase mention in prose) and falsely pass. Instead, isolate + // the structural region that gates on INTERACTIVE and assert the Skill + // invocation lives inside it. const content = fs.readFileSync(workflowPath, 'utf8'); + const interactiveMarker = '**If `INTERACTIVE` is set:**'; + const branchStart = content.indexOf(interactiveMarker); + assert.notStrictEqual( + branchStart, -1, + `workflow must define an explicit '${interactiveMarker}' branch`, + ); + // Bound the branch by the next "**If `..." prose marker (the non-interactive + // sibling) or, failing that, the next ``/`` boundary. + const afterStart = branchStart + interactiveMarker.length; + const candidates = [ + content.indexOf('**If `INTERACTIVE` is NOT set', afterStart), + content.indexOf('**If `', afterStart), + content.indexOf('', afterStart), + content.indexOf(' i !== -1); + assert.ok(candidates.length > 0, 'INTERACTIVE branch must have a closing boundary'); + const branchEnd = Math.min(...candidates); + const branch = content.slice(branchStart, branchEnd); + + // The branch must invoke the hyphen-form Skill. Tolerate whitespace + // around `(`, `skill`, and `=` so harmless reformatting doesn't break this. + const skillCall = /Skill\(\s*skill\s*=\s*['"]gsd-discuss-phase['"]/.test(branch); assert.ok( - content.includes('gsd:discuss-phase') && content.includes('INTERACTIVE'), - 'workflow should invoke gsd:discuss-phase when INTERACTIVE is set' + skillCall, + `INTERACTIVE branch must invoke Skill(skill="gsd-discuss-phase"). Got branch:\n${branch}`, ); }); diff --git a/tests/bug-2543-gsd-slash-namespace.test.cjs b/tests/bug-2543-gsd-slash-namespace.test.cjs index 0a34af800..08dcbecbf 100644 --- a/tests/bug-2543-gsd-slash-namespace.test.cjs +++ b/tests/bug-2543-gsd-slash-namespace.test.cjs @@ -1,5 +1,7 @@ 'use strict'; +// allow-test-rule: structural-regression-guard + /** * Slash-command namespace invariant (#2543, updated by #2697). * @@ -14,8 +16,8 @@ * * Invariant enforced here: * No `/gsd:` pattern in user-facing source text. - * `Skill(skill="gsd:")` calls (no leading slash) are ALLOWED — they use - * frontmatter `name:` resolution internally and are not user-typed commands. + * `Skill(skill="gsd:")` calls are checked by the skill frontmatter + * parity tests and should use `Skill(skill="gsd-")`. * * Exceptions: * - CHANGELOG.md: historical entries document commands under their original names. @@ -39,6 +41,45 @@ const SEARCH_DIRS = [ COMMANDS_DIR, ]; +// Discover user-facing markdown surfaces dynamically so a freshly added +// doc (a new RELEASE-*.md, a new top-level guide) is automatically scanned +// for namespace drift. A hand-curated list silently weakens drift detection +// over time — every time a doc is added, someone has to remember to extend +// the list, and the failure mode is invisible: the test passes but doesn't +// actually inspect the new file. We scan every .md under docs/ plus +// README.md at the repo root. +function discoverDocSearchFiles(root) { + const out = []; + const readme = path.join(root, 'README.md'); + if (fs.existsSync(readme)) out.push(readme); + // Walk docs/ recursively. Localized translation trees (docs/ja-JP/, + // docs/zh-CN/, docs/ko-KR/, docs/pt-BR/) and nested doc collections + // (docs/skills/, docs/superpowers/) all carry user-facing markdown that + // can drift; a top-level-only scan would silently exclude them. Iterative + // stack walk avoids recursion limits on deep trees. + const stack = [path.join(root, 'docs')]; + while (stack.length > 0) { + const dir = stack.pop(); + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + continue; + } + for (const entry of entries) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + stack.push(full); + } else if (entry.isFile() && entry.name.endsWith('.md')) { + out.push(full); + } + } + } + return out.sort(); +} + +const DOC_SEARCH_FILES = discoverDocSearchFiles(ROOT); + const EXTENSIONS = new Set(['.md', '.cjs', '.js']); function collectFiles(dir, results = []) { @@ -62,6 +103,7 @@ const cmdNames = fs.readdirSync(COMMANDS_DIR) const retiredPattern = new RegExp(`/gsd:(${cmdNames.join('|')})(?=[^a-zA-Z0-9_-]|$)`); const allFiles = SEARCH_DIRS.flatMap(d => collectFiles(d)); +const allUserFacingFiles = allFiles.concat(DOC_SEARCH_FILES.filter((file) => fs.existsSync(file))); describe('slash-command namespace invariant (#2697)', () => { test('commands/gsd/ directory contains known command files', () => { @@ -72,7 +114,7 @@ describe('slash-command namespace invariant (#2697)', () => { test('no /gsd: retired syntax in user-facing source files', () => { const violations = []; - for (const file of allFiles) { + for (const file of allUserFacingFiles) { const src = fs.readFileSync(file, 'utf-8'); const lines = src.split('\n'); for (let i = 0; i < lines.length; i++) { @@ -88,6 +130,58 @@ describe('slash-command namespace invariant (#2697)', () => { ); }); + test('command filenames use canonical hyphenated command slugs', () => { + const underscoreFiles = fs.readdirSync(COMMANDS_DIR) + .filter((f) => f.endsWith('.md') && f.includes('_')); + assert.deepStrictEqual( + underscoreFiles, + [], + 'command filenames feed generated skill/autocomplete names and must not contain underscores', + ); + }); + + describe('fix-slash-commands transformer behavior', () => { + const { transformContent } = require(path.join(ROOT, 'scripts', 'fix-slash-commands.cjs')); + // Use the live command names so the transformer matches the same surface + // the production CLI rewrites. + const liveCmdNames = cmdNames; + + test('rewrites /gsd: to /gsd-', () => { + const out = transformContent('See /gsd:plan-phase for details.', liveCmdNames); + assert.ok(out.includes('/gsd-plan-phase'), `expected /gsd-plan-phase, got: ${out}`); + assert.ok(!out.includes('/gsd:plan-phase'), `colon form must not survive, got: ${out}`); + }); + + test('rewrites multiple occurrences in one pass', () => { + const out = transformContent('Run /gsd:plan-phase then /gsd:execute-phase.', liveCmdNames); + assert.ok(out.includes('/gsd-plan-phase')); + assert.ok(out.includes('/gsd-execute-phase')); + assert.ok(!out.match(/\/gsd:[a-z]/), `no colon form may remain, got: ${out}`); + }); + + test('does not rewrite canonical hyphen form (idempotent)', () => { + const input = '/gsd-plan-phase is the canonical name.'; + assert.strictEqual(transformContent(input, liveCmdNames), input, + 'transformer must be a no-op when input is already canonical'); + }); + + test('does not rewrite gsd-sdk or gsd-tools (not slash commands)', () => { + // Edge case: even though sdk/tools aren't in cmdNames, defensively check + // that strings like "/gsd:sdk" pass through untouched. + const input = 'Run /gsd:sdk query and /gsd:tools init.'; + assert.strictEqual(transformContent(input, liveCmdNames), input, + 'transformer must leave non-command identifiers alone'); + }); + + test('respects word boundary — does not rewrite /gsd:plan-phase-extra', () => { + // The trailing -extra means this is NOT the plan-phase command. + // The negative lookahead `[^a-zA-Z0-9_-]|$` should prevent the match. + const out = transformContent('/gsd:plan-phase-extra', liveCmdNames); + assert.strictEqual(out, '/gsd:plan-phase-extra', + 'word-boundary lookahead must prevent partial matches'); + }); + }); + test('gsd-sdk and gsd-tools identifiers are not rewritten', () => { for (const file of allFiles) { const src = fs.readFileSync(file, 'utf-8'); diff --git a/tests/bug-2643-skill-frontmatter-name.test.cjs b/tests/bug-2643-skill-frontmatter-name.test.cjs index 662229c76..bd12d54de 100644 --- a/tests/bug-2643-skill-frontmatter-name.test.cjs +++ b/tests/bug-2643-skill-frontmatter-name.test.cjs @@ -40,20 +40,63 @@ function collectFiles(dir, results) { return results; } +/** + * Extract every `Skill(skill="")` invocation as a structured record. + * + * Per project test rigor (`feedback_no_source_grep_tests.md`), this parses + * each call as a unit instead of leaning on a single regex over raw bytes. + * The flow is: + * + * 1. Strip HTML comments so commented-out examples don't count as drift. + * 2. Walk the content for `Skill(` openers; for each, find the matching + * `)` closer (Skill bodies are simple kwarg lists, no nesting). + * 3. Parse the call body for the `skill = "..."` keyword argument. + * Permissive whitespace around the keyword and `=`, permissive + * single/double quoting (with optional `\` escapes from string- + * embedded examples), permissive name body — so malformed drift like + * `Skill(skill="gsd:extract_learnings")` is surfaced rather than + * silently skipped by an over-strict character class. + * + * Returns `[{ name, raw }]` per call. Filtering by namespace (gsd- vs gsd:) + * happens at the call site so the extractor stays neutral. + */ +function extractSkillCalls(content) { + const stripped = content.replace(//g, ''); + const calls = []; + // Body class excludes backslash so the extractor doesn't include an + // escape character that precedes the closing quote in embedded examples + // (e.g. `Skill(skill=\"gsd-plan-phase\", …)` written inside a string + // context). A trailing `\` is permitted on the closing-quote side via the + // optional `\\?` so both `\"` and `"` close the value cleanly. + const argRe = /^\s*skill\s*=\s*\\?(['"])([^'"\\]+)\\?\1/i; + let i = 0; + while (i < stripped.length) { + const open = stripped.indexOf('Skill(', i); + if (open === -1) break; + const close = stripped.indexOf(')', open); + if (close === -1) break; + const body = stripped.slice(open + 'Skill('.length, close); + const match = body.match(argRe); + if (match) calls.push({ name: match[2], raw: stripped.slice(open, close + 1) }); + i = close + 1; + } + return calls; +} + function extractSkillNamesHyphen(content) { - const names = new Set(); - const rx = /Skill\(skill=['"]gsd-([a-z0-9-]+)['"]/gi; - let m; - while ((m = rx.exec(content)) !== null) names.add('gsd-' + m[1]); - return names; + return new Set( + extractSkillCalls(content) + .map((c) => c.name) + .filter((n) => n.startsWith('gsd-')), + ); } function extractSkillNamesColon(content) { - const names = new Set(); - const rx = /Skill\(skill=['"]gsd:([a-z0-9-]+)['"]/gi; - let m; - while ((m = rx.exec(content)) !== null) names.add('gsd:' + m[1]); - return names; + return new Set( + extractSkillCalls(content) + .map((c) => c.name) + .filter((n) => n.startsWith('gsd:')), + ); } describe('skill frontmatter name parity (#2643 / #2808)', () => { diff --git a/tests/bug-2808-skill-hyphen-name.test.cjs b/tests/bug-2808-skill-hyphen-name.test.cjs index 66b78ddb2..1fcf25231 100644 --- a/tests/bug-2808-skill-hyphen-name.test.cjs +++ b/tests/bug-2808-skill-hyphen-name.test.cjs @@ -11,7 +11,7 @@ * #2643. Since then, workflows have been updated to use hyphen form (#2808). * * Fix: skillFrontmatterName() now returns the hyphen form unchanged. - * Four workflow Skill() colon calls updated to hyphen. + * Workflow Skill() colon calls are updated to hyphen. * * This test verifies: * 1. skillFrontmatterName returns hyphen form (not colon). @@ -27,9 +27,10 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); +const { cleanup, createTempDir } = require('./helpers.cjs'); const ROOT = path.join(__dirname, '..'); -const { convertClaudeCommandToClaudeSkill, skillFrontmatterName } = +const { convertClaudeCommandToClaudeSkill, copyCommandsAsClaudeSkills, skillFrontmatterName } = require(path.join(ROOT, 'bin', 'install.js')); const WORKFLOWS_DIR = path.join(ROOT, 'get-shit-done', 'workflows'); @@ -100,7 +101,16 @@ describe('bug-2808: SKILL.md name: uses hyphen form', () => { // Parsing line-by-line is more precise than a multi-line regex // and avoids false positives from incidental matches in prose. for (const line of stripped.split('\n')) { - const colonCallRe = /Skill\(skill=['"]gsd:([a-z0-9-]+)['"]/gi; + // Tolerate whitespace around the parenthesis, the `skill` keyword, + // and the `=` so variants like `Skill( skill = "gsd:foo" )` are still + // flagged. Without the `\s*` allowances, drift slips through this guard. + // + // The local-name capture must be permissive (`[^'"\s)]+`, not + // `[a-z0-9-]+`) — the whole purpose of this guard is to surface + // *malformed* drift, including legacy underscore-form names like + // `gsd:extract_learnings`. A character-class that excludes the very + // characters we need to flag would silently let drift through. + const colonCallRe = /Skill\(\s*skill\s*=\s*\\?['"]gsd:([^'"\s)]+)\\?['"]/gi; let m; while ((m = colonCallRe.exec(line)) !== null) { colonCalls.push(`${path.basename(f)}: Skill(skill="gsd:${m[1]}")`); @@ -113,4 +123,55 @@ describe('bug-2808: SKILL.md name: uses hyphen form', () => { 'deprecated colon-form Skill() calls found — update to gsd-: ' + colonCalls.join(', ') ); }); + + test('generated autocomplete skill surface uses hyphen names without underscores', (t) => { + const tmp = createTempDir('gsd-autocomplete-surface-'); + t.after(() => cleanup(tmp)); + const skillsDir = path.join(tmp, 'skills'); + copyCommandsAsClaudeSkills(COMMANDS_DIR, skillsDir, 'gsd', '$HOME/.claude/', 'claude', true); + + // Don't filter the directory listing by `startsWith('gsd-')` — that + // would silently hide exactly the kind of drift this test exists to + // catch (a `gsd:extract-learnings` colon variant or a bare + // `extract-learnings` without the namespace prefix would never be + // collected, and the loop below would never see them). Capture every + // generated directory and assert the namespace invariants explicitly. + const skillDirs = fs.readdirSync(skillsDir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .sort(); + + assert.ok(skillDirs.length > 0, 'expected generated skill directories under skillsDir'); + for (const dir of skillDirs) { + assert.ok( + dir.startsWith('gsd-'), + `${dir}: generated skill directory must start with the canonical 'gsd-' namespace`, + ); + assert.ok( + !dir.includes(':'), + `${dir}: generated skill directory must not contain the retired colon namespace separator`, + ); + assert.ok( + !dir.includes('_'), + `${dir}: generated skill directory must use hyphens, not underscores`, + ); + } + + assert.ok(skillDirs.includes('gsd-extract-learnings'), 'autocomplete surface must include gsd-extract-learnings'); + assert.ok(!skillDirs.includes('gsd-extract_learnings'), 'autocomplete surface must not include gsd-extract_learnings'); + + for (const skillDir of skillDirs) { + const skillContent = fs.readFileSync(path.join(skillsDir, skillDir, 'SKILL.md'), 'utf-8'); + // Scope the name: lookup to the YAML frontmatter block so a stray + // `name:` line in the body cannot satisfy the assertion. + const fmMatch = skillContent.match(/^---\n([\s\S]*?)\n---/); + assert.ok(fmMatch, `${skillDir}: generated SKILL.md must include frontmatter`); + const nameLine = fmMatch[1].split('\n').find((l) => /^name:\s*/.test(l)); + assert.ok(nameLine, `${skillDir}: generated SKILL.md is missing name: frontmatter`); + const name = nameLine.replace(/^name:\s*/, '').trim(); + assert.ok(name.startsWith('gsd-'), `${skillDir}: autocomplete name must start with gsd-, got ${name}`); + assert.ok(!name.includes(':'), `${skillDir}: autocomplete name must not contain colon, got ${name}`); + assert.ok(!name.includes('_'), `${skillDir}: autocomplete name must not contain underscore, got ${name}`); + } + }); }); diff --git a/tests/commands-doc-parity.test.cjs b/tests/commands-doc-parity.test.cjs index af399d439..6cdab8dd6 100644 --- a/tests/commands-doc-parity.test.cjs +++ b/tests/commands-doc-parity.test.cjs @@ -35,9 +35,7 @@ function mentionedInInventory(slug) { describe('every shipped command is documented somewhere', () => { for (const file of commandFiles) { - // Command files may use `_` in their filename (e.g. extract_learnings.md) - // while the user-facing slash command uses `-` (/gsd-extract-learnings). - const slug = file.replace(/\.md$/, '').replace(/_/g, '-'); + const slug = file.replace(/\.md$/, ''); test(`/gsd-${slug}`, () => { const inCommandsDoc = mentionedInCommandsDoc(slug); const inInventory = mentionedInInventory(slug); diff --git a/tests/extract-learnings.test.cjs b/tests/extract-learnings.test.cjs index b81b4dc12..dbf1db280 100644 --- a/tests/extract-learnings.test.cjs +++ b/tests/extract-learnings.test.cjs @@ -11,12 +11,12 @@ const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); -const COMMAND_PATH = path.join(__dirname, '..', 'commands', 'gsd', 'extract_learnings.md'); +const COMMAND_PATH = path.join(__dirname, '..', 'commands', 'gsd', 'extract-learnings.md'); const WORKFLOW_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'extract_learnings.md'); describe('extract-learnings command', () => { test('command file exists', () => { - assert.ok(fs.existsSync(COMMAND_PATH), 'commands/gsd/extract_learnings.md should exist'); + assert.ok(fs.existsSync(COMMAND_PATH), 'commands/gsd/extract-learnings.md should exist'); }); test('command file has correct name frontmatter', () => {