* 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 <noreply@anthropic.com> * chore(#619): add changeset for codebase-drift-gate shim fix Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
5
.changeset/silly-orcas-dance.md
Normal file
5
.changeset/silly-orcas-dance.md
Normal file
@@ -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.
|
||||
@@ -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)
|
||||
```
|
||||
|
||||
|
||||
141
tests/bug-619-codebase-drift-gate-shim.test.cjs
Normal file
141
tests/bug-619-codebase-drift-gate-shim.test.cjs
Normal file
@@ -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');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user