* 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` / `<step>` 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 <trekkie@nomorestars.com>
169 lines
7.8 KiB
JavaScript
169 lines
7.8 KiB
JavaScript
/**
|
|
* Extract-Learnings Command & Workflow Tests
|
|
*
|
|
* Validates command file existence, frontmatter correctness, workflow content,
|
|
* 4 learning categories, capture_thought handling, graceful degradation,
|
|
* LEARNINGS.md output, and missing artifact handling.
|
|
*/
|
|
|
|
const { describe, test } = require('node:test');
|
|
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 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');
|
|
});
|
|
|
|
test('command file has correct name frontmatter', () => {
|
|
const content = fs.readFileSync(COMMAND_PATH, 'utf-8');
|
|
assert.ok(content.includes('name: gsd:extract-learnings'), 'Command must have name: gsd:extract-learnings');
|
|
});
|
|
|
|
test('command file has description frontmatter', () => {
|
|
const content = fs.readFileSync(COMMAND_PATH, 'utf-8');
|
|
assert.ok(content.includes('description:'), 'Command must have description frontmatter');
|
|
});
|
|
|
|
test('command file has argument-hint for phase-number', () => {
|
|
const content = fs.readFileSync(COMMAND_PATH, 'utf-8');
|
|
assert.ok(content.includes('argument-hint:'), 'Command must have argument-hint');
|
|
assert.ok(content.includes('<phase-number>'), 'argument-hint must reference <phase-number>');
|
|
});
|
|
|
|
test('command file has allowed-tools list', () => {
|
|
const content = fs.readFileSync(COMMAND_PATH, 'utf-8');
|
|
assert.ok(content.includes('allowed-tools:'), 'Command must have allowed-tools');
|
|
assert.ok(content.includes('Read'), 'allowed-tools must include Read');
|
|
assert.ok(content.includes('Write'), 'allowed-tools must include Write');
|
|
assert.ok(content.includes('Bash'), 'allowed-tools must include Bash');
|
|
assert.ok(content.includes('Grep'), 'allowed-tools must include Grep');
|
|
assert.ok(content.includes('Glob'), 'allowed-tools must include Glob');
|
|
assert.ok(content.includes('Agent'), 'allowed-tools must include Agent');
|
|
});
|
|
|
|
test('command file has type: prompt', () => {
|
|
const content = fs.readFileSync(COMMAND_PATH, 'utf-8');
|
|
assert.ok(content.includes('type: prompt'), 'Command must have type: prompt');
|
|
});
|
|
|
|
test('command references the workflow via execution_context', () => {
|
|
const content = fs.readFileSync(COMMAND_PATH, 'utf-8');
|
|
assert.ok(
|
|
content.includes('workflows/extract_learnings.md'),
|
|
'Command must reference workflows/extract_learnings.md in execution_context'
|
|
);
|
|
});
|
|
});
|
|
|
|
describe('extract-learnings workflow', () => {
|
|
test('workflow file exists', () => {
|
|
assert.ok(fs.existsSync(WORKFLOW_PATH), 'workflows/extract_learnings.md should exist');
|
|
});
|
|
|
|
test('workflow has objective tag', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('<objective>'), 'Workflow must have <objective> tag');
|
|
assert.ok(content.includes('</objective>'), 'Workflow must close <objective> tag');
|
|
});
|
|
|
|
test('workflow has process tag', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('<process>'), 'Workflow must have <process> tag');
|
|
assert.ok(content.includes('</process>'), 'Workflow must close <process> tag');
|
|
});
|
|
|
|
test('workflow has step tags', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('<step name='), 'Workflow must have named step tags');
|
|
assert.ok(content.includes('</step>'), 'Workflow must close step tags');
|
|
});
|
|
|
|
test('workflow has success_criteria tag', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('<success_criteria>'), 'Workflow must have <success_criteria> tag');
|
|
assert.ok(content.includes('</success_criteria>'), 'Workflow must close <success_criteria> tag');
|
|
});
|
|
|
|
test('workflow has critical_rules tag', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('<critical_rules>'), 'Workflow must have <critical_rules> tag');
|
|
assert.ok(content.includes('</critical_rules>'), 'Workflow must close <critical_rules> tag');
|
|
});
|
|
|
|
test('workflow reads required artifacts (PLAN.md and SUMMARY.md)', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('PLAN.md'), 'Workflow must reference PLAN.md');
|
|
assert.ok(content.includes('SUMMARY.md'), 'Workflow must reference SUMMARY.md');
|
|
});
|
|
|
|
test('workflow reads optional artifacts (VERIFICATION.md, UAT.md, STATE.md)', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('VERIFICATION.md'), 'Workflow must reference VERIFICATION.md');
|
|
assert.ok(content.includes('UAT.md'), 'Workflow must reference UAT.md');
|
|
assert.ok(content.includes('STATE.md'), 'Workflow must reference STATE.md');
|
|
});
|
|
|
|
test('workflow extracts all 4 learning categories', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.toLowerCase().includes('decision'), 'Workflow must extract decisions');
|
|
assert.ok(content.toLowerCase().includes('lesson'), 'Workflow must extract lessons');
|
|
assert.ok(content.toLowerCase().includes('pattern'), 'Workflow must extract patterns');
|
|
assert.ok(content.toLowerCase().includes('surprise'), 'Workflow must extract surprises');
|
|
});
|
|
|
|
test('workflow handles capture_thought tool availability', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('capture_thought'), 'Workflow must reference capture_thought tool');
|
|
});
|
|
|
|
test('workflow degrades gracefully when capture_thought is unavailable', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(
|
|
content.includes('graceful') || content.includes('not available') || content.includes('unavailable') || content.includes('fallback'),
|
|
'Workflow must handle graceful degradation when capture_thought is unavailable'
|
|
);
|
|
});
|
|
|
|
test('workflow outputs LEARNINGS.md', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('LEARNINGS.md'), 'Workflow must output LEARNINGS.md');
|
|
});
|
|
|
|
test('workflow handles missing artifacts gracefully', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(
|
|
content.includes('missing') || content.includes('not found') || content.includes('optional'),
|
|
'Workflow must handle missing artifacts'
|
|
);
|
|
});
|
|
|
|
test('workflow includes source attribution for extracted items', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(
|
|
content.includes('source') || content.includes('attribution') || content.includes('Source:'),
|
|
'Workflow must include source attribution for extracted items'
|
|
);
|
|
});
|
|
|
|
test('workflow specifies LEARNINGS.md YAML frontmatter fields', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(content.includes('phase'), 'LEARNINGS.md frontmatter must include phase');
|
|
assert.ok(content.includes('phase_name'), 'LEARNINGS.md frontmatter must include phase_name');
|
|
assert.ok(content.includes('generated'), 'LEARNINGS.md frontmatter must include generated');
|
|
assert.ok(content.includes('missing_artifacts'), 'LEARNINGS.md frontmatter must include missing_artifacts');
|
|
});
|
|
|
|
test('workflow supports overwriting previous LEARNINGS.md on re-run', () => {
|
|
const content = fs.readFileSync(WORKFLOW_PATH, 'utf-8');
|
|
assert.ok(
|
|
content.includes('overwrite') || content.includes('overwrit') || content.includes('replace'),
|
|
'Workflow must support overwriting previous LEARNINGS.md'
|
|
);
|
|
});
|
|
});
|