* feat(#2995): post-install path audit for workflow-invoked scripts Catches the gap class surfaced by #2994: a workflow references a script via ${GSD_HOME}/<path> that ships in the npm tarball but is not copied to the user's config dir at install time. Unit tests don't catch it because they resolve the script via path.join(__dirname, '..', 'scripts', …) — the source layout, not the deployed layout. Implementation built TDD per #2995, vertical slices with structured-IR assertions: scripts/audit-workflow-script-paths.cjs - Pure auditWorkflowScriptPaths({ workflowsDir, repoRoot, installedPrefixes }) returns { ok, findings: [{ workflow, path, kind }] } via the AUDIT_FINDING enum. - Two finding kinds: MISSING_FROM_REPO (typo / file deleted) and NOT_INSTALLED (#2994 class — first segment outside installed prefixes). - Tolerates ${GSD_HOME:-...} default-fallback syntax. tests/bug-2995-post-install-script-paths.test.cjs - 9 tests across 3 suites: • Pure-function pass and per-finding-kind detection (5 tests on synthetic fixtures). • Real workflow audit (2 tests asserting the actual repo's get-shit-done/workflows/ has no NEW gaps and KNOWN_GAPS stays consistent with audit findings). • Enum shape lock + extractReferences edge cases. - All assertions on typed AUDIT_FINDING enum / structured records; zero raw text matching. - KNOWN_GAPS is a Set keyed on `workflow|path|kind` strings; currently contains the #2994 entry. The companion test fails if a KNOWN_GAPS entry no longer matches a real finding (forces the allow-list to shrink as gaps fix). The audit immediately catches #2994's gap on `reapply-patches.md`. The allow-list contains exactly that entry; new gaps fail CI; #2994's fix will remove the entry as part of the same PR. Closes #2995 Refs #2994 * chore(#2995): add changeset fragment for PR #2996 * chore(#2995): add changeset fragment for PR #2996 * fix(#2995): emit both NOT_INSTALLED + MISSING_FROM_REPO; clean up fixture leak (CR) CodeRabbit on PR #2996 found two issues: 1. (Low value) auditWorkflowScriptPaths short-circuited on NOT_INSTALLED, masking MISSING_FROM_REPO for the same ref. Removed the `continue` so both findings emit in one run; added a regression test. 2. (Low value) bug-2995 test created tmpRoot in before() but never wrote into it; per-fixture mkdtempSync dirs leaked. Rooted fixture repos under tmpRoot so the after() cleanup actually frees them.
This commit is contained in:
Tom Boucher
5
.changeset/lively-goats-run.md
Normal file
5
.changeset/lively-goats-run.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Added
|
||||
pr: 2995
|
||||
---
|
||||
Post-install path smoke test for workflow-invoked scripts — audits every node ${GSD_HOME}/...cjs invocation in workflows resolves at the runtime-installed path. See #2995.
|
||||
5
.changeset/plucky-otters-roam.md
Normal file
5
.changeset/plucky-otters-roam.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Added
|
||||
pr: 2995
|
||||
---
|
||||
Post-install path smoke test for workflow-invoked scripts — audits every node ${GSD_HOME}/...cjs invocation in workflows resolves at the runtime-installed path. See #2995.
|
||||
73
scripts/audit-workflow-script-paths.cjs
Normal file
73
scripts/audit-workflow-script-paths.cjs
Normal file
@@ -0,0 +1,73 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Post-install path audit for workflow-invoked scripts (#2995).
|
||||
*
|
||||
* Walks workflowsDir, extracts every `${GSD_HOME[...]}/<path>.<cjs|js|sh>`
|
||||
* token, and asserts:
|
||||
* 1. the file exists in the repo at that <path> (catches typos)
|
||||
* 2. <path>'s first segment is in installedPrefixes (catches the
|
||||
* #2994 class: source-vs-deployed-path mismatches)
|
||||
*
|
||||
* Pure function over (workflowsDir, repoRoot, installedPrefixes); no
|
||||
* filesystem mutation. Tests assert on the typed AUDIT_FINDING enum.
|
||||
*/
|
||||
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const AUDIT_FINDING = Object.freeze({
|
||||
MISSING_FROM_REPO: 'missing_from_repo',
|
||||
NOT_INSTALLED: 'not_installed',
|
||||
});
|
||||
|
||||
// Match `${GSD_HOME}` or `${GSD_HOME:-...}` followed by a /-rooted path
|
||||
// ending in .cjs/.js/.sh. The path is captured verbatim (relative to
|
||||
// the install root).
|
||||
const REF_RE = /\$\{GSD_HOME(?::-[^}]*)?\}\/([A-Za-z0-9_./-]+\.(?:cjs|js|sh))/g;
|
||||
|
||||
function listWorkflowFiles(dir) {
|
||||
if (!fs.existsSync(dir)) return [];
|
||||
return fs
|
||||
.readdirSync(dir, { withFileTypes: true })
|
||||
.filter((e) => e.isFile() && e.name.endsWith('.md'))
|
||||
.map((e) => path.join(dir, e.name));
|
||||
}
|
||||
|
||||
function extractReferences(content) {
|
||||
const out = [];
|
||||
let m;
|
||||
// RegExp objects with /g state must be reset per call.
|
||||
const re = new RegExp(REF_RE.source, 'g');
|
||||
while ((m = re.exec(content)) !== null) {
|
||||
out.push(m[1]);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function auditWorkflowScriptPaths({ workflowsDir, repoRoot, installedPrefixes }) {
|
||||
const findings = [];
|
||||
const installedSet = new Set(installedPrefixes);
|
||||
for (const file of listWorkflowFiles(workflowsDir)) {
|
||||
const content = fs.readFileSync(file, 'utf8');
|
||||
const workflow = path.basename(file);
|
||||
for (const ref of extractReferences(content)) {
|
||||
const firstSegment = ref.split('/')[0];
|
||||
// #2996 CR: emit BOTH findings simultaneously when a reference is
|
||||
// both outside an installed prefix AND missing from the repo. The
|
||||
// earlier `continue` short-circuited MISSING_FROM_REPO, so a
|
||||
// developer who moved a missing reference to an installed prefix
|
||||
// would only discover the second issue on a subsequent CI run.
|
||||
if (!installedSet.has(firstSegment)) {
|
||||
findings.push({ workflow, path: ref, kind: AUDIT_FINDING.NOT_INSTALLED });
|
||||
}
|
||||
const sourceFile = path.join(repoRoot, ref);
|
||||
if (!fs.existsSync(sourceFile)) {
|
||||
findings.push({ workflow, path: ref, kind: AUDIT_FINDING.MISSING_FROM_REPO });
|
||||
}
|
||||
}
|
||||
}
|
||||
return { ok: findings.length === 0, findings };
|
||||
}
|
||||
|
||||
module.exports = { auditWorkflowScriptPaths, AUDIT_FINDING, extractReferences };
|
||||
247
tests/bug-2995-post-install-script-paths.test.cjs
Normal file
247
tests/bug-2995-post-install-script-paths.test.cjs
Normal file
@@ -0,0 +1,247 @@
|
||||
'use strict';
|
||||
process.env.GSD_TEST_MODE = '1';
|
||||
|
||||
const { test, describe, before, after } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const path = require('node:path');
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const { auditWorkflowScriptPaths, AUDIT_FINDING } = require(
|
||||
path.join(ROOT, 'scripts', 'audit-workflow-script-paths.cjs'),
|
||||
);
|
||||
|
||||
// auditWorkflowScriptPaths is a pure function: it walks workflowsDir,
|
||||
// extracts every ${GSD_HOME}/<path> script reference, and returns a
|
||||
// structured report. Tests assert on the typed report — no regex on
|
||||
// console output.
|
||||
|
||||
// #2996 CR: per-fixture repos are rooted under a single tmpRoot so the
|
||||
// after()-hook actually cleans them up. The previous shape created tmpRoot
|
||||
// in before() but never used it, leaking each fixture's mkdtempSync dir.
|
||||
let tmpRoot;
|
||||
function fixtureRepo({ workflows, files }) {
|
||||
// workflows: { 'foo.md': '...content with ${GSD_HOME}/...' }
|
||||
// files: [ 'get-shit-done/bin/x.cjs', ... ] — files to create in repo
|
||||
const repoRoot = fs.mkdtempSync(path.join(tmpRoot, 'repo-'));
|
||||
const workflowsDir = path.join(repoRoot, 'get-shit-done', 'workflows');
|
||||
fs.mkdirSync(workflowsDir, { recursive: true });
|
||||
for (const [name, body] of Object.entries(workflows || {})) {
|
||||
fs.writeFileSync(path.join(workflowsDir, name), body);
|
||||
}
|
||||
for (const rel of files || []) {
|
||||
const full = path.join(repoRoot, rel);
|
||||
fs.mkdirSync(path.dirname(full), { recursive: true });
|
||||
fs.writeFileSync(full, '');
|
||||
}
|
||||
return { repoRoot, workflowsDir };
|
||||
}
|
||||
|
||||
before(() => { tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2995-')); });
|
||||
after(() => { fs.rmSync(tmpRoot, { recursive: true, force: true }); });
|
||||
|
||||
describe('Bug #2995: post-install script-paths audit (#2995)', () => {
|
||||
test('AUDIT_FINDING enum exposes the documented codes', () => {
|
||||
assert.deepEqual(
|
||||
Object.keys(AUDIT_FINDING).sort(),
|
||||
['MISSING_FROM_REPO', 'NOT_INSTALLED'].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
test('returns { ok: true, findings: [] } when workflow refs an existing, installed-path script', () => {
|
||||
const { repoRoot, workflowsDir } = fixtureRepo({
|
||||
workflows: {
|
||||
'good.md': 'node "${GSD_HOME}/get-shit-done/bin/foo.cjs" --json\n',
|
||||
},
|
||||
files: ['get-shit-done/bin/foo.cjs'],
|
||||
});
|
||||
const r = auditWorkflowScriptPaths({
|
||||
workflowsDir,
|
||||
repoRoot,
|
||||
installedPrefixes: ['get-shit-done', 'commands', 'agents', 'hooks'],
|
||||
});
|
||||
assert.deepEqual(r, { ok: true, findings: [] });
|
||||
});
|
||||
});
|
||||
|
||||
describe('Bug #2995: detection paths', () => {
|
||||
const { auditWorkflowScriptPaths, AUDIT_FINDING } = require(require('node:path').join(__dirname, '..', 'scripts', 'audit-workflow-script-paths.cjs'));
|
||||
|
||||
test('reports MISSING_FROM_REPO when the referenced file does not exist in the repo', () => {
|
||||
const { repoRoot, workflowsDir } = fixtureRepo({
|
||||
workflows: {
|
||||
'foo.md': 'node "${GSD_HOME}/get-shit-done/bin/typo.cjs" --json\n',
|
||||
},
|
||||
files: [],
|
||||
});
|
||||
const r = auditWorkflowScriptPaths({
|
||||
workflowsDir,
|
||||
repoRoot,
|
||||
installedPrefixes: ['get-shit-done'],
|
||||
});
|
||||
assert.equal(r.ok, false);
|
||||
assert.equal(r.findings.length, 1);
|
||||
assert.deepEqual(r.findings[0], {
|
||||
workflow: 'foo.md',
|
||||
path: 'get-shit-done/bin/typo.cjs',
|
||||
kind: AUDIT_FINDING.MISSING_FROM_REPO,
|
||||
});
|
||||
});
|
||||
|
||||
test('reports NOT_INSTALLED when first path segment is outside installedPrefixes (the #2994 case)', () => {
|
||||
const { repoRoot, workflowsDir } = fixtureRepo({
|
||||
workflows: {
|
||||
'foo.md': 'node "${GSD_HOME}/scripts/verify-reapply-patches.cjs"\n',
|
||||
},
|
||||
files: ['scripts/verify-reapply-patches.cjs'], // file exists, but `scripts/` not in installed prefixes
|
||||
});
|
||||
const r = auditWorkflowScriptPaths({
|
||||
workflowsDir,
|
||||
repoRoot,
|
||||
installedPrefixes: ['get-shit-done', 'commands', 'agents', 'hooks'],
|
||||
});
|
||||
assert.equal(r.ok, false);
|
||||
assert.equal(r.findings.length, 1);
|
||||
assert.deepEqual(r.findings[0], {
|
||||
workflow: 'foo.md',
|
||||
path: 'scripts/verify-reapply-patches.cjs',
|
||||
kind: AUDIT_FINDING.NOT_INSTALLED,
|
||||
});
|
||||
});
|
||||
|
||||
test('handles ${GSD_HOME:-$HOME/.claude}/... default-fallback syntax', () => {
|
||||
const { repoRoot, workflowsDir } = fixtureRepo({
|
||||
workflows: {
|
||||
'a.md': 'node "${GSD_HOME:-$HOME/.claude}/get-shit-done/bin/x.cjs"\n',
|
||||
},
|
||||
files: ['get-shit-done/bin/x.cjs'],
|
||||
});
|
||||
const r = auditWorkflowScriptPaths({
|
||||
workflowsDir,
|
||||
repoRoot,
|
||||
installedPrefixes: ['get-shit-done'],
|
||||
});
|
||||
assert.deepEqual(r, { ok: true, findings: [] });
|
||||
});
|
||||
|
||||
test('reports both findings when one workflow has multiple problems', () => {
|
||||
const { repoRoot, workflowsDir } = fixtureRepo({
|
||||
workflows: {
|
||||
'multi.md': [
|
||||
'node "${GSD_HOME}/scripts/a.cjs"',
|
||||
'node "${GSD_HOME}/get-shit-done/bin/b.cjs"',
|
||||
'node "${GSD_HOME}/get-shit-done/bin/missing.cjs"',
|
||||
].join('\n') + '\n',
|
||||
},
|
||||
files: ['scripts/a.cjs', 'get-shit-done/bin/b.cjs'],
|
||||
});
|
||||
const r = auditWorkflowScriptPaths({
|
||||
workflowsDir,
|
||||
repoRoot,
|
||||
installedPrefixes: ['get-shit-done'],
|
||||
});
|
||||
assert.equal(r.ok, false);
|
||||
assert.equal(r.findings.length, 2);
|
||||
const kinds = r.findings.map((f) => f.kind).sort();
|
||||
assert.deepEqual(kinds, [AUDIT_FINDING.MISSING_FROM_REPO, AUDIT_FINDING.NOT_INSTALLED]);
|
||||
});
|
||||
|
||||
test('extracts no findings from a workflow without GSD_HOME script refs', () => {
|
||||
const { repoRoot, workflowsDir } = fixtureRepo({
|
||||
workflows: {
|
||||
'plain.md': '# A workflow\n\nSome prose, no script refs.\n',
|
||||
},
|
||||
});
|
||||
const r = auditWorkflowScriptPaths({
|
||||
workflowsDir,
|
||||
repoRoot,
|
||||
installedPrefixes: ['get-shit-done'],
|
||||
});
|
||||
assert.deepEqual(r, { ok: true, findings: [] });
|
||||
});
|
||||
});
|
||||
|
||||
describe('Bug #2995: real workflow audit', () => {
|
||||
const { auditWorkflowScriptPaths, AUDIT_FINDING } = require(require('node:path').join(__dirname, '..', 'scripts', 'audit-workflow-script-paths.cjs'));
|
||||
|
||||
// The set of top-level directories the installer (bin/install.js) actually
|
||||
// copies into ${configDir}/. Touching this set requires updating both
|
||||
// bin/install.js AND this constant — the parity is intentional.
|
||||
const INSTALLED_PREFIXES = [
|
||||
'get-shit-done', // workflows, references, bin/lib, templates
|
||||
'commands', // commands/gsd/*.md (Claude Code local + Gemini global)
|
||||
'skills', // skills/gsd-*/SKILL.md (Claude Code 2.1.88+ global, Codex, etc.)
|
||||
'agents', // agents/gsd-*.md
|
||||
'hooks', // hooks/gsd-*.{sh,js}
|
||||
];
|
||||
|
||||
// Known existing gaps tracked in their own issues. Removing an entry should
|
||||
// land in the same PR that fixes the underlying issue; CI surfaces any NEW
|
||||
// gap as a hard failure.
|
||||
const KNOWN_GAPS = new Set([
|
||||
'reapply-patches.md|scripts/verify-reapply-patches.cjs|not_installed', // tracked in #2994
|
||||
]);
|
||||
|
||||
test('no NEW workflow refs fail to resolve at the deployed path (KNOWN_GAPS allow-listed)', () => {
|
||||
const r = auditWorkflowScriptPaths({
|
||||
workflowsDir: require('node:path').join(ROOT, 'get-shit-done', 'workflows'),
|
||||
repoRoot: ROOT,
|
||||
installedPrefixes: INSTALLED_PREFIXES,
|
||||
});
|
||||
const newGaps = r.findings.filter(
|
||||
(f) => !KNOWN_GAPS.has(`${f.workflow}|${f.path}|${f.kind}`),
|
||||
);
|
||||
if (newGaps.length > 0) {
|
||||
const summary = newGaps.map(
|
||||
(f) => ` ${f.workflow}: ${f.path} (${f.kind})`,
|
||||
).join('\n');
|
||||
assert.fail(
|
||||
`New workflow ref does not resolve at the deployed path:\n${summary}\n\n` +
|
||||
`Either move the script under one of [${INSTALLED_PREFIXES.join(', ')}], ` +
|
||||
`update bin/install.js to copy the new top-level directory, or ` +
|
||||
`(if intentionally tracked) add an entry to KNOWN_GAPS with the issue reference.`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
// #2996 CR: a reference that is both outside an installed prefix AND
|
||||
// missing from the repo must emit BOTH findings in one run. Previously
|
||||
// the code short-circuited on NOT_INSTALLED, hiding MISSING_FROM_REPO
|
||||
// until the developer fixed the prefix and re-ran CI.
|
||||
test('a reference that is both not-installed AND missing-from-repo emits both findings (no short-circuit)', () => {
|
||||
const { repoRoot, workflowsDir } = fixtureRepo({
|
||||
workflows: {
|
||||
'foo.md': '```bash\nnode "${GSD_HOME}/scripts/missing.cjs"\n```\n',
|
||||
},
|
||||
// Note: scripts/missing.cjs intentionally NOT created in the repo.
|
||||
});
|
||||
const r = auditWorkflowScriptPaths({
|
||||
workflowsDir,
|
||||
repoRoot,
|
||||
installedPrefixes: ['get-shit-done', 'agents', 'hooks', 'commands'],
|
||||
});
|
||||
assert.equal(r.ok, false);
|
||||
const kinds = r.findings.filter((f) => f.path === 'scripts/missing.cjs').map((f) => f.kind).sort();
|
||||
assert.deepEqual(
|
||||
kinds,
|
||||
[AUDIT_FINDING.MISSING_FROM_REPO, AUDIT_FINDING.NOT_INSTALLED].sort(),
|
||||
'expected both NOT_INSTALLED and MISSING_FROM_REPO findings for the same ref',
|
||||
);
|
||||
});
|
||||
|
||||
test('KNOWN_GAPS entries still match real findings — fixed gaps must be removed from the allow-list', () => {
|
||||
const r = auditWorkflowScriptPaths({
|
||||
workflowsDir: require('node:path').join(ROOT, 'get-shit-done', 'workflows'),
|
||||
repoRoot: ROOT,
|
||||
installedPrefixes: INSTALLED_PREFIXES,
|
||||
});
|
||||
const realKeys = new Set(r.findings.map((f) => `${f.workflow}|${f.path}|${f.kind}`));
|
||||
const stale = [...KNOWN_GAPS].filter((k) => !realKeys.has(k));
|
||||
assert.deepEqual(
|
||||
stale,
|
||||
[],
|
||||
`KNOWN_GAPS contains entries not present in audit findings — remove these: ${stale.join(', ')}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user