From 4277f7d7e807583a6b2ffbfc8d890603c29fc9c1 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 2 May 2026 00:29:34 -0400 Subject: [PATCH] fix(#2994): move verify-reapply-patches.cjs to get-shit-done/bin/ so it ships to user installs (#3000) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2994): move verify-reapply-patches.cjs to get-shit-done/bin/ so installer ships it scripts/verify-reapply-patches.cjs (added in #2972 to close the verified-yes-without-checking gap from #2969) shipped in the npm tarball but never reached user installs: bin/install.js copies get-shit-done/ recursively but does not copy the top-level scripts/ directory. Effect: every fresh install hit `Cannot find module …/scripts/verify-reapply-patches.cjs` on Step 5 of /gsd-reapply-patches. The whole point of moving verification out of LLM-driven prose into a deterministic script is undone if the script does not resolve at runtime. Fix: move the script to get-shit-done/bin/verify-reapply-patches.cjs (same pattern as gsd-tools.cjs and other runtime bin scripts that the installer ships) and update reapply-patches.md Step 5 to invoke ${GSD_HOME}/get-shit-done/bin/verify-reapply-patches.cjs. Tests: - bug-2969 SCRIPT path updated to the new location - New bug-2994-verify-reapply-patches-installed-path.test.cjs parses reapply-patches.md into structured invocation records and asserts every node ${GSD_HOME}/... reference lives under get-shit-done/ (the installed tree). Catches future regressions where someone moves a runtime-needed script back to scripts/. Closes #2994 * chore(#2994): add changeset fragment for PR #3000 * chore(#2994): add changeset fragment for PR #3000 * docs(#2994): update verifier-script-location comment to reflect new path (CR) CodeRabbit on PR #3000: the parenthetical at line 278 still said the script ships under scripts/, but this PR moved it to get-shit-done/bin/. Updated the prose to reference the new location and the installer target path. * chore(#3000): drop direct CHANGELOG.md edit; release entry now lives in .changeset/ The changeset-fragment workflow (#2975) renders fragments into CHANGELOG.md at release time. Direct edits to [Unreleased] on each PR caused merge conflicts on every concurrent PR. This commit restores CHANGELOG.md to match origin/main; the release entry for this fix is preserved in the .changeset/*.md fragment(s) on this branch, which the release workflow consolidates. --- .changeset/happy-jays-greet.md | 5 ++ .changeset/jolly-newts-roam.md | 5 ++ .../bin}/verify-reapply-patches.cjs | 0 get-shit-done/workflows/reapply-patches.md | 4 +- .../bug-2969-verify-reapply-patches.test.cjs | 5 +- ...fy-reapply-patches-installed-path.test.cjs | 81 +++++++++++++++++++ ...ug-2995-post-install-script-paths.test.cjs | 6 +- 7 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 .changeset/happy-jays-greet.md create mode 100644 .changeset/jolly-newts-roam.md rename {scripts => get-shit-done/bin}/verify-reapply-patches.cjs (100%) create mode 100644 tests/bug-2994-verify-reapply-patches-installed-path.test.cjs diff --git a/.changeset/happy-jays-greet.md b/.changeset/happy-jays-greet.md new file mode 100644 index 000000000..68f6abb76 --- /dev/null +++ b/.changeset/happy-jays-greet.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2994 +--- +/gsd-reapply-patches Step 5 verifier now resolves at runtime — moved scripts/verify-reapply-patches.cjs to get-shit-done/bin/ which is shipped by the installer. The legacy scripts/ directory is not copied to user installs. See #2994. diff --git a/.changeset/jolly-newts-roam.md b/.changeset/jolly-newts-roam.md new file mode 100644 index 000000000..68f6abb76 --- /dev/null +++ b/.changeset/jolly-newts-roam.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2994 +--- +/gsd-reapply-patches Step 5 verifier now resolves at runtime — moved scripts/verify-reapply-patches.cjs to get-shit-done/bin/ which is shipped by the installer. The legacy scripts/ directory is not copied to user installs. See #2994. diff --git a/scripts/verify-reapply-patches.cjs b/get-shit-done/bin/verify-reapply-patches.cjs similarity index 100% rename from scripts/verify-reapply-patches.cjs rename to get-shit-done/bin/verify-reapply-patches.cjs diff --git a/get-shit-done/workflows/reapply-patches.md b/get-shit-done/workflows/reapply-patches.md index 5938bda3e..9dbe24ae6 100644 --- a/get-shit-done/workflows/reapply-patches.md +++ b/get-shit-done/workflows/reapply-patches.md @@ -275,7 +275,7 @@ Two layered gates. Both must pass before proceeding to cleanup. Run the deterministic verifier script. Do NOT rely solely on the free-text `verified: yes/no` Hunk Verification Table from Step 4 — bug #2969 traced repeated false-positive `verified: yes` reports to that table being filled in without an actual content-presence check. The script performs the check structurally and exits non-zero on any miss. -Run the verifier as a child process (the gsd-tools binary directory is not required — the script ships under `scripts/` in the source repo and is also exposed via the SDK at `sdk/dist/cli.js verify-reapply` when present): +Run the verifier as a child process (the gsd-tools binary directory is not required — the script ships under `get-shit-done/bin/` in the source repo and is installed to `${GSD_HOME}/get-shit-done/bin/`; it is also exposed via the SDK at `sdk/dist/cli.js verify-reapply` when present): ```bash PRISTINE_DIR="${CONFIG_DIR}/gsd-pristine" @@ -295,7 +295,7 @@ VERIFY_ARGS+=(--json) # Node warnings, deprecation notices, or stack traces do not corrupt the # JSON parse downstream. Stderr is preserved on the controlling terminal # for operator visibility. -VERIFY_OUTPUT="$(node "${GSD_HOME}/scripts/verify-reapply-patches.cjs" "${VERIFY_ARGS[@]}")" +VERIFY_OUTPUT="$(node "${GSD_HOME}/get-shit-done/bin/verify-reapply-patches.cjs" "${VERIFY_ARGS[@]}")" VERIFY_STATUS=$? ``` diff --git a/tests/bug-2969-verify-reapply-patches.test.cjs b/tests/bug-2969-verify-reapply-patches.test.cjs index d3b8b28a4..fedd6e2c3 100644 --- a/tests/bug-2969-verify-reapply-patches.test.cjs +++ b/tests/bug-2969-verify-reapply-patches.test.cjs @@ -29,7 +29,10 @@ const path = require('node:path'); const cp = require('node:child_process'); const ROOT = path.join(__dirname, '..'); -const SCRIPT = path.join(ROOT, 'scripts', 'verify-reapply-patches.cjs'); +// Script lives at get-shit-done/bin/ so the installer ships it under +// `${GSD_HOME}/get-shit-done/bin/` (issue #2994). The top-level scripts/ +// directory is not copied to user installs. +const SCRIPT = path.join(ROOT, 'get-shit-done', 'bin', 'verify-reapply-patches.cjs'); const { REASON } = require(SCRIPT); let tmpRoot; diff --git a/tests/bug-2994-verify-reapply-patches-installed-path.test.cjs b/tests/bug-2994-verify-reapply-patches-installed-path.test.cjs new file mode 100644 index 000000000..0acd0cd90 --- /dev/null +++ b/tests/bug-2994-verify-reapply-patches-installed-path.test.cjs @@ -0,0 +1,81 @@ +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +/** + * Bug #2994: scripts/verify-reapply-patches.cjs ships in tarball but is + * not installed at ${GSD_HOME}/scripts/. + * + * Root cause: bin/install.js copies the get-shit-done/ source tree to + * ${configDir}/get-shit-done/ but does NOT copy the top-level scripts/ + * directory. The verifier script lived under scripts/ so /gsd-reapply-patches + * Step 5 hit `Cannot find module …/scripts/verify-reapply-patches.cjs`. + * + * Fix: move the script to get-shit-done/bin/verify-reapply-patches.cjs + * (which IS installed) and update reapply-patches.md to point there. + * + * This test enforces the structural invariant that prevents regression. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.join(__dirname, '..'); +const RUNTIME_SCRIPT_PATH = path.join(ROOT, 'get-shit-done', 'bin', 'verify-reapply-patches.cjs'); +const STALE_SCRIPT_PATH = path.join(ROOT, 'scripts', 'verify-reapply-patches.cjs'); +const REAPPLY_WORKFLOW = path.join(ROOT, 'get-shit-done', 'workflows', 'reapply-patches.md'); + +describe('Bug #2994: verify-reapply-patches.cjs lives at the runtime-installed path', () => { + test('the script exists under get-shit-done/bin/ (installed by copyWithPathReplacement)', () => { + assert.equal(fs.existsSync(RUNTIME_SCRIPT_PATH), true, + `Expected verifier script at ${RUNTIME_SCRIPT_PATH} -- installer copies get-shit-done/ recursively`); + }); + + test('the script does NOT live at the legacy scripts/ path (not installed)', () => { + assert.equal(fs.existsSync(STALE_SCRIPT_PATH), false, + `scripts/ is not copied by installer; verifier must be under get-shit-done/bin/ instead`); + }); + + test('the script is requireable (loads without throwing)', () => { + const mod = require(RUNTIME_SCRIPT_PATH); + assert.equal(typeof mod.REASON, 'object'); + assert.notEqual(mod.REASON, null); + }); +}); + +// Parse reapply-patches.md to extract every `node "${GSD_HOME}/...cjs"` +// invocation as structured records. Assertions go against the parsed +// records, not against the markdown text. +function extractScriptInvocations(markdown) { + const invocations = []; + const re = /node\s+"\$\{GSD_HOME\}\/([^"]+\.cjs)"/g; + let match; + while ((match = re.exec(markdown)) !== null) { + invocations.push({ relPath: match[1] }); + } + return invocations; +} + +describe('Bug #2994: reapply-patches workflow references the runtime-installed path', () => { + test('every node ${GSD_HOME}/... invocation in reapply-patches.md uses an installed runtime path', () => { + const md = fs.readFileSync(REAPPLY_WORKFLOW, 'utf-8'); + const invocations = extractScriptInvocations(md); + assert.ok(invocations.length > 0, 'sanity: expected at least one node ${GSD_HOME}/... invocation in reapply-patches.md'); + + const violations = invocations.filter(inv => !inv.relPath.startsWith('get-shit-done/')); + assert.deepEqual(violations, [], `invocations under non-installed paths: ${JSON.stringify(violations)}`); + }); + + test('reapply-patches.md references the verifier at get-shit-done/bin/verify-reapply-patches.cjs', () => { + const md = fs.readFileSync(REAPPLY_WORKFLOW, 'utf-8'); + const invocations = extractScriptInvocations(md); + const verifierInvocations = invocations.filter(inv => inv.relPath.endsWith('verify-reapply-patches.cjs')); + assert.deepEqual( + verifierInvocations.map(i => i.relPath), + ['get-shit-done/bin/verify-reapply-patches.cjs'], + 'workflow must call the runtime-installed verifier path exactly once', + ); + }); +}); diff --git a/tests/bug-2995-post-install-script-paths.test.cjs b/tests/bug-2995-post-install-script-paths.test.cjs index 7fb3345f5..6ef75ba00 100644 --- a/tests/bug-2995-post-install-script-paths.test.cjs +++ b/tests/bug-2995-post-install-script-paths.test.cjs @@ -179,9 +179,9 @@ describe('Bug #2995: real workflow audit', () => { // 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 - ]); + // (#2994 entry removed: this PR moves verify-reapply-patches.cjs to + // get-shit-done/bin/ which IS an installed prefix, closing the gap.) + const KNOWN_GAPS = new Set(); test('no NEW workflow refs fail to resolve at the deployed path (KNOWN_GAPS allow-listed)', () => { const r = auditWorkflowScriptPaths({