* fix(#2851): replace bare gsd-tools invocations with absolute path `gsd-tools` is not a published bin entry — package.json declares only get-shit-done-cc and gsd-sdk. The shipped invocation pattern is `node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" <subcommand>`, used by every other workflow file. Two leaked bare invocations: - get-shit-done/workflows/plan-phase.md §13e (gap-analysis) — reported in #2851; gap-analysis silently skipped on every plan-phase run - get-shit-done/workflows/ingest-docs.md §finalize (commit) — caught by the new structural test; ingest-docs commit step was broken Both updated to canonical absolute-path form. Adds tests/bug-2851-workflow-bare-gsd-tools.test.cjs which parses every markdown file under get-shit-done/workflows/, extracts shell-fenced code blocks, tokenizes each line, and asserts no token in command position is the bare string `gsd-tools` (the trailing `.cjs` is a different token). The test also asserts plan-phase.md's gap-analysis call uses the canonical `node …/gsd-tools.cjs` form. Closes #2851 * fix(#2851): catch third bare gsd-tools call in ingest-docs.md init After the first commit, the structural test was strengthened to detect bare `gsd-tools` inside `$(...)` and backtick command-substitution forms. The improved test surfaced a third leak: ingest-docs.md:55: INIT=$(gsd-tools init ingest-docs) Fixed to canonical form INIT=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" init ingest-docs) plus the standard `@file:` handoff line that every other workflow uses when capturing INIT (required by tests/windows-robustness.test.cjs). Updated tests/bug-2801-ingest-docs-handler.test.cjs to match either the bare `gsd-tools init ingest-docs` or canonical `gsd-tools.cjs" init ingest-docs` form — the test's intent is to verify the dispatch handler is wired, not to lock the bare-bin form that #2851 removes. Closes #2851 * test(#2851): tighten ingest-docs and gap-analysis assertions to canonical form CodeRabbit caught two soft assertions in the regression tests: 1. tests/bug-2801: the init-ingest-docs assertion accepted both the legacy bare `gsd-tools` form and the canonical node-path form. Since #2851 is the fix that removes the bare form, the test should only accept the canonical absolute-path invocation. Switched to parsed-bash-block extraction with an anchored regex on the full `node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs"` path. 2. tests/bug-2851: the gap-analysis assertion used two loose .includes()/word-boundary checks. Replaced with a single assert.match() against the full canonical path so non-canonical forms fail. * test(#2851): env-assignment skip accepts lowercase identifiers too CodeRabbit caught: the cmdIdx-skip regex /^[A-Z_][A-Z0-9_]*=/ only matched uppercase variable names, so a line like `tmp=1 gsd-tools init` would tokenize to ['tmp=1','gsd-tools','init'], the regex would fail on 'tmp=1', cmdIdx would stay at 0, and the command-position check would compare 'tmp=1' against 'gsd-tools' — false negative. POSIX shell variable names are [A-Za-z_][A-Za-z0-9_]*. Widen the regex to match the actual lexical rule. Existing uppercase forms still work (FOO=bar gsd-tools); now lowercase forms (tmp=1 gsd-tools) and mixed case forms are also detected.
This commit is contained in:
Tom Boucher
@@ -52,7 +52,8 @@ If `PATH_NOT_FOUND` or `MANIFEST_NOT_FOUND`: display error and exit.
|
||||
Run the init query:
|
||||
|
||||
```bash
|
||||
INIT=$(gsd-tools init ingest-docs)
|
||||
INIT=$(node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" init ingest-docs)
|
||||
if [[ "$INIT" == @file:* ]]; then INIT=$(cat "${INIT#@file:}"); fi
|
||||
```
|
||||
|
||||
Parse `project_exists`, `planning_exists`, `has_git`, `project_path` from INIT.
|
||||
@@ -289,7 +290,8 @@ Preview the merge diff to the user and gate via approve-revise-abort before writ
|
||||
Commit the ingest results:
|
||||
|
||||
```bash
|
||||
gsd-tools commit "docs: ingest {N} docs from {SCAN_PATH} (#2387)" --files \
|
||||
node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" commit \
|
||||
"docs: ingest {N} docs from {SCAN_PATH} (#2387)" --files \
|
||||
.planning/PROJECT.md \
|
||||
.planning/REQUIREMENTS.md \
|
||||
.planning/ROADMAP.md \
|
||||
|
||||
@@ -1438,11 +1438,11 @@ one place before execution begins.
|
||||
```bash
|
||||
POST_PLANNING_GAPS=$(gsd-sdk query config-get workflow.post_planning_gaps --default true 2>/dev/null || echo true)
|
||||
if [ "$POST_PLANNING_GAPS" = "true" ]; then
|
||||
gsd-tools gap-analysis --phase-dir "${PHASE_DIR}"
|
||||
node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" gap-analysis --phase-dir "${PHASE_DIR}"
|
||||
fi
|
||||
```
|
||||
|
||||
(`gsd-tools gap-analysis` reads `.planning/REQUIREMENTS.md`, `${PHASE_DIR}/CONTEXT.md`,
|
||||
(`gsd-tools.cjs gap-analysis` reads `.planning/REQUIREMENTS.md`, `${PHASE_DIR}/CONTEXT.md`,
|
||||
and `${PHASE_DIR}/*-PLAN.md`, then prints a markdown table with one row per
|
||||
REQ-ID and D-ID. Word-boundary matching prevents `REQ-1` from being mistaken for
|
||||
`REQ-10`.)
|
||||
|
||||
@@ -126,11 +126,19 @@ describe('bug-2801: ingest-docs.md workflow calls gsd-tools not gsd-sdk', () =>
|
||||
);
|
||||
});
|
||||
|
||||
test('ingest-docs.md init step uses gsd-tools init ingest-docs', () => {
|
||||
test('ingest-docs.md init step uses canonical node-path gsd-tools.cjs invocation', () => {
|
||||
const content = fs.readFileSync(WORKFLOW_FILE, 'utf-8');
|
||||
const lines = content.split('\n');
|
||||
const initLine = lines.find(l => /gsd-tools\s+init\s+ingest-docs/.test(l));
|
||||
assert.ok(initLine, 'workflow must contain "gsd-tools init ingest-docs"');
|
||||
// Parse fenced bash blocks structurally — do not match raw markdown text.
|
||||
const codeBlockRe = /```bash\n([\s\S]*?)```/g;
|
||||
const bashLines = [...content.matchAll(codeBlockRe)]
|
||||
.flatMap((m) => m[1].split('\n'))
|
||||
.filter((l) => !/^\s*#/.test(l));
|
||||
// Per #2851 the only valid form is the absolute-path node invocation; the
|
||||
// legacy bare `gsd-tools` is the bug being fixed and must not be accepted.
|
||||
const initLine = bashLines.find((l) =>
|
||||
/\bnode\s+["']?\$HOME\/\.claude\/get-shit-done\/bin\/gsd-tools\.cjs["']?\s+init\s+ingest-docs\b/.test(l)
|
||||
);
|
||||
assert.ok(initLine, 'workflow must invoke init ingest-docs via canonical node-path gsd-tools.cjs');
|
||||
});
|
||||
|
||||
test('cmdInitIngestDocs is exported from init.cjs', () => {
|
||||
|
||||
162
tests/bug-2851-workflow-bare-gsd-tools.test.cjs
Normal file
162
tests/bug-2851-workflow-bare-gsd-tools.test.cjs
Normal file
@@ -0,0 +1,162 @@
|
||||
/**
|
||||
* Bug #2851: plan-phase.md §13e calls bare `gsd-tools` — incomplete fix of #2245
|
||||
*
|
||||
* `gsd-tools` is NOT a published bin entry. The shipped invocation pattern is:
|
||||
*
|
||||
* node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" <subcommand> [args]
|
||||
*
|
||||
* Some workflow markdown files leaked the bare `gsd-tools <subcommand>` form,
|
||||
* which fails with `command not found` at runtime.
|
||||
*
|
||||
* This test parses every markdown file in get-shit-done/workflows/ structurally:
|
||||
* it tokenizes the content into fenced code blocks, then on each shell-block
|
||||
* line checks whether `gsd-tools` appears as a bare command (not preceded by
|
||||
* `node `, not part of the filename `gsd-tools.cjs`, not inside a comment).
|
||||
*
|
||||
* Per project rule: this test does NOT use grep/regex .includes() on raw file
|
||||
* content as the assertion surface. Instead, it splits into code-fenced blocks
|
||||
* and tokenizes each line — only command-position tokens count as violations.
|
||||
*/
|
||||
'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 WORKFLOWS_DIR = path.join(__dirname, '..', 'get-shit-done', 'workflows');
|
||||
|
||||
/**
|
||||
* Extract shell-fenced code blocks from a markdown file.
|
||||
* Returns an array of { startLine, lines } where lines are the contents
|
||||
* between the ```bash / ```sh / ```shell fence markers.
|
||||
*/
|
||||
function extractShellBlocks(content) {
|
||||
const allLines = content.split('\n');
|
||||
const blocks = [];
|
||||
let inBlock = false;
|
||||
let blockLang = null;
|
||||
let blockStart = 0;
|
||||
let blockLines = [];
|
||||
|
||||
for (let i = 0; i < allLines.length; i++) {
|
||||
const line = allLines[i];
|
||||
const fenceOpen = line.match(/^```(\w+)?/);
|
||||
if (!inBlock && fenceOpen) {
|
||||
inBlock = true;
|
||||
blockLang = (fenceOpen[1] || '').toLowerCase();
|
||||
blockStart = i + 2; // 1-indexed line number of first content line
|
||||
blockLines = [];
|
||||
continue;
|
||||
}
|
||||
if (inBlock && /^```\s*$/.test(line)) {
|
||||
if (['bash', 'sh', 'shell', 'zsh', ''].includes(blockLang)) {
|
||||
blocks.push({ startLine: blockStart, lines: blockLines });
|
||||
}
|
||||
inBlock = false;
|
||||
blockLang = null;
|
||||
blockLines = [];
|
||||
continue;
|
||||
}
|
||||
if (inBlock) {
|
||||
blockLines.push(line);
|
||||
}
|
||||
}
|
||||
return blocks;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check a single shell-block line for a bare `gsd-tools` command-position token.
|
||||
* Returns true if the line is a violation.
|
||||
*/
|
||||
function lineHasBareGsdTools(line) {
|
||||
// Strip leading whitespace and any prompt prefix ($ , > , # )
|
||||
let l = line.replace(/^\s*[$>]\s*/, '');
|
||||
// Skip pure comment lines
|
||||
if (/^\s*#/.test(l)) return false;
|
||||
// Strip inline comment (# preceded by whitespace, not inside a string)
|
||||
// Conservative: only strip if # appears after whitespace and outside quotes —
|
||||
// we just look for the first ` #` outside of quoted context. For our needs,
|
||||
// splitting on `^[^"']*?(\s#)` is good enough.
|
||||
const hashIdx = l.search(/(?:^|[^"'\w])#/);
|
||||
if (hashIdx > 0) l = l.slice(0, hashIdx);
|
||||
|
||||
// Unwrap command-substitution forms so the substituted command is in
|
||||
// command position. `$(cmd …)` and `` `cmd …` `` both run the inner string
|
||||
// as a fresh command, so a bare `gsd-tools` inside them is just as broken
|
||||
// as one at the start of the line. Iterate until stable for nested forms.
|
||||
let prev;
|
||||
do {
|
||||
prev = l;
|
||||
l = l.replace(/\$\(([^()]*)\)/g, ' $1 ').replace(/`([^`]*)`/g, ' $1 ');
|
||||
} while (l !== prev);
|
||||
|
||||
// Tokenize on whitespace, semicolons, pipes, and && / ||
|
||||
// Then walk tokens — a violation is a token that starts with `gsd-tools`
|
||||
// followed by a word boundary (so `gsd-tools.cjs` does NOT match), and the
|
||||
// preceding token is NOT `node`.
|
||||
const segments = l.split(/(?:\s*(?:&&|\|\||;|\|)\s*)/);
|
||||
for (const seg of segments) {
|
||||
const tokens = seg.trim().split(/\s+/).filter(Boolean);
|
||||
if (tokens.length === 0) continue;
|
||||
// Skip env var assignments at the start (FOO=bar gsd-tools …, tmp=1 gsd-tools …).
|
||||
// POSIX shell variable names are [A-Za-z_][A-Za-z0-9_]*; lowercase is valid.
|
||||
let cmdIdx = 0;
|
||||
while (cmdIdx < tokens.length && /^[A-Za-z_][A-Za-z0-9_]*=/.test(tokens[cmdIdx])) {
|
||||
cmdIdx++;
|
||||
}
|
||||
if (cmdIdx >= tokens.length) continue;
|
||||
const cmd = tokens[cmdIdx];
|
||||
// Match `gsd-tools` exactly (no extension), as command position.
|
||||
if (cmd === 'gsd-tools') return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
describe('bug-2851: workflow files must not call bare `gsd-tools` (#2245 sweep regression)', () => {
|
||||
test('no get-shit-done/workflows/*.md file contains a bare gsd-tools command', () => {
|
||||
const files = fs.readdirSync(WORKFLOWS_DIR).filter((f) => f.endsWith('.md'));
|
||||
assert.ok(files.length > 0, 'expected workflow files to exist');
|
||||
|
||||
const violations = [];
|
||||
for (const f of files) {
|
||||
const full = path.join(WORKFLOWS_DIR, f);
|
||||
const content = fs.readFileSync(full, 'utf-8');
|
||||
const blocks = extractShellBlocks(content);
|
||||
for (const blk of blocks) {
|
||||
for (let i = 0; i < blk.lines.length; i++) {
|
||||
if (lineHasBareGsdTools(blk.lines[i])) {
|
||||
violations.push(`${f}:${blk.startLine + i}: ${blk.lines[i].trim()}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
assert.deepStrictEqual(
|
||||
violations,
|
||||
[],
|
||||
'Bare `gsd-tools` invocations found in workflow shell blocks. ' +
|
||||
'Use `node "$HOME/.claude/get-shit-done/bin/gsd-tools.cjs" <subcommand>` instead.\n' +
|
||||
violations.join('\n'),
|
||||
);
|
||||
});
|
||||
|
||||
test('plan-phase.md §13e gap-analysis uses canonical absolute-path invocation', () => {
|
||||
const planPhase = fs.readFileSync(path.join(WORKFLOWS_DIR, 'plan-phase.md'), 'utf-8');
|
||||
const blocks = extractShellBlocks(planPhase);
|
||||
let foundGapAnalysisCall = false;
|
||||
for (const blk of blocks) {
|
||||
for (const line of blk.lines) {
|
||||
if (/gap-analysis/.test(line) && !/^\s*#/.test(line)) {
|
||||
foundGapAnalysisCall = true;
|
||||
assert.match(
|
||||
line,
|
||||
/\bnode\s+["']?\$HOME\/\.claude\/get-shit-done\/bin\/gsd-tools\.cjs["']?\s+gap-analysis\b/,
|
||||
`gap-analysis call must use canonical absolute-path invocation, got: ${line.trim()}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
assert.ok(foundGapAnalysisCall, 'expected at least one gap-analysis invocation in plan-phase.md');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user