From b38ae412438a6f76021f44f52abb961a206b0e8e Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 3 Jun 2026 08:08:34 -0400 Subject: [PATCH] fix(#619): resolve gsd-tools via runtime shim in codebase-drift-gate (#645) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#619): resolve gsd-tools via runtime shim in codebase-drift-gate The post-execution drift check ran the bare PATH binary `gsd-tools verify codebase-drift`. On a shim-only install (gsd-tools.cjs present, `gsd-tools` not on PATH) that exits 127, `2>/dev/null` hides it, and the `|| echo` fallback marks the gate skipped — so codebase-drift detection silently never runs. Non-blocking by contract, so nothing surfaced; it just quietly stopped working. Resolve gsd-tools through the runtime shim launcher (gsd_run) instead. The canonical launcher preamble is now defined once in the always-run drift-check block (the file's first gsd_run block); the conditional auto-remap block reuses gsd_run from the workflow's shared shell scope, keeping the file compliant with the single-canonical-preamble parity invariant (tests/runtime-launcher-parity.test.cjs). This is the same single-preamble pattern established by discuss-phase (#614). Non-blocking is preserved for the drift command's internal failures via the unchanged `|| echo '{"skipped":...}'` fallback. Scope decision (the issue's open question): workflow step-file bash blocks share one shell scope, so the preamble is defined once before the first gsd_run call — matching discuss-phase and enforced by the parity test. Regression test (bug-619-...): contract assertions (gsd_run not bare gsd-tools; single preamble in the drift block; fallback intact) plus a behavioral proof that runs the shipped drift-check block against a shim-only topology and asserts the shim actually executes where the old bare-binary form would have skipped. Red→green verified. Co-Authored-By: Claude Opus 4.8 * chore(#619): add changeset for codebase-drift-gate shim fix Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/silly-orcas-dance.md | 5 + .../steps/codebase-drift-gate.md | 17 ++- .../bug-619-codebase-drift-gate-shim.test.cjs | 141 ++++++++++++++++++ 3 files changed, 161 insertions(+), 2 deletions(-) create mode 100644 .changeset/silly-orcas-dance.md create mode 100644 tests/bug-619-codebase-drift-gate-shim.test.cjs diff --git a/.changeset/silly-orcas-dance.md b/.changeset/silly-orcas-dance.md new file mode 100644 index 000000000..a1b8f0655 --- /dev/null +++ b/.changeset/silly-orcas-dance.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 645 +--- +**Post-execution codebase-drift detection no longer silently disables itself on shim-only installs** — `codebase-drift-gate` now resolves `gsd-tools` through the runtime shim launcher (`gsd_run`) instead of the bare PATH binary, which exited 127 (hidden by `2>/dev/null`) and marked the gate skipped whenever `gsd-tools` wasn't on `PATH`. The gate remains fully non-blocking. diff --git a/gsd-core/workflows/execute-phase/steps/codebase-drift-gate.md b/gsd-core/workflows/execute-phase/steps/codebase-drift-gate.md index 03787afda..51029abcd 100644 --- a/gsd-core/workflows/execute-phase/steps/codebase-drift-gate.md +++ b/gsd-core/workflows/execute-phase/steps/codebase-drift-gate.md @@ -6,7 +6,17 @@ error here MUST fall through and continue to `verify_phase_goal`. The phase is never failed by this gate. ```bash -DRIFT=$(gsd-tools verify codebase-drift 2>/dev/null || echo '{"skipped":true,"reason":"sdk-failed"}') +# Resolve gsd-tools through the runtime shim launcher, NOT the bare PATH binary. On a +# shim-only install (gsd-tools.cjs present, `gsd-tools` not on PATH) the bare call exits +# 127, `2>/dev/null` hides it, and this non-blocking gate would silently skip drift +# detection forever (#619). The canonical launcher preamble is defined once here — the +# always-run drift check, the file's first launcher block — and the conditional auto-remap +# block below reuses the launcher function from this shared shell scope (the single-preamble +# pattern established by discuss-phase #614, enforced by tests/runtime-launcher-parity.test.cjs). +# Non-blocking is preserved: an internal drift-command failure still falls through to the +# skip JSON via the `|| echo` below. +_GSD_SHIM_NAME="gsd-tools.cjs"; _GSD_RUNTIME_ROOT="${RUNTIME_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"; GSD_TOOLS="${_GSD_RUNTIME_ROOT}/gsd-core/bin/${_GSD_SHIM_NAME}"; if [ -f "$GSD_TOOLS" ]; then gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif command -v gsd-tools >/dev/null 2>&1; then GSD_TOOLS="$(command -v gsd-tools)"; gsd_run() { "$GSD_TOOLS" "$@"; }; elif [ -f "$HOME/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="$HOME/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; else echo "ERROR: gsd-tools.cjs not found at $GSD_TOOLS and gsd-tools is not on PATH. Run: npx -y @opengsd/gsd-core@latest --claude --local" >&2; exit 1; fi +DRIFT=$(gsd_run verify codebase-drift 2>/dev/null || echo '{"skipped":true,"reason":"sdk-failed"}') ``` Parse JSON for: `skipped`, `reason`, `action_required`, `directive`, @@ -45,7 +55,10 @@ First load the mapper agent's skill bundle (the executor's `AGENT_SKILLS` from step `init_context` is for `gsd-executor`, not the mapper): ```bash -_GSD_SHIM_NAME="gsd-tools.cjs"; _GSD_RUNTIME_ROOT="${RUNTIME_DIR:-$(git rev-parse --show-toplevel 2>/dev/null || pwd)}"; GSD_TOOLS="${_GSD_RUNTIME_ROOT}/gsd-core/bin/${_GSD_SHIM_NAME}"; if [ -f "$GSD_TOOLS" ]; then gsd_run() { node "$GSD_TOOLS" "$@"; }; elif [ -f "${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="${_GSD_RUNTIME_ROOT}/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; elif command -v gsd-tools >/dev/null 2>&1; then GSD_TOOLS="$(command -v gsd-tools)"; gsd_run() { "$GSD_TOOLS" "$@"; }; elif [ -f "$HOME/.claude/gsd-core/bin/${_GSD_SHIM_NAME}" ]; then GSD_TOOLS="$HOME/.claude/gsd-core/bin/${_GSD_SHIM_NAME}"; gsd_run() { node "$GSD_TOOLS" "$@"; }; else echo "ERROR: gsd-tools.cjs not found at $GSD_TOOLS and gsd-tools is not on PATH. Run: npx -y @opengsd/gsd-core@latest --claude --local" >&2; exit 1; fi +# gsd_run is defined by the canonical preamble in the drift-check block above and reused +# here via the workflow's shared shell scope — defining it once keeps the file compliant +# with the single-canonical-preamble parity invariant (#619). This block only runs on the +# `auto-remap` directive, which is always reached after the drift check above has run. AGENT_SKILLS_MAPPER=$(gsd_run query agent-skills gsd-codebase-mapper) ``` diff --git a/tests/bug-619-codebase-drift-gate-shim.test.cjs b/tests/bug-619-codebase-drift-gate-shim.test.cjs new file mode 100644 index 000000000..55f9a33f8 --- /dev/null +++ b/tests/bug-619-codebase-drift-gate-shim.test.cjs @@ -0,0 +1,141 @@ +// allow-test-rule: source-text-is-the-product +// codebase-drift-gate.md is the shipped orchestration step contract. Bug #619: +// the initial drift check ran the bare PATH binary `gsd-tools verify codebase-drift`. +// On a shim-only install (gsd-tools.cjs present, `gsd-tools` not on PATH) that exits +// 127, `2>/dev/null` hides it, and the `|| echo` fallback marks the gate skipped — +// so post-execution drift detection silently never runs. The fix resolves gsd-tools +// through the runtime shim launcher (gsd_run), defining the canonical preamble once in +// this always-run block so the file stays compliant with the single-preamble parity +// invariant (the conditional auto-remap block reuses the launcher via shared shell scope). +// +// This file locks the source contract AND behaviorally proves the shim resolves: it runs +// the exact shipped drift-check block against a shim-only topology and asserts the shim +// actually executes, where the old bare-binary form would have skipped. + +'use strict'; + +const { test, describe } = 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 { execFileSync } = require('node:child_process'); +const { cleanup } = require('./helpers.cjs'); + +const GATE_MD = path.join( + __dirname, '..', 'gsd-core', 'workflows', 'execute-phase', 'steps', 'codebase-drift-gate.md', +); +const SNIPPET_FILE = path.join(__dirname, '..', 'gsd-core', 'workflows', '_runtime-launcher.snippet.sh'); + +function readGate() { + return fs.readFileSync(GATE_MD, 'utf8'); +} + +// Extract the Nth (0-based) ```bash fenced block body from the file. +function bashBlock(content, n) { + const blocks = []; + const re = /```bash\r?\n([\s\S]*?)```/g; + let m; + while ((m = re.exec(content)) !== null) blocks.push(m[1]); + assert.ok(blocks.length > n, `expected at least ${n + 1} bash blocks, found ${blocks.length}`); + return blocks[n]; +} + +describe('bug #619 — codebase-drift-gate resolves gsd-tools via the runtime shim, not the bare PATH binary', () => { + test('codebase-drift-gate.md is readable', () => { + assert.ok(readGate().length > 0, 'codebase-drift-gate.md must not be empty'); + }); + + // ── Source contract (the .md is the product) ────────────────────────────── + + test('the drift check resolves gsd-tools via the shim launcher (gsd_run), not the bare binary (#619)', () => { + const content = readGate(); + assert.match( + content, + /DRIFT=\$\(gsd_run verify codebase-drift 2>\/dev\/null \|\| echo '\{"skipped":true,"reason":"sdk-failed"\}'\)/, + 'drift check must call `gsd_run verify codebase-drift` with the non-blocking skip fallback', + ); + assert.doesNotMatch( + content, + /\bgsd-tools verify codebase-drift\b/, + 'the bare `gsd-tools verify codebase-drift` PATH-binary call (the #619 bug) must be gone', + ); + }); + + test('non-blocking contract preserved: the skip JSON fallback is intact (#619)', () => { + const content = readGate(); + assert.match( + content, + /\|\| echo '\{"skipped":true,"reason":"sdk-failed"\}'/, + 'an internal drift-command failure must still fall through to the skip JSON', + ); + }); + + test('exactly one canonical launcher preamble, in the drift-check block, before any launcher call (#619)', () => { + const content = readGate(); + const snippet = fs.readFileSync(SNIPPET_FILE, 'utf8').replace(/\n$/, ''); + + // Count canonical preamble occurrences across the whole file (parity: exactly one). + let count = 0; + let pos = 0; + for (;;) { + const idx = content.indexOf(snippet, pos); + if (idx === -1) break; + count++; + pos = idx + snippet.length; + } + assert.equal(count, 1, `expected exactly one canonical preamble; found ${count}`); + + // The preamble must live in the first (drift-check) bash block, before the DRIFT call. + const block0 = bashBlock(content, 0); + assert.ok(block0.includes(snippet), 'the canonical preamble must be in the drift-check block'); + assert.ok( + block0.indexOf(snippet) < block0.indexOf('gsd_run verify codebase-drift'), + 'the preamble must precede the gsd_run drift call in the same block', + ); + + // The auto-remap block reuses gsd_run but must NOT carry its own preamble. + const content2 = content.slice(content.indexOf('AGENT_SKILLS_MAPPER')); + assert.ok(!content2.includes(snippet), 'the auto-remap block must not re-declare the preamble (single-preamble parity)'); + }); + + // ── Behavioral proof: the shim resolves on a shim-only topology ─────────── + + test('shipped drift-check block runs the shim (gsd-tools.cjs), not skip, on a shim-only install (#619)', () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-619-')); + try { + // Shim-only topology: gsd-tools.cjs present under RUNTIME_DIR; no `gsd-tools` on PATH. + const binDir = path.join(tmp, 'gsd-core', 'bin'); + fs.mkdirSync(binDir, { recursive: true }); + fs.writeFileSync( + path.join(binDir, 'gsd-tools.cjs'), + 'if (process.argv[2] === "verify" && process.argv[3] === "codebase-drift") {\n' + + ' process.stdout.write(JSON.stringify({ action_required: false, sentinel: "SHIM_RAN" }));\n' + + '}\n', + ); + + const block = bashBlock(readGate(), 0) + '\nprintf "%s" "$DRIFT"\n'; + const out = execFileSync('bash', ['-c', block], { + env: { ...process.env, RUNTIME_DIR: tmp }, + encoding: 'utf8', + }); + + assert.match(out, /SHIM_RAN/, 'the drift check must execute the resolved shim, proving gsd_run resolution'); + assert.doesNotMatch(out, /sdk-failed/, 'the gate must NOT silently skip when the shim is present (#619)'); + } finally { + cleanup(tmp); + } + }); + + test('red-proof: the old bare `gsd-tools` form would skip when gsd-tools is not on PATH', () => { + // Documents the #619 bug: the pre-fix bare-binary call, with no `gsd-tools` on PATH, + // hits the 127 → `|| echo` skip path even though the shim (gsd-tools.cjs) exists. + const oldForm = + 'DRIFT=$(gsd-tools verify codebase-drift 2>/dev/null || echo \'{"skipped":true,"reason":"sdk-failed"}\'); printf "%s" "$DRIFT"'; + const out = execFileSync('bash', ['-c', 'export PATH=/nonexistent-empty-path; ' + oldForm], { + env: { ...process.env }, + encoding: 'utf8', + }); + assert.match(out, /sdk-failed/, 'sanity: the bare-binary form skips without gsd-tools on PATH — the bug the fix removes'); + }); +});