From ec07861228aa43f021bc4c44f09b33af1b1ddf11 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 1 May 2026 11:25:26 -0400 Subject: [PATCH] fix(#2948): wire spike --wrap-up flag dispatch (#2951) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2948): wire spike --wrap-up flag dispatch Add dispatch block to commands/gsd/spike.md so that /gsd-spike --wrap-up routes to the spike-wrap-up workflow instead of silently no-oping. Also add spike-wrap-up.md to execution_context so the runtime can load it, and update both companion references in workflows/spike.md from the deleted /gsd-spike-wrap-up entry-point to /gsd-spike --wrap-up. Fixes #2948 Co-Authored-By: Claude Sonnet 4.6 * test(#2948): rewrite dispatch test using parseFrontmatter + section extraction Replace raw fs.readFileSync + text.includes() / regex assertions with structural parsing: parseFrontmatter extracts the YAML frontmatter fields and _body, extractSection pulls named XML blocks, and parseExecutionContextRefs resolves the @-prefixed workflow references. Assertions now target the argument-hint frontmatter field, the execution_context @-ref list, and the routing text within / sections — not arbitrary substrings in the raw file. Co-Authored-By: Claude Sonnet 4.6 * test(#2948): tighten dispatch assertion to line-level rule check Replace the co-occurrence check (dispatchText.includes('--wrap-up') && dispatchText.includes('spike-wrap-up')) with line-level assertions that parse the section's rules array, find the exact '- If it is `--wrap-up`:' line, verify it includes 'strip the flag' and 'spike-wrap-up', and assert the '- Otherwise:' fallback still routes to the spike workflow. Co-Authored-By: Claude Sonnet 4.6 * test(#2948): anchor parseFrontmatter to line 0 to avoid mid-file --- delimiters parseFrontmatter was scanning the whole file for the first two '---' lines, which can match a mid-document horizontal rule as the opening delimiter. Now requires lines[0].trim() === '---'; returns { _body: content } for files with no frontmatter, and searches for the closing '---' from line 1 onward. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- CHANGELOG.md | 1 + commands/gsd/spike.md | 6 +- .../bug-2948-spike-wrap-up-dispatch.test.cjs | 162 ++++++++++++++++++ 3 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 tests/bug-2948-spike-wrap-up-dispatch.test.cjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 99834457b..8f1c56c15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **Stale deleted command references updated across workflow files** — `help.md`, `do.md`, `settings.md`, `discuss-phase.md`, `new-project.md`, `plan-phase.md`, `spike.md`, and `sketch.md` referenced command names removed in #2790; updated to new consolidated equivalents. (#2950) ### Fixed — 1.40.0-rc.1 +- **`spike --wrap-up` now dispatches correctly** — `/gsd-spike --wrap-up` was silently no-oping because the flag dispatch wiring was omitted when the micro-skill entry point was absorbed in #2790. (#2948) - **`config-get context_window` returns `200000` when key absent** — querying an unset `context_window` previously exited 1 with "Key not found", surfacing a confusing error in planning logs even though the workflow fallback worked correctly. `cmdConfigGet` now consults a `SCHEMA_DEFAULTS` map and returns the documented default (`200000`, exit 0) for absent schema-defaulted keys; unknown absent keys still error as before. (#2943) - **`gap-analysis` now parses non-`REQ-` requirement IDs and ignores traceability table headers** — `parseRequirements()` no longer hard-codes the `REQ-` prefix and now accepts uppercase prefixed IDs such as `TST-01`, `BACK-07`, and `INSP-04`; markdown table header rows (for example `| REQ-ID | ... |`) are excluded so header tokens are not reported as phantom uncovered requirements. Added regression coverage for mixed-prefix REQUIREMENTS files with traceability tables. (#2897) - **Gemini slash commands namespaced as `/gsd:` instead of `/gsd-`** — diff --git a/commands/gsd/spike.md b/commands/gsd/spike.md index edc227426..423cf5474 100644 --- a/commands/gsd/spike.md +++ b/commands/gsd/spike.md @@ -30,6 +30,7 @@ Does not require `/gsd-new-project` — auto-creates `.planning/spikes/` if need @~/.claude/get-shit-done/workflows/spike.md +@~/.claude/get-shit-done/workflows/spike-wrap-up.md @~/.claude/get-shit-done/references/ui-brand.md @@ -47,6 +48,9 @@ Idea: $ARGUMENTS -Execute the spike workflow from @~/.claude/get-shit-done/workflows/spike.md end-to-end. +Parse the first token of $ARGUMENTS: +- If it is `--wrap-up`: strip the flag, execute the spike-wrap-up workflow from @~/.claude/get-shit-done/workflows/spike-wrap-up.md. +- Otherwise: pass all of $ARGUMENTS as the idea to the spike workflow from @~/.claude/get-shit-done/workflows/spike.md end-to-end. + Preserve all workflow gates (prior spike check, decomposition, research, risk ordering, observability assessment, verification, MANIFEST updates, commit patterns). diff --git a/tests/bug-2948-spike-wrap-up-dispatch.test.cjs b/tests/bug-2948-spike-wrap-up-dispatch.test.cjs new file mode 100644 index 000000000..dc23d4173 --- /dev/null +++ b/tests/bug-2948-spike-wrap-up-dispatch.test.cjs @@ -0,0 +1,162 @@ +/** + * Regression test for bug #2948 + * + * `/gsd-spike --wrap-up` was silently no-oping because: + * 1. `commands/gsd/spike.md` listed `--wrap-up` as a flag but had no dispatch block. + * 2. `workflows/spike.md` still referenced the deleted `/gsd-spike-wrap-up` entry-point + * instead of the correct `/gsd-spike --wrap-up` form. + * + * Fix: + * - `commands/gsd/spike.md` now has a dispatch block that routes `--wrap-up` to + * spike-wrap-up.md, and spike-wrap-up.md is listed in execution_context so the + * runtime can find it. + * - `workflows/spike.md` companion references updated from `/gsd-spike-wrap-up` to + * `/gsd-spike --wrap-up`. + */ + +// allow-test-rule: source-text-is-the-product +// commands/gsd/*.md files ARE what the runtime loads — testing their +// frontmatter and section content tests the deployed system-prompt contract. + +'use strict'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const SPIKE_CMD_PATH = path.join(__dirname, '..', 'commands', 'gsd', 'spike.md'); +const SPIKE_WORKFLOW_PATH = path.join(__dirname, '..', 'get-shit-done', 'workflows', 'spike.md'); + +/** + * Parse YAML frontmatter + body from a markdown file. + * Returns a shallow { key: value } map of frontmatter fields plus `_body`. + * Mirrors the parseFrontmatter utility used in enh-2792-namespace-skills.test.cjs. + */ +function parseFrontmatter(content) { + const lines = content.split(/\r?\n/); + + // Frontmatter must start at the very first line; a mid-file '---' is a + // horizontal rule, not a frontmatter delimiter. + if (lines[0]?.trim() !== '---') { + return { _body: content }; + } + + let closeIdx = -1; + for (let i = 1; i < lines.length; i += 1) { + if (lines[i].trim() === '---') { + closeIdx = i; + break; + } + } + assert.ok(closeIdx !== -1, 'frontmatter block must be delimited by --- on its own lines'); + const fm = {}; + for (const line of lines.slice(1, closeIdx)) { + const m = line.match(/^([A-Za-z][A-Za-z0-9_-]*):\s*(.*)$/); + if (!m) continue; + const [, key, raw] = m; + fm[key] = raw.trim().replace(/^["']|["']$/g, ''); + } + fm._body = lines.slice(closeIdx + 1).join('\n'); + return fm; +} + +/** + * Extract the text content of a named XML-like section from a markdown body. + * Returns null if the section is absent. + */ +function extractSection(body, tag) { + const open = `<${tag}>`; + const close = ``; + const start = body.indexOf(open); + const end = body.indexOf(close); + if (start === -1 || end === -1) return null; + return body.slice(start + open.length, end); +} + +/** + * Parse the @-prefixed workflow references out of an execution_context section. + * Returns an array of resolved reference strings (@ stripped). + */ +function parseExecutionContextRefs(section) { + return section + .split(/\r?\n/) + .map(l => l.trim()) + .filter(l => l.startsWith('@')) + .map(l => l.slice(1).trim()); +} + +describe('bug-2948: /gsd-spike --wrap-up dispatch wiring', () => { + describe('commands/gsd/spike.md — frontmatter and section contract', () => { + test('spike.md command file exists and has valid frontmatter', () => { + assert.ok(fs.existsSync(SPIKE_CMD_PATH), 'commands/gsd/spike.md should exist'); + const fm = parseFrontmatter(fs.readFileSync(SPIKE_CMD_PATH, 'utf-8')); + assert.ok(fm.name, 'frontmatter must have a name field'); + }); + + test('argument-hint frontmatter field advertises --wrap-up flag', () => { + const fm = parseFrontmatter(fs.readFileSync(SPIKE_CMD_PATH, 'utf-8')); + assert.ok( + fm['argument-hint'] && fm['argument-hint'].includes('--wrap-up'), + `argument-hint must advertise --wrap-up; got: "${fm['argument-hint']}"` + ); + }); + + test('execution_context section includes spike-wrap-up workflow reference', () => { + const fm = parseFrontmatter(fs.readFileSync(SPIKE_CMD_PATH, 'utf-8')); + const execSection = extractSection(fm._body, 'execution_context'); + assert.ok(execSection !== null, 'spike.md must have an section'); + const refs = parseExecutionContextRefs(execSection); + assert.ok( + refs.some(r => r.includes('spike-wrap-up')), + `execution_context must declare a spike-wrap-up reference so the runtime can load the workflow; ` + + `declared refs: ${JSON.stringify(refs)}` + ); + }); + + test('process section dispatches first-token --wrap-up to spike-wrap-up workflow', () => { + const fm = parseFrontmatter(fs.readFileSync(SPIKE_CMD_PATH, 'utf-8')); + const processSection = extractSection(fm._body, 'process'); + assert.ok(processSection, 'spike.md must have a section'); + + const rules = processSection + .split(/\r?\n/) + .map(line => line.trim()) + .filter(Boolean); + + const wrapUpRule = rules.find(line => line.startsWith('- If it is `--wrap-up`:')); + const fallbackRule = rules.find(line => line.startsWith('- Otherwise:')); + + assert.ok( + wrapUpRule && wrapUpRule.includes('strip the flag') && wrapUpRule.includes('spike-wrap-up'), + 'process must define a --wrap-up branch that strips the flag and routes to spike-wrap-up' + ); + assert.ok( + fallbackRule && fallbackRule.includes('spike workflow'), + 'process must define an Otherwise fallback to the normal spike workflow' + ); + }); + }); + + describe('get-shit-done/workflows/spike.md — companion references', () => { + test('spike workflow file exists', () => { + assert.ok(fs.existsSync(SPIKE_WORKFLOW_PATH), 'get-shit-done/workflows/spike.md should exist'); + }); + + test('does NOT reference the deleted /gsd-spike-wrap-up entry-point', () => { + const fm = parseFrontmatter(fs.readFileSync(SPIKE_WORKFLOW_PATH, 'utf-8')); + assert.ok( + !fm._body.includes('/gsd-spike-wrap-up'), + 'workflows/spike.md must not reference the deleted /gsd-spike-wrap-up command; use /gsd-spike --wrap-up instead' + ); + }); + + test('references /gsd-spike --wrap-up as the canonical wrap-up invocation', () => { + const fm = parseFrontmatter(fs.readFileSync(SPIKE_WORKFLOW_PATH, 'utf-8')); + assert.ok( + fm._body.includes('/gsd-spike --wrap-up'), + 'workflows/spike.md must reference /gsd-spike --wrap-up as the canonical wrap-up command' + ); + }); + }); +});