From c2ada7e7991b6e1fe4b54acfb42dafda7ae82874 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 1 May 2026 21:13:45 -0400 Subject: [PATCH] feat(#2995): post-install path audit for workflow-invoked scripts (#2996) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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}/ 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. --- .changeset/lively-goats-run.md | 5 + .changeset/plucky-otters-roam.md | 5 + scripts/audit-workflow-script-paths.cjs | 73 ++++++ ...ug-2995-post-install-script-paths.test.cjs | 247 ++++++++++++++++++ 4 files changed, 330 insertions(+) create mode 100644 .changeset/lively-goats-run.md create mode 100644 .changeset/plucky-otters-roam.md create mode 100644 scripts/audit-workflow-script-paths.cjs create mode 100644 tests/bug-2995-post-install-script-paths.test.cjs diff --git a/.changeset/lively-goats-run.md b/.changeset/lively-goats-run.md new file mode 100644 index 000000000..a237fd744 --- /dev/null +++ b/.changeset/lively-goats-run.md @@ -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. diff --git a/.changeset/plucky-otters-roam.md b/.changeset/plucky-otters-roam.md new file mode 100644 index 000000000..a237fd744 --- /dev/null +++ b/.changeset/plucky-otters-roam.md @@ -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. diff --git a/scripts/audit-workflow-script-paths.cjs b/scripts/audit-workflow-script-paths.cjs new file mode 100644 index 000000000..f85d840e2 --- /dev/null +++ b/scripts/audit-workflow-script-paths.cjs @@ -0,0 +1,73 @@ +'use strict'; + +/** + * Post-install path audit for workflow-invoked scripts (#2995). + * + * Walks workflowsDir, extracts every `${GSD_HOME[...]}/.` + * token, and asserts: + * 1. the file exists in the repo at that (catches typos) + * 2. '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 }; diff --git a/tests/bug-2995-post-install-script-paths.test.cjs b/tests/bug-2995-post-install-script-paths.test.cjs new file mode 100644 index 000000000..7fb3345f5 --- /dev/null +++ b/tests/bug-2995-post-install-script-paths.test.cjs @@ -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}/ 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(', ')}`, + ); + }); +});