diff --git a/.changeset/fierce-yaks-glide.md b/.changeset/fierce-yaks-glide.md new file mode 100644 index 000000000..ee8291e7e --- /dev/null +++ b/.changeset/fierce-yaks-glide.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4289 +--- +**`/gsd:progress --do` now routes specific commands before generic keywords, confirms the route before dispatch, and forwards only arguments the target command accepts** — freeform requests like "set up this existing codebase" or "wrap up the spike findings" no longer preempt to the wrong lifecycle command, and no command runs without your confirmation. (#4051) diff --git a/commands/gsd/execute-phase.md b/commands/gsd/execute-phase.md index 1d9e6038f..ba464ad69 100644 --- a/commands/gsd/execute-phase.md +++ b/commands/gsd/execute-phase.md @@ -1,6 +1,6 @@ --- name: gsd:execute-phase -description: Execute all plans in a phase with wave-based parallelization +description: SDD phase execution — execute all plans in a phase with dependency-aware wave parallelization argument-hint: " [--wave N] [--gaps-only] [--interactive] [--tdd]" effort: max allowed-tools: diff --git a/commands/gsd/phase.md b/commands/gsd/phase.md index 2aef20dde..b5e2ba972 100644 --- a/commands/gsd/phase.md +++ b/commands/gsd/phase.md @@ -1,6 +1,6 @@ --- name: gsd:phase -description: CRUD for phases in ROADMAP.md — add, insert, remove, or edit phases +description: Multi-phase management — add, insert, remove, or edit phases in ROADMAP.md (roadmap phase CRUD) argument-hint: "[--insert | --remove | --edit] " allowed-tools: - Read diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 7a2f99664..dd0cc5d90 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -564,6 +564,8 @@ - REQ-DO-02: System MUST map intent to the best matching GSD command - REQ-DO-03: System MUST confirm the routing with the user before executing - REQ-DO-04: System MUST handle project-exists vs no-project contexts differently +- REQ-DO-05: Routing rules MUST order specific operations before the generic keyword rules they shadow (specific-before-generic) +- REQ-DO-06: Dispatch MUST forward only arguments the selected command accepts; the freeform sentence is forwarded only when that command explicitly accepts a freeform task description --- diff --git a/docs/features/freeform-routing.md b/docs/features/freeform-routing.md index 597038e01..b39112531 100644 --- a/docs/features/freeform-routing.md +++ b/docs/features/freeform-routing.md @@ -13,3 +13,5 @@ group: Planning Features - REQ-DO-02: System MUST map intent to the best matching GSD command - REQ-DO-03: System MUST confirm the routing with the user before executing - REQ-DO-04: System MUST handle project-exists vs no-project contexts differently +- REQ-DO-05: Routing rules MUST order specific operations before the generic keyword rules they shadow (specific-before-generic) +- REQ-DO-06: Dispatch MUST forward only arguments the selected command accepts; the freeform sentence is forwarded only when that command explicitly accepts a freeform task description diff --git a/gsd-core/workflows/do.md b/gsd-core/workflows/do.md index 3a4067fed..79d154352 100644 --- a/gsd-core/workflows/do.md +++ b/gsd-core/workflows/do.md @@ -41,23 +41,31 @@ Track whether `.planning/` exists — some routes require it, others don't. **Match intent to command.** -Evaluate `$ARGUMENTS` against these routing rules. Apply the **first matching** rule: +Evaluate `$ARGUMENTS` against these routing rules. Rules are ordered **most-specific first**: apply the **first matching** rule, and never let a generic keyword rule ("set up", "spike", "review") preempt a more specific operation that also matches ("set up this existing codebase", "wrap up the spike findings", "review the changed source code"). | If the text describes... | Route to | Why | |--------------------------|----------|-----| -| Starting a new greenfield project, "set up", "initialize" | `/gsd:new-project` | Needs full project initialization | -| First-time setup for an existing codebase, brownfield onboarding | `/gsd:onboard` | Safe map → docs ingest → project setup sequence | +| First-time setup for an existing codebase, brownfield onboarding, "onboard this codebase" | `/gsd:onboard` | Safe map → docs ingest → project setup sequence | +| Starting a new greenfield project, "set up", "initialize" (no existing codebase named) | `/gsd:new-project` | Needs full project initialization | | Mapping or analyzing an existing codebase map | `/gsd:map-codebase` | Codebase discovery or refresh | | A bug, error, crash, failure, or something broken | `/gsd:debug` | Needs systematic investigation | -| Spiking, "test if", "will this work", "experiment", "prove this out", validate feasibility | `/gsd:spike` | Throwaway experiment to validate feasibility | -| Sketching, "mockup", "what would this look like", "prototype the UI", "design this", explore visual direction | `/gsd:sketch` | Throwaway HTML mockups to explore design | | Wrapping up spikes, "package the spikes", "consolidate spike findings" | `/gsd:spike --wrap-up` | Package spike findings into reusable skill | | Wrapping up sketches, "package the designs", "consolidate sketch findings" | `/gsd:sketch --wrap-up` | Package sketch findings into reusable skill | +| Spiking, "test if", "will this work", "experiment", "prove this out", validate feasibility | `/gsd:spike` | Throwaway experiment to validate feasibility | +| Sketching, "mockup", "what would this look like", "prototype the UI", "design this", explore visual direction | `/gsd:sketch` | Throwaway HTML mockups to explore design | +| Reviewing changed source code for bugs, security issues, or code quality ("code review the changes") | `/gsd:code-review` | Source review of phase-changed files | +| Requesting peer review of phase plans from another AI CLI ("plan review", "review the plan") | `/gsd:review` | Cross-AI plan review | +| Reviewing or hardening implemented UI ("visual audit", "review the UI") | `/gsd:ui-review` | Retroactive 6-pillar visual audit | +| Verifying security mitigations of a completed phase ("security check", "secure phase N") | `/gsd:secure-phase` | Retroactive threat-mitigation verification | +| Auditing milestone completion against original intent ("audit the milestone") | `/gsd:audit-milestone` | Milestone audit against original intent | +| An autonomous audit-to-fix pass ("audit and fix", "audit the repo and fix what it finds") | `/gsd:audit-fix` | Audit-to-fix pipeline | +| Generating or updating project documentation ("update the docs", "documentation update") | `/gsd:docs-update` | Docs verified against the codebase | | Exploring, researching, comparing, or "how does X work" | `/gsd:explore` | Socratic ideation and idea routing | | Discussing vision, "how should X look", brainstorming | `/gsd:discuss-phase` | Needs context gathering | -| A complex task: refactoring, migration, multi-file architecture, system redesign | `/gsd:phase` | Needs a full phase with plan/build cycle | | Planning a specific phase or "plan phase N" | `/gsd:plan-phase` | Direct planning request | -| Executing a phase or "build phase N", "run phase N" | `/gsd:execute-phase` | Direct execution request | +| Executing a phase or "build phase N", "run phase N" (SDD dependency-aware wave execution) | `/gsd:execute-phase` | Direct execution request | +| Adding, inserting, removing, or editing phases in the roadmap ("multi-phase", roadmap phase management) | `/gsd:phase` | Roadmap phase CRUD | +| A complex task: refactoring, migration, multi-file architecture, system redesign | `/gsd:plan-phase` | Needs a full phase with plan/build cycle | | Running all remaining phases automatically | `/gsd:autonomous` | Full autonomous execution | | A review or quality concern about existing work | `/gsd:verify-work` | Needs verification | | Checking progress, status, "where am I" | `/gsd:progress` | Status check | @@ -73,7 +81,7 @@ Evaluate `$ARGUMENTS` against these routing rules. Apply the **first matching** ``` "Refactor the authentication system" could be: -1. /gsd:phase — Full planning cycle (recommended for multi-file refactors) +1. /gsd:plan-phase — Full planning cycle (recommended for multi-file refactors) 2. /gsd:quick — Quick execution (if scope is small and clear) Which approach fits better? @@ -92,12 +100,33 @@ Which approach fits better? ``` + +**Confirm the route before dispatching (REQ-DO-03).** + +Before invoking anything, ask the user to confirm the displayed route via AskUserQuestion: + +``` +Route to {chosen command}? +1. Yes — proceed with {chosen command} (recommended) +2. Choose a different command +3. Cancel — do not dispatch +``` + +- **Yes / proceed:** continue to the dispatch step. +- **Choose a different command:** present the 2-3 next-best routes from the routing table as options and loop back through display + confirm with the new selection. +- **Cancel:** stop. Do not invoke any command. + +**TEXT_MODE:** present the same choices as a plain-text numbered list and ask the user to type their choice number, exactly like other AskUserQuestion calls in this workflow. + + -**Invoke the chosen command.** +**Invoke the chosen command with only the arguments it accepts.** -Run the selected `/gsd-*` command, passing `$ARGUMENTS` as args. +Read the chosen command's frontmatter `argument-hint` (in `commands/gsd/.md`) and forward **only arguments that command accepts**. Do NOT pass the full freeform sentence wholesale. -If the chosen command expects a phase number and one wasn't provided in the text, extract it from context or ask via AskUserQuestion. +- If the command expects a phase number or flags only (e.g. `/gsd:verify-work [phase number]`, `/gsd:plan-phase`, `/gsd:execute-phase`), extract the phase number / flags from the input; if none was provided, extract it from context or ask via AskUserQuestion. Drop the surrounding prose. +- If the command explicitly accepts a freeform task description (e.g. `/gsd:quick`, `/gsd:debug`, `/gsd:spike`, `/gsd:sketch`), forward the relevant portion of `$ARGUMENTS` as the description. +- If the command takes no arguments, invoke it without arguments. After invoking the command, stop. The dispatched command handles everything from here. @@ -110,6 +139,7 @@ After invoking the command, stop. The dispatched command handles everything from - [ ] Ambiguity resolved via user question (if needed) - [ ] Project existence checked for routes that require it - [ ] Routing decision displayed before dispatch -- [ ] Command invoked with appropriate arguments +- [ ] Route confirmed by the user before dispatch (REQ-DO-03), with TEXT_MODE equivalent +- [ ] Command invoked with only the arguments it accepts (argument-hint aware; freeform text only where the command takes a freeform description) - [ ] No work done directly — dispatcher only diff --git a/skills/gsd-execute-phase/SKILL.md b/skills/gsd-execute-phase/SKILL.md index d5bf1392d..c2f5b0614 100644 --- a/skills/gsd-execute-phase/SKILL.md +++ b/skills/gsd-execute-phase/SKILL.md @@ -1,6 +1,6 @@ --- name: gsd-execute-phase -description: "Execute all plans in a phase with wave-based parallelization" +description: "SDD phase execution — execute all plans in a phase with dependency-aware wave parallelization" argument-hint: " [--wave N] [--gaps-only] [--interactive] [--tdd]" allowed-tools: - Read diff --git a/skills/gsd-phase/SKILL.md b/skills/gsd-phase/SKILL.md index 4c5fe7ade..2ce83adbf 100644 --- a/skills/gsd-phase/SKILL.md +++ b/skills/gsd-phase/SKILL.md @@ -1,6 +1,6 @@ --- name: gsd-phase -description: "CRUD for phases in ROADMAP.md — add, insert, remove, or edit phases" +description: "Multi-phase management — add, insert, remove, or edit phases in ROADMAP.md (roadmap phase CRUD)" argument-hint: "[--insert | --remove | --edit] " allowed-tools: - Read diff --git a/tests/do-routing-specificity.test.cjs b/tests/do-routing-specificity.test.cjs new file mode 100644 index 000000000..d951602a1 --- /dev/null +++ b/tests/do-routing-specificity.test.cjs @@ -0,0 +1,200 @@ +// allow-test-rule: source-text-is-the-product (see #4051) +// Workflow .md and command .md files — their text IS what the runtime loads. +// Testing text content tests the deployed contract. Per CONTRIBUTING.md +// exception matrix. + +/** + * GSD Tools Tests - #4051 freeform routing specificity contract + * + * Validates that the `/gsd:progress --do` dispatcher workflow + * (gsd-core/workflows/do.md): + * 1. Orders specific routing rules BEFORE the generic rules they shadow + * (brownfield onboarding before greenfield "set up"; spike/sketch + * wrap-up before generic spike/sketch). + * 2. Routes existing command families distinctly: code review, docs + * update, plan review, audit, UI review, security, SDD phase + * execution, and multi-phase (roadmap phase CRUD) management. + * 3. Confirms the displayed route before dispatch (REQ-DO-03 of the + * freeform-routing feature requirements), with a TEXT_MODE equivalent. + * 4. Forwards only arguments the selected command accepts. + * + * Also validates that the command frontmatter descriptions distinguish + * SDD execution (execute-phase) from multi-phase coordination (phase). + * + * Negative-space guard: generic fallback routes (greenfield setup, new + * spikes/sketches, verify-work quality concerns, debug, quick) MUST keep + * existing after the specific rows — this fix must not over-suppress. + * + * Closes: #4051 + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); + +describe('#4051 freeform routing specificity', () => { + const repoRoot = path.join(__dirname, '..'); + const doMd = () => fs.readFileSync(path.join(repoRoot, 'gsd-core', 'workflows', 'do.md'), 'utf8'); + const commandMd = (name) => + fs.readFileSync(path.join(repoRoot, 'commands', 'gsd', `${name}.md`), 'utf8'); + + test('do.md orders specific routes before the generic rules they shadow', () => { + const content = doMd(); + + // Specific brownfield onboarding BEFORE generic greenfield "set up" — + // "set up this existing codebase" must not first-match the greenfield row. + const onboardIdx = content.indexOf('/gsd:onboard'); + const newProjectIdx = content.indexOf('/gsd:new-project'); + assert.ok(onboardIdx > -1, 'do.md must route to /gsd:onboard'); + assert.ok(newProjectIdx > -1, 'do.md must route to /gsd:new-project'); + assert.ok( + onboardIdx < newProjectIdx, + 'the brownfield onboarding (/gsd:onboard) rule must precede the greenfield (/gsd:new-project) "set up" rule so "set up this existing codebase" matches onboarding first' + ); + + // Spike/sketch wrap-up BEFORE the generic spike/sketch rows — + // "wrap up the spike findings" must not first-match "start a new spike". + const spikeWrapIdx = content.indexOf('/gsd:spike --wrap-up'); + const sketchWrapIdx = content.indexOf('/gsd:sketch --wrap-up'); + assert.ok(spikeWrapIdx > -1, 'do.md must route spike wrap-up to /gsd:spike --wrap-up'); + assert.ok(sketchWrapIdx > -1, 'do.md must route sketch wrap-up to /gsd:sketch --wrap-up'); + const genericSpikeIdx = content.indexOf('| `/gsd:spike` |'); + const genericSketchIdx = content.indexOf('| `/gsd:sketch` |'); + assert.ok(genericSpikeIdx > -1, 'do.md must keep the generic /gsd:spike route'); + assert.ok(genericSketchIdx > -1, 'do.md must keep the generic /gsd:sketch route'); + assert.ok( + spikeWrapIdx < genericSpikeIdx, + 'the spike wrap-up rule must precede the generic spike rule' + ); + assert.ok( + sketchWrapIdx < genericSketchIdx, + 'the sketch wrap-up rule must precede the generic sketch rule' + ); + + // Source review must be routable to /gsd:code-review, not only the + // generic verify-work fallback. + const codeReviewIdx = content.indexOf('/gsd:code-review'); + const verifyWorkIdx = content.indexOf('| `/gsd:verify-work` |'); + assert.ok(codeReviewIdx > -1, 'do.md must route source review to /gsd:code-review'); + assert.ok(verifyWorkIdx > -1, 'do.md must keep the generic /gsd:verify-work route'); + assert.ok( + codeReviewIdx < verifyWorkIdx, + 'the source code review rule must precede the generic verify-work quality rule' + ); + }); + + test('do.md routes existing command families distinctly', () => { + const content = doMd(); + const requiredRoutes = [ + ['/gsd:onboard', 'brownfield onboarding'], + ['/gsd:code-review', 'source code review'], + ['/gsd:review', 'plan review (cross-AI peer review)'], + ['/gsd:docs-update', 'documentation update'], + ['/gsd:audit-milestone', 'milestone audit'], + ['/gsd:audit-fix', 'autonomous audit-to-fix pass'], + ['/gsd:ui-review', 'UI review'], + ['/gsd:secure-phase', 'security verification'], + ['/gsd:execute-phase', 'SDD phase execution'], + ['/gsd:phase', 'multi-phase (roadmap CRUD) management'], + ]; + for (const [route, family] of requiredRoutes) { + assert.ok(content.includes(route), `do.md routing table must cover ${family} via ${route}`); + } + }); + + test('do.md confirms the route before dispatch (REQ-DO-03)', () => { + const content = doMd(); + const displayIdx = content.indexOf(''); + const confirmIdx = content.indexOf(''); + const dispatchIdx = content.indexOf(''); + assert.ok(displayIdx > -1, 'do.md must have a display step'); + assert.ok(dispatchIdx > -1, 'do.md must have a dispatch step'); + assert.ok(confirmIdx > -1, 'do.md must have a confirm step (REQ-DO-03: confirm before executing)'); + assert.ok( + displayIdx < confirmIdx && confirmIdx < dispatchIdx, + 'the confirm step must sit between display and dispatch' + ); + const confirmSlice = content.slice(confirmIdx, dispatchIdx); + assert.ok( + /AskUserQuestion/.test(confirmSlice), + 'the confirm step must ask via AskUserQuestion' + ); + assert.ok( + /TEXT_MODE/.test(confirmSlice), + 'the confirm step must provide a TEXT_MODE numbered-list equivalent' + ); + assert.ok( + /proceed|dispatch/i.test(confirmSlice) && /cancel|stop/i.test(confirmSlice), + 'the confirm step must offer proceed and cancel choices' + ); + }); + + test('do.md forwards only arguments the selected command accepts', () => { + const content = doMd(); + const dispatchIdx = content.indexOf(''); + assert.ok(dispatchIdx > -1, 'do.md must have a dispatch step'); + const dispatchSlice = content.slice(dispatchIdx); + assert.ok( + /argument-hint|accepted argument/i.test(dispatchSlice), + 'the dispatch step must derive arguments from the target command argument-hint / accepted arguments' + ); + assert.ok( + !/passing \$ARGUMENTS as args/.test(dispatchSlice), + 'the dispatch step must not forward $ARGUMENTS wholesale' + ); + assert.ok( + /freeform task description|freeform description/i.test(dispatchSlice), + 'the dispatch step must carve out commands that explicitly accept a freeform task description' + ); + }); + + test('command descriptions distinguish SDD execution and multi-phase coordination', () => { + const executePhase = commandMd('execute-phase'); + const executeDesc = /description:\s*(.+)/.exec(executePhase)?.[1] ?? ''; + assert.ok( + /SDD|spec-driven|specification-driven/i.test(executeDesc) && /wave|parallel|dependency/i.test(executeDesc), + `execute-phase description must name SDD/spec-driven dependency-aware execution, got: "${executeDesc}"` + ); + + const phase = commandMd('phase'); + const phaseDesc = /description:\s*(.+)/.exec(phase)?.[1] ?? ''; + assert.ok( + /multi-phase|phase management|add, insert, remove, or edit phases/i.test(phaseDesc), + `phase description must name multi-phase management, got: "${phaseDesc}"` + ); + }); + + test('do.md keeps generic fallback routes dispatching (no over-suppression)', () => { + const content = doMd(); + const genericRoutes = [ + ['| `/gsd:new-project` |', 'greenfield project setup'], + ['| `/gsd:spike` |', 'new spike'], + ['| `/gsd:sketch` |', 'new sketch'], + ['| `/gsd:verify-work` |', 'generic quality concern fallback'], + ['| `/gsd:debug` |', 'bug investigation'], + ['| `/gsd:quick` |', 'small actionable task'], + ['| `/gsd:explore` |', 'research / how-does-X-work'], + ['| `/gsd:progress` |', 'status check'], + ['| `/gsd:capture` |', 'note capture'], + ]; + for (const [marker, family] of genericRoutes) { + assert.ok(content.includes(marker), `do.md must keep routing ${family} (${marker})`); + } + }); + + test('do.md success criteria require confirmation and argument-aware dispatch', () => { + const content = doMd(); + const criteriaIdx = content.indexOf(''); + assert.ok(criteriaIdx > -1, 'do.md must have success criteria'); + const criteria = content.slice(criteriaIdx); + assert.ok( + /confirm/i.test(criteria), + 'success criteria must require confirming the route before dispatch' + ); + assert.ok( + /argument/i.test(criteria), + 'success criteria must require argument-aware dispatch' + ); + }); +});