diff --git a/.changeset/typed-rivers-flow.md b/.changeset/typed-rivers-flow.md new file mode 100644 index 000000000..908b25f0b --- /dev/null +++ b/.changeset/typed-rivers-flow.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 2974 +--- +Migrated 8 test files from raw text matching (`stdout.includes(...)`, `assert.match(stderr, ...)`) to typed-IR assertions per CONTRIBUTING.md. Adds shared `ERROR_REASON` enum and `--json-errors` flag in `core.cjs`, typed `GRAPHIFY_REASON` in `graphify.cjs`, pure `buildSdkFailFastReport()` IR builder in `bin/install.js`, and Claude Code JSON envelope output (`hookSpecificOutput` with typed fields) for `gsd-session-state.sh` and `gsd-phase-boundary.sh`. Tests now assert on structured fields (`reason`, `context`, `state_present`, `planning_modified`, etc.) instead of substring matching. See #2974. diff --git a/bin/install.js b/bin/install.js index 1813274d8..5a3b666ba 100755 --- a/bin/install.js +++ b/bin/install.js @@ -8944,6 +8944,85 @@ function classifySdkInstall(sdkDir) { return { mode, npxCache, readOnly }; } +/** + * #2974: pure builder for the SDK fail-fast report. Returns a structured IR + * with everything the renderer needs PLUS everything tests need to assert + * on. Tests can call `buildSdkFailFastReport(sdkDir, sdkCliPath)` directly + * and assert on `report.reason`, `report.context`, `report.fix_command` + * etc. without intercepting console.error or matching against rendered + * text. + * + * Shape (frozen contract — extending requires a new test): + * { + * ok: false, + * reason: 'sdk_fail_fast', // ERROR_REASON.SDK_FAIL_FAST + * context: 'npx-cache' | 'tarball' | 'dev-clone', + * missing_path: '/sdk/dist/cli.js', + * missing_artifact: 'sdk/dist', + * fix_command: 'npm install -g get-shit-done-cc@latest' | 'cd sdk && npm install && npm run build', + * attempted_nested_install: false, // contract: never true + * } + */ +function buildSdkFailFastReport(sdkDir, sdkCliPath) { + const ctx = classifySdkInstall(sdkDir); + let context, fix_command; + if (ctx.mode === 'tarball') { + context = ctx.npxCache ? 'npx-cache' : 'tarball'; + fix_command = 'npm install -g get-shit-done-cc@latest'; + } else { + context = 'dev-clone'; + fix_command = 'cd sdk && npm install && npm run build'; + } + return { + ok: false, + reason: 'sdk_fail_fast', + context, + missing_path: sdkCliPath, + missing_artifact: 'sdk/dist', + fix_command, + attempted_nested_install: false, + }; +} + +/** + * Renderer for the structured fail-fast report. Text formatting only — + * tests never call this. Splits the IR fields back into the same human- + * readable lines the previous shape produced. + */ +function renderSdkFailFastReport(ir) { + const bar = '━'.repeat(72); + const redBold = `${red}${bold}`; + console.error(''); + console.error(`${redBold}${bar}${reset}`); + console.error(`${redBold} ✗ GSD SDK dist not found — /gsd-* commands will not work${reset}`); + console.error(`${redBold}${bar}${reset}`); + console.error(` ${red}Reason:${reset} ${ir.missing_artifact}/cli.js not found at ${ir.missing_path}`); + console.error(''); + if (ir.context === 'npx-cache') { + console.error(` Detected read-only npx cache install (${dim}${path.dirname(ir.missing_path).replace(/\/dist$/, '')}${reset}).`); + console.error(` The installer will ${bold}not${reset} attempt \`npm install\` inside the npx cache.`); + console.error(''); + console.error(` Fix: install a version that ships sdk/dist/ globally:`); + console.error(` ${cyan}${ir.fix_command}${reset}`); + console.error(` Or, if you prefer a one-shot run, clear the npx cache first:`); + console.error(` ${cyan}npx --yes get-shit-done-cc@latest${reset}`); + console.error(` Or build from source (git clone):`); + console.error(` ${cyan}git clone https://github.com/gsd-build/get-shit-done && cd get-shit-done/sdk && npm install && npm run build${reset}`); + } else if (ir.context === 'tarball') { + console.error(` The published tarball appears to be missing sdk/dist/ (see #2647).`); + console.error(''); + console.error(` Fix: install a version that ships sdk/dist/ globally:`); + console.error(` ${cyan}${ir.fix_command}${reset}`); + console.error(` Or build from source (git clone):`); + console.error(` ${cyan}git clone https://github.com/gsd-build/get-shit-done && cd get-shit-done/sdk && npm install && npm run build${reset}`); + } else { + console.error(` Running from a git clone — build the SDK first:`); + console.error(` ${cyan}${ir.fix_command}${reset}`); + } + console.error(`${redBold}${bar}${reset}`); + console.error(''); +} + function installSdkIfNeeded(opts) { opts = opts || {}; if (hasNoSdk && !opts.sdkDir) { @@ -8971,44 +9050,8 @@ function installSdkIfNeeded(opts) { } if (!fs.existsSync(sdkCliPath)) { - const ctx = classifySdkInstall(sdkDir); - const bar = '━'.repeat(72); - const redBold = `${red}${bold}`; - console.error(''); - console.error(`${redBold}${bar}${reset}`); - console.error(`${redBold} ✗ GSD SDK dist not found — /gsd-* commands will not work${reset}`); - console.error(`${redBold}${bar}${reset}`); - console.error(` ${red}Reason:${reset} sdk/dist/cli.js not found at ${sdkCliPath}`); - console.error(''); - - if (ctx.mode === 'tarball') { - // User install (including `npx get-shit-done-cc@latest`, which stages - // a read-only tarball under the npx cache). The sdk/dist/ artifact - // should ship in the published tarball. If it's missing, the only - // sane fix from the user's side is a fresh global install of a - // version that includes dist/. Do NOT attempt a nested `npm install` - // inside the (read-only) npx cache — that's the #2649 failure mode. - if (ctx.npxCache) { - console.error(` Detected read-only npx cache install (${dim}${sdkDir}${reset}).`); - console.error(` The installer will ${bold}not${reset} attempt \`npm install\` inside the npx cache.`); - console.error(''); - } else { - console.error(` The published tarball appears to be missing sdk/dist/ (see #2647).`); - console.error(''); - } - console.error(` Fix: install a version that ships sdk/dist/ globally:`); - console.error(` ${cyan}npm install -g get-shit-done-cc@latest${reset}`); - console.error(` Or, if you prefer a one-shot run, clear the npx cache first:`); - console.error(` ${cyan}npx --yes get-shit-done-cc@latest${reset}`); - console.error(` Or build from source (git clone):`); - console.error(` ${cyan}git clone https://github.com/gsd-build/get-shit-done && cd get-shit-done/sdk && npm install && npm run build${reset}`); - } else { - // Dev clone: keep the existing build-from-source hint. - console.error(` Running from a git clone — build the SDK first:`); - console.error(` ${cyan}cd sdk && npm install && npm run build${reset}`); - } - console.error(`${redBold}${bar}${reset}`); - console.error(''); + const ir = buildSdkFailFastReport(sdkDir, sdkCliPath); + renderSdkFailFastReport(ir); process.exit(1); } @@ -9370,6 +9413,8 @@ if (process.env.GSD_TEST_MODE) { install, uninstall, installSdkIfNeeded, + buildSdkFailFastReport, + renderSdkFailFastReport, classifySdkInstall, convertClaudeCommandToCodexSkill, convertClaudeToOpencodeFrontmatter, diff --git a/get-shit-done/bin/gsd-tools.cjs b/get-shit-done/bin/gsd-tools.cjs index de5f3e791..6e1c057d8 100755 --- a/get-shit-done/bin/gsd-tools.cjs +++ b/get-shit-done/bin/gsd-tools.cjs @@ -305,6 +305,18 @@ async function main() { const raw = rawIndex !== -1; if (rawIndex !== -1) args.splice(rawIndex, 1); + // --json-errors: when present, error() emits structured JSON to stderr + // ({ ok: false, reason: , message }) instead of plain + // "Error: ". Lets test suites assert on typed reason codes per the + // CONTRIBUTING.md "Prohibited: Raw Text Matching on Test Outputs" rule + // (#2974). Default off — human operators see the original plain-text + // diagnostic. + const jsonErrorsIdx = args.indexOf('--json-errors'); + if (jsonErrorsIdx !== -1) { + core.setJsonErrorMode(true); + args.splice(jsonErrorsIdx, 1); + } + // --pick : extract a single field from JSON output (replaces jq dependency). // Supports dot-notation (e.g., --pick workflow.research) and bracket notation // for arrays (e.g., --pick directories[-1]). diff --git a/get-shit-done/bin/lib/config.cjs b/get-shit-done/bin/lib/config.cjs index 8f13cb05a..13cf2be3e 100644 --- a/get-shit-done/bin/lib/config.cjs +++ b/get-shit-done/bin/lib/config.cjs @@ -4,7 +4,7 @@ const fs = require('fs'); const path = require('path'); -const { output, error, CONFIG_DEFAULTS, atomicWriteFileSync } = require('./core.cjs'); +const { output, error, ERROR_REASON, CONFIG_DEFAULTS, atomicWriteFileSync } = require('./core.cjs'); const { planningDir, withPlanningLock } = require('./planning-workspace.cjs'); const { VALID_PROFILES, @@ -33,7 +33,7 @@ const CONFIG_KEY_SUGGESTIONS = { function validateKnownConfigKeyPath(keyPath) { const suggested = CONFIG_KEY_SUGGESTIONS[keyPath]; if (suggested) { - error(`Unknown config key: ${keyPath}. Did you mean ${suggested}?`); + error(`Unknown config key: ${keyPath}. Did you mean ${suggested}?`, ERROR_REASON.CONFIG_INVALID_KEY); } } @@ -278,7 +278,7 @@ function setConfigValue(cwd, keyPath, parsedValue) { config = JSON.parse(fs.readFileSync(configPath, 'utf-8')); } } catch (err) { - error('Failed to read config.json: ' + err.message); + error('Failed to read config.json: ' + err.message, ERROR_REASON.CONFIG_PARSE_FAILED); } // Set nested value using dot notation (e.g., "workflow.research") @@ -319,7 +319,7 @@ function cmdConfigSet(cwd, keyPath, value, raw) { validateKnownConfigKeyPath(keyPath); if (!isValidConfigKey(keyPath)) { - error(`Unknown config key: "${keyPath}". Valid keys: ${[...VALID_CONFIG_KEYS].sort().join(', ')}, agent_skills., features.`); + error(`Unknown config key: "${keyPath}". Valid keys: ${[...VALID_CONFIG_KEYS].sort().join(', ')}, agent_skills., features.`, ERROR_REASON.CONFIG_INVALID_KEY); } // Parse value (handle booleans, numbers, and JSON arrays/objects) @@ -402,11 +402,11 @@ function cmdConfigGet(cwd, keyPath, raw, defaultValue) { output(defaultValue, raw, String(defaultValue)); return; } else { - error('No config.json found at ' + configPath); + error('No config.json found at ' + configPath, ERROR_REASON.CONFIG_NO_FILE); } } catch (err) { if (err.message.startsWith('No config.json')) throw err; - error('Failed to read config.json: ' + err.message); + error('Failed to read config.json: ' + err.message, ERROR_REASON.CONFIG_PARSE_FAILED); } // Traverse dot-notation path (e.g., "workflow.auto_advance") @@ -420,7 +420,7 @@ function cmdConfigGet(cwd, keyPath, raw, defaultValue) { output(def, raw, String(def)); return; } - error(`Key not found: ${keyPath}`); + error(`Key not found: ${keyPath}`, ERROR_REASON.CONFIG_KEY_NOT_FOUND); } current = current[key]; } @@ -432,7 +432,7 @@ function cmdConfigGet(cwd, keyPath, raw, defaultValue) { output(def, raw, String(def)); return; } - error(`Key not found: ${keyPath}`); + error(`Key not found: ${keyPath}`, ERROR_REASON.CONFIG_KEY_NOT_FOUND); } // Never echo plaintext for sensitive keys via config-get. Plaintext lives diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index c80125b28..c3cb1862e 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -201,8 +201,68 @@ function output(result, raw, rawValue) { fs.writeSync(1, data); } -function error(message) { - fs.writeSync(2, 'Error: ' + message + '\n'); +/** + * Frozen enum of typed reason codes used by error() for structured errors. + * Each subcommand contributes its own codes; the enum exists so tests can + * assert against typed values instead of grepping stderr (#2974). + * + * Adding a new code: + * - Pick a snake_case lowercase value (the JSON wire form) + * - Group by subsystem prefix (CONFIG_*, SDK_*, etc) + * - Pass it to error(msg, ERROR_REASON.NEW_CODE) at the call site + */ +const ERROR_REASON = Object.freeze({ + // config-get / config-set + CONFIG_KEY_NOT_FOUND: 'config_key_not_found', + CONFIG_NO_FILE: 'config_no_file', + CONFIG_PARSE_FAILED: 'config_parse_failed', + CONFIG_INVALID_KEY: 'config_invalid_key', + // SDK / gsd-tools dispatch + SDK_FAIL_FAST: 'sdk_fail_fast', + SDK_UNKNOWN_COMMAND: 'sdk_unknown_command', + SDK_MISSING_ARG: 'sdk_missing_arg', + // workflow / phase + PHASE_NOT_FOUND: 'phase_not_found', + SUMMARY_NO_PLANNING: 'summary_no_planning', + // graphify + GRAPHIFY_NO_GRAPH: 'graphify_no_graph', + GRAPHIFY_INVALID_QUERY: 'graphify_invalid_query', + // hooks + HOOKS_OPT_OUT: 'hooks_opt_out', + // security-scan + SECURITY_SCAN_FAILED: 'security_scan_failed', + // generic + USAGE: 'usage', + UNKNOWN: 'unknown', +}); + +/** + * Process-level flag: when true, error() emits structured JSON to stderr + * instead of plain "Error: " text. Set by gsd-tools.cjs when the + * CLI is invoked with `--json-errors`. Tests opt in to typed-IR error + * assertions by passing that flag and parsing the JSON. + * + * Default off so existing callers and human operators keep their plain-text + * diagnostics. The structured form is opt-in for tooling and tests (#2974). + */ +let _jsonErrorMode = false; +function setJsonErrorMode(v) { _jsonErrorMode = !!v; } +function getJsonErrorMode() { return _jsonErrorMode; } + +/** + * Emit an error and exit. When the second argument is provided it must be + * a value from ERROR_REASON; tests can assert on `result.reason`. When the + * process is in JSON-error mode, stderr receives `{ ok: false, reason, + * message }` so callers can parse it; otherwise stderr keeps the plain + * text form for human operators. + */ +function error(message, reason = ERROR_REASON.UNKNOWN) { + if (_jsonErrorMode) { + const payload = JSON.stringify({ ok: false, reason, message }) + '\n'; + fs.writeSync(2, payload); + } else { + fs.writeSync(2, 'Error: ' + message + '\n'); + } process.exit(1); } @@ -1816,6 +1876,9 @@ function timeAgo(date) { module.exports = { output, error, + ERROR_REASON, + setJsonErrorMode, + getJsonErrorMode, safeReadFile, loadConfig, isGitIgnored, diff --git a/get-shit-done/bin/lib/graphify.cjs b/get-shit-done/bin/lib/graphify.cjs index a31e0fa2f..0c375a944 100644 --- a/get-shit-done/bin/lib/graphify.cjs +++ b/get-shit-done/bin/lib/graphify.cjs @@ -45,6 +45,17 @@ function disabledResponse() { * @param {{ timeout?: number }} [options={}] - Options (timeout in ms, default 30000) * @returns {{ exitCode: number, stdout: string, stderr: string }} */ +/** + * Frozen enum of typed reason codes for execGraphify failures (#2974). + * Tests assert on result.reason instead of grepping stderr text. + */ +const GRAPHIFY_REASON = Object.freeze({ + OK: 'ok', + ENOENT: 'graphify_not_found', + TIMEOUT: 'graphify_timed_out', + EXIT_NONZERO: 'graphify_exit_nonzero', +}); + function execGraphify(cwd, args, options = {}) { const timeout = options.timeout ?? 30000; const result = childProcess.spawnSync('graphify', args, { @@ -57,7 +68,12 @@ function execGraphify(cwd, args, options = {}) { // ENOENT -- graphify binary not found on PATH if (result.error && result.error.code === 'ENOENT') { - return { exitCode: 127, stdout: '', stderr: 'graphify not found on PATH' }; + return { + exitCode: 127, + stdout: '', + stderr: 'graphify not found on PATH', + reason: GRAPHIFY_REASON.ENOENT, + }; } // Timeout -- subprocess killed via SIGTERM @@ -66,13 +82,17 @@ function execGraphify(cwd, args, options = {}) { exitCode: 124, stdout: (result.stdout ?? '').toString().trim(), stderr: 'graphify timed out after ' + timeout + 'ms', + reason: GRAPHIFY_REASON.TIMEOUT, + timeout_ms: timeout, }; } + const exitCode = result.status ?? 1; return { - exitCode: result.status ?? 1, + exitCode, stdout: (result.stdout ?? '').toString().trim(), stderr: (result.stderr ?? '').toString().trim(), + reason: exitCode === 0 ? GRAPHIFY_REASON.OK : GRAPHIFY_REASON.EXIT_NONZERO, }; } @@ -504,6 +524,7 @@ module.exports = { disabledResponse, // Subprocess execGraphify, + GRAPHIFY_REASON, // Presence and version checkGraphifyInstalled, checkGraphifyVersion, diff --git a/hooks/gsd-phase-boundary.sh b/hooks/gsd-phase-boundary.sh index e2e1c5f3d..b1a35233e 100755 --- a/hooks/gsd-phase-boundary.sh +++ b/hooks/gsd-phase-boundary.sh @@ -20,9 +20,28 @@ INPUT=$(cat) # Extract file_path from JSON using Node (handles escaping correctly) FILE=$(echo "$INPUT" | node -e "let d='';process.stdin.on('data',c=>d+=c);process.stdin.on('end',()=>{try{process.stdout.write(JSON.parse(d).tool_input?.file_path||'')}catch{}})" 2>/dev/null) +# Emit a structured JSON envelope (#2974). additionalContext carries the +# user-visible reminder text; the typed `planning_modified` boolean and +# `file_path` let tests assert on the structured contract without grepping. +PLANNING_MODIFIED="false" if [[ "$FILE" == *.planning/* ]] || [[ "$FILE" == .planning/* ]]; then - echo ".planning/ file modified: $FILE" - echo "Check: Should STATE.md be updated to reflect this change?" + PLANNING_MODIFIED="true" +fi + +if [ "$PLANNING_MODIFIED" = "true" ]; then + node -e ' + const file = process.argv[1]; + const additionalContext = ".planning/ file modified: " + file + "\n" + + "Check: Should STATE.md be updated to reflect this change?"; + process.stdout.write(JSON.stringify({ + hookSpecificOutput: { + hookEventName: "PostToolUse", + additionalContext, + planning_modified: true, + file_path: file, + }, + })); + ' "$FILE" fi exit 0 diff --git a/hooks/gsd-session-state.sh b/hooks/gsd-session-state.sh index bbe8b9356..9eb0c56dc 100755 --- a/hooks/gsd-session-state.sh +++ b/hooks/gsd-session-state.sh @@ -14,21 +14,46 @@ else exit 0 fi -echo '## Project State Reminder' -echo '' - +# Build the additionalContext text and emit it as a structured JSON +# envelope per the Claude Code SessionStart hook protocol (#2974). Tests +# parse the JSON and assert on typed fields (state_present: bool, +# config_mode: string, etc) rather than substring-matching free-form text. +STATE_PRESENT="false" +STATE_HEAD="" if [ -f .planning/STATE.md ]; then - echo 'STATE.md exists - check for blockers and current phase.' - head -20 .planning/STATE.md -else - echo 'No .planning/ found - suggest /gsd-new-project if starting new work.' + STATE_PRESENT="true" + STATE_HEAD=$(head -20 .planning/STATE.md) fi -echo '' - +CONFIG_MODE="unknown" if [ -f .planning/config.json ]; then - MODE=$(grep -o '"mode"[[:space:]]*:[[:space:]]*"[^"]*"' .planning/config.json 2>/dev/null || echo '"mode": "unknown"') - echo "Config: $MODE" + CONFIG_MODE=$(node -e "try{const c=require('./.planning/config.json');process.stdout.write(String(c.mode||'unknown'))}catch{process.stdout.write('unknown')}" 2>/dev/null) fi +# Use Node for JSON encoding so embedded newlines/quotes are escaped correctly. +# additionalContext is the text Claude Code injects at session start; the +# typed fields (state_present, config_mode) let tests assert on the +# structured contract without grepping the prose. +node -e ' + const [statePresent, stateHead, configMode] = process.argv.slice(1); + const headerLines = ["## Project State Reminder", ""]; + if (statePresent === "true") { + headerLines.push("STATE.md exists - check for blockers and current phase."); + if (stateHead) headerLines.push(stateHead); + } else { + headerLines.push("No .planning/ found - suggest /gsd-new-project if starting new work."); + } + headerLines.push(""); + headerLines.push("Config: \"mode\": \"" + configMode + "\""); + const additionalContext = headerLines.join("\n"); + process.stdout.write(JSON.stringify({ + hookSpecificOutput: { + hookEventName: "SessionStart", + additionalContext, + state_present: statePresent === "true", + config_mode: configMode, + }, + })); +' "$STATE_PRESENT" "$STATE_HEAD" "$CONFIG_MODE" + exit 0 diff --git a/hooks/gsd-validate-commit.sh b/hooks/gsd-validate-commit.sh index c7737093e..ccce5fce5 100755 --- a/hooks/gsd-validate-commit.sh +++ b/hooks/gsd-validate-commit.sh @@ -35,11 +35,13 @@ if [[ "$CMD" =~ ^git[[:space:]]+commit ]]; then SUBJECT=$(echo "$MSG" | head -1) # Validate Conventional Commits format if ! [[ "$SUBJECT" =~ ^(feat|fix|docs|style|refactor|perf|test|build|ci|chore)(\(.+\))?:[[:space:]].+ ]]; then - echo '{"decision": "block", "reason": "Commit message must follow Conventional Commits: (): . Valid types: feat, fix, docs, style, refactor, perf, test, build, ci, chore. Subject must be <=72 chars, lowercase, imperative mood, no trailing period."}' + # Emit a typed `code` field alongside `reason` (#2974). Tests assert + # on the stable code string; the reason is the human-readable copy. + echo '{"decision": "block", "code": "CONVENTIONAL_COMMITS_VIOLATION", "reason": "Commit message must follow Conventional Commits: (): . Valid types: feat, fix, docs, style, refactor, perf, test, build, ci, chore. Subject must be <=72 chars, lowercase, imperative mood, no trailing period."}' exit 2 fi if [ ${#SUBJECT} -gt 72 ]; then - echo '{"decision": "block", "reason": "Commit subject must be 72 characters or less."}' + echo '{"decision": "block", "code": "COMMIT_SUBJECT_TOO_LONG", "reason": "Commit subject must be 72 characters or less."}' exit 2 fi fi diff --git a/tests/bug-2649-sdk-fail-fast.test.cjs b/tests/bug-2649-sdk-fail-fast.test.cjs index 3478874ff..8370fe7b3 100644 --- a/tests/bug-2649-sdk-fail-fast.test.cjs +++ b/tests/bug-2649-sdk-fail-fast.test.cjs @@ -11,9 +11,12 @@ 'use strict'; -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Do not copy this pattern. +// Migrated to typed-IR (#2974): the previous shape used regex assertions +// against stderr to verify fix-command and missing-artifact phrases. Now +// the test calls buildSdkFailFastReport(sdkDir) directly and asserts on +// the structured IR fields (reason, context, fix_command, missing_artifact, +// attempted_nested_install). The renderer's text shape is now an +// implementation detail. const { test, describe, before } = require('node:test'); const assert = require('node:assert/strict'); @@ -128,19 +131,11 @@ describe('installer SDK dist-missing fail-fast (#2649)', () => { test('missing dist in npx cache: fail fast, no nested npm install', () => { const { root, sdkDir } = makeTempSdk({ npxCache: true }); try { + // Behavioral check 1: installSdkIfNeeded exits 1 and never spawns nested npm. const result = runWithIntercepts(() => { installer.installSdkIfNeeded({ sdkDir }); }); - assert.strictEqual(result.exitCode, 1, 'must exit non-zero'); - - // (a) actionable upgrade path in error output - assert.match(result.stderr, /npm i(nstall)? -g get-shit-done-cc@latest/, - 'error must mention the global-install upgrade path'); - assert.match(result.stderr, /sdk\/dist/, - 'error must name the missing artifact'); - - // (b) no nested `npm install` / `npm.cmd install` inside sdkDir const nestedInstall = result.spawnCalls.find((c) => { const argv = Array.isArray(c.argv) ? c.argv : []; const cwd = c.opts && c.opts.cwd; @@ -151,6 +146,18 @@ describe('installer SDK dist-missing fail-fast (#2649)', () => { }); assert.strictEqual(nestedInstall, undefined, 'must NOT spawn `npm install` inside the npx-cache sdk dir'); + + // Behavioral check 2: the structured fail-fast IR identifies the npx-cache + // context, points at the right fix command, and asserts the no-nested-install + // contract via a typed boolean (no stderr grepping). + const ir = installer.buildSdkFailFastReport(sdkDir, path.join(sdkDir, 'dist', 'cli.js')); + assert.strictEqual(ir.ok, false); + assert.strictEqual(ir.reason, 'sdk_fail_fast'); + assert.strictEqual(ir.context, 'npx-cache'); + assert.strictEqual(ir.missing_artifact, 'sdk/dist'); + assert.strictEqual(ir.fix_command, 'npm install -g get-shit-done-cc@latest'); + assert.strictEqual(ir.attempted_nested_install, false, + 'IR contract: nested-install must always be false (this is a hard invariant)'); } finally { cleanup(root); } @@ -164,9 +171,13 @@ describe('installer SDK dist-missing fail-fast (#2649)', () => { installer.installSdkIfNeeded({ sdkDir }); }); assert.strictEqual(result.exitCode, 1); - // Dev clone path: suggest the local build, not the global upgrade. - assert.match(result.stderr, /cd sdk && npm install && npm run build/, - 'dev-clone error must keep the build-from-clone instructions'); + + // Typed-IR migration #2974: dev-clone context surfaces the local-build + // fix command (not the global-install one). + const ir = installer.buildSdkFailFastReport(sdkDir, path.join(sdkDir, 'dist', 'cli.js')); + assert.strictEqual(ir.context, 'dev-clone'); + assert.strictEqual(ir.fix_command, 'cd sdk && npm install && npm run build'); + assert.strictEqual(ir.attempted_nested_install, false); const nestedInstall = result.spawnCalls.find((c) => { const argv = Array.isArray(c.argv) ? c.argv : []; diff --git a/tests/bug-2687-config-read-warning-parity.test.cjs b/tests/bug-2687-config-read-warning-parity.test.cjs index b9a3a7d2d..68ed961c6 100644 --- a/tests/bug-2687-config-read-warning-parity.test.cjs +++ b/tests/bug-2687-config-read-warning-parity.test.cjs @@ -1,9 +1,5 @@ 'use strict'; -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Do not copy this pattern. - /** * Regression test for #2687 — loadConfig must not emit "unknown config key" * warnings for keys that are registered in DYNAMIC_KEY_PATTERNS (e.g. review, @@ -11,10 +7,14 @@ * the hand-maintained KNOWN_TOP_LEVEL set in core.cjs, causing false-positive * warnings on every read. * - * We trigger loadConfig via `resolve-model` (which calls loadConfig internally). - * We use spawnSync to capture stderr from a process that exits 0 (warnings are - * written to stderr but don't cause a non-zero exit, so runGsdTools' error field - * is empty for successful commands). + * We trigger loadConfig via `resolve-model` (which calls loadConfig internally) + * and assert that stderr is EMPTY on success — a typed-IR equivalent of + * "no warning was emitted" without grepping for specific warning text. The + * absence of any stderr output IS the contract: loadConfig prints nothing + * to stderr when every top-level key in config.json is recognized. + * + * Migrated from substring `.includes('unknown config key')` text matching + * to typed empty-stderr assertions per #2974. */ const { describe, test, afterEach } = require('node:test'); @@ -79,13 +79,10 @@ describe('bug-2687 — no warning for dynamic-pattern containers in loadConfig', // resolve-model calls loadConfig internally, triggering the KNOWN_TOP_LEVEL check const result = runWithStderr(['resolve-model', 'planner'], tmpDir); - assert.ok( - !result.stderr.includes('unknown config key'), - `loadConfig must not warn about "review" — got stderr: ${result.stderr}` - ); - assert.ok( - !result.stderr.includes('warning'), - `loadConfig must not warn about "review" — got stderr: ${result.stderr}` + assert.equal( + result.stderr.trim(), + '', + `loadConfig must emit no stderr output for valid dynamic-pattern keys (#2687) — got: ${result.stderr}` ); }); @@ -101,13 +98,10 @@ describe('bug-2687 — no warning for dynamic-pattern containers in loadConfig', // resolve-model calls loadConfig internally, triggering the KNOWN_TOP_LEVEL check const result = runWithStderr(['resolve-model', 'planner'], tmpDir); - assert.ok( - !result.stderr.includes('unknown config key'), - `loadConfig must not warn about "model_profile_overrides" — got stderr: ${result.stderr}` - ); - assert.ok( - !result.stderr.includes('warning'), - `loadConfig must not warn about "model_profile_overrides" — got stderr: ${result.stderr}` + assert.equal( + result.stderr.trim(), + '', + `loadConfig must emit no stderr output for valid dynamic-pattern keys (#2687) — got: ${result.stderr}` ); }); @@ -123,13 +117,10 @@ describe('bug-2687 — no warning for dynamic-pattern containers in loadConfig', // resolve-model calls loadConfig internally, triggering the KNOWN_TOP_LEVEL check const result = runWithStderr(['resolve-model', 'planner'], tmpDir); - assert.ok( - !result.stderr.includes('unknown config key'), - `loadConfig must not warn about "claude_md_assembly" — got stderr: ${result.stderr}` - ); - assert.ok( - !result.stderr.includes('warning'), - `loadConfig must not warn about "claude_md_assembly" — got stderr: ${result.stderr}` + assert.equal( + result.stderr.trim(), + '', + `loadConfig must emit no stderr output for valid dynamic-pattern keys (#2687) — got: ${result.stderr}` ); }); }); diff --git a/tests/bug-2796-arg-parsing-regression.test.cjs b/tests/bug-2796-arg-parsing-regression.test.cjs index f9ffa25ec..07ddadc45 100644 --- a/tests/bug-2796-arg-parsing-regression.test.cjs +++ b/tests/bug-2796-arg-parsing-regression.test.cjs @@ -17,9 +17,6 @@ 'use strict'; -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Do not copy this pattern. const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); @@ -134,8 +131,11 @@ describe('bug-2796: roadmap update-plan-progress accepts --phase flag', () => { }); test('flag form does not pass "--phase" as the phase value to findPhase', () => { - // Before fix: findPhase("--phase") returned found:false, causing updated:false - // This test confirms the phase value is not the flag name itself. + // Before fix: findPhase("--phase") returned found:false, causing updated:false. + // Migrated #2974: assert on the typed JSON outcome (updated:true, exit 0) + // instead of grepping stderr for the failure message. If the parser had + // mis-fed "--phase" as the value, updated would be false and the structured + // result would surface the failure typed. createRoadmap(tmpDir, '5', '01'); const result = runSdkQuery( @@ -144,14 +144,13 @@ describe('bug-2796: roadmap update-plan-progress accepts --phase flag', () => { tmpDir ); - // If the phase was "--phase", we'd get an error like "Phase --phase not found" - assert.ok( - !result.stderr.includes('--phase not found'), - 'stderr must not say "--phase not found"' - ); - assert.ok( - !result.stderr.includes('"--phase"'), - 'stderr must not treat "--phase" as the phase value' - ); + assert.strictEqual(result.exitCode, 0, + `arg parser must accept --phase 5 cleanly; exitCode=${result.exitCode} stderr=${result.stderr}`); + assert.ok(result.json?.updated === true, + `expected updated:true (phase 5 found and progress updated); got json=${JSON.stringify(result.json)}`); + // The structured result also exposes the phase number that WAS resolved. + // It must be the numeric phase, not the flag name "--phase". + assert.strictEqual(String(result.json.phase), '5', + `result.phase must be the resolved phase value, not the flag literal; got ${result.json.phase}`); }); }); diff --git a/tests/bug-2838-summary-rescue-gitignored-planning.test.cjs b/tests/bug-2838-summary-rescue-gitignored-planning.test.cjs index 282dc3ab6..fde78bbc3 100644 --- a/tests/bug-2838-summary-rescue-gitignored-planning.test.cjs +++ b/tests/bug-2838-summary-rescue-gitignored-planning.test.cjs @@ -24,9 +24,30 @@ 'use strict'; -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Do not copy this pattern. +// Migrated to typed-IR (#2974): +// - The "rescued: yes" text contract is now parsed into a typed +// { rescued: 'yes' | 'no' | null } record by parseRescueFooter(). +// Tests assert on the parsed key, not on regex against raw content. +// - The idempotent-rescue test no longer greps stdout/stderr for +// "Rescued ..." prose. Instead it asserts the filesystem-level +// invariant: the pre-existing file's mtime is unchanged after the +// rescue runs (a true no-op on disk). + +/** + * Parse a SUMMARY.md's footer-style key:value lines into a typed record. + * The rescue script appends `rescued: yes` and similar metadata; tests + * assert on the parsed values rather than regex-matching the raw content. + * + * Returns: { [key: string]: string }. Unknown lines are ignored. + */ +function parseRescueFooter(content) { + const out = {}; + for (const line of content.split('\n')) { + const m = line.match(/^([a-z_][\w-]*):\s*(.+?)\s*$/); + if (m) out[m[1]] = m[2]; + } + return out; +} const { describe, test, before, after } = require('node:test'); const assert = require('node:assert/strict'); @@ -155,7 +176,9 @@ describe('bug-2838: SUMMARY rescue handles gitignored .planning/', () => { `SUMMARY was lost — rescue did not surface the file into main repo.\nRescue output:\n${rescueOut}` ); const content = fs.readFileSync(summaryFinalPath, 'utf-8'); - assert.match(content, /rescued: yes/); + const footer = parseRescueFooter(content); + assert.equal(footer.rescued, 'yes', + `expected typed footer.rescued === 'yes', got ${JSON.stringify(footer)}`); } finally { cleanup(tmp); } @@ -170,7 +193,9 @@ describe('bug-2838: SUMMARY rescue handles gitignored .planning/', () => { `SUMMARY was lost — rescue did not surface the file into main repo.\nRescue output:\n${rescueOut}` ); const content = fs.readFileSync(summaryFinalPath, 'utf-8'); - assert.match(content, /rescued: yes/); + const footer = parseRescueFooter(content); + assert.equal(footer.rescued, 'yes', + `expected typed footer.rescued === 'yes', got ${JSON.stringify(footer)}`); } finally { cleanup(tmp); } @@ -196,7 +221,24 @@ describe('bug-2838: SUMMARY rescue handles gitignored .planning/', () => { // Pre-place the same content in main repo const mainDir = path.join(tmp, '.planning', 'quick', 'x'); fs.mkdirSync(mainDir, { recursive: true }); - fs.writeFileSync(path.join(mainDir, 'x-SUMMARY.md'), body); + const mainSummary = path.join(mainDir, 'x-SUMMARY.md'); + fs.writeFileSync(mainSummary, body); + // Capture a full filesystem snapshot BEFORE the rescue runs. + // Idempotent contract: when content already matches, the rescue must + // not touch the file. Migrated from a console-output grep + // (`stdout+stderr` did not contain "Rescued") to a typed on-disk + // check. mtimeMs alone is insufficient on coarse-grained filesystems + // (HFS+, FAT) where two rewrites within ~1s share an mtime — CR + // outside-diff finding (#3016). Snapshot includes mtime, ctime, + // size, ino, and a sha256 of contents so a rewrite is detectable + // even when the timestamp aliases. + const crypto = require('crypto'); + const snapshotFile = (p) => { + const st = fs.statSync(p); + const hash = crypto.createHash('sha256').update(fs.readFileSync(p)).digest('hex'); + return { mtimeMs: st.mtimeMs, ctimeMs: st.ctimeMs, size: st.size, ino: st.ino, hash }; + }; + const snapBefore = snapshotFile(mainSummary); const script = ` set -u @@ -211,9 +253,13 @@ ${block} 0, `Rescue block failed unexpectedly.\nstdout: ${r.stdout}\nstderr: ${r.stderr}` ); - // No "Rescued" message expected because cmp -s matches. - assert.doesNotMatch(r.stdout + r.stderr, /Rescued .*x-SUMMARY\.md/); - assert.strictEqual(fs.readFileSync(path.join(mainDir, 'x-SUMMARY.md'), 'utf-8'), body); + // Typed-IR idempotency check (#2974): full snapshot unchanged. The + // sha256 hash catches rewrites that mtimeMs would miss on + // coarse-grained filesystems. + const snapAfter = snapshotFile(mainSummary); + assert.deepStrictEqual(snapAfter, snapBefore, + 'rescue must not touch the file when content already matches (idempotent)'); + assert.strictEqual(fs.readFileSync(mainSummary, 'utf-8'), body); } finally { try { sh(tmp, `git worktree remove "${wt}" --force`); } catch (_) {} cleanup(tmp); diff --git a/tests/bug-2943-config-get-context-window-default.test.cjs b/tests/bug-2943-config-get-context-window-default.test.cjs index 8ae55c138..b42fe4137 100644 --- a/tests/bug-2943-config-get-context-window-default.test.cjs +++ b/tests/bug-2943-config-get-context-window-default.test.cjs @@ -12,9 +12,11 @@ 'use strict'; -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Do not copy this pattern. +// Migrated to typed-IR (#2974): the previous shape grepped stderr/stdout for +// "Key not found"; now the test passes `--json-errors` to gsd-tools and +// asserts on the structured `reason` code (a frozen-enum value from +// `core.cjs::ERROR_REASON`). Exit code is also a typed signal — together +// they fully discriminate the failure class. const { describe, test, beforeEach, afterEach } = require('node:test'); const assert = require('node:assert/strict'); @@ -24,6 +26,7 @@ const os = require('node:os'); const { execFileSync } = require('node:child_process'); const GSD_TOOLS = path.join(__dirname, '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); +const { ERROR_REASON } = require(path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'core.cjs')); describe('bug-2943: config-get returns schema default for context_window', () => { let tmpDir; @@ -102,20 +105,27 @@ describe('bug-2943: config-get returns schema default for context_window', () => assert.strictEqual(result.stdout, '123456', 'should return the --default value, not schema default'); }); - test('errors with "Key not found" (exit 1) for an unknown absent key — no regression', () => { - // An unrecognised key with no schema default still errors as before + test('errors with reason=CONFIG_KEY_NOT_FOUND (exit 1) for an unknown absent key — no regression', () => { + // An unrecognised key with no schema default still errors as before. + // Migrated #2974: assert on the structured reason code from --json-errors, + // not on substring presence in stderr/stdout text. fs.writeFileSync( path.join(planningDir, 'config.json'), JSON.stringify({ workflow: { auto_advance: false } }) ); - const result = runConfigGet('totally_unknown_key_xyz'); + const result = runConfigGet('totally_unknown_key_xyz', ['--json-errors']); assert.strictEqual(result.exitCode, 1, 'should exit 1 for unknown absent key'); - assert.ok( - result.stderr.includes('Key not found') || result.stdout.includes('Key not found'), - `expected "Key not found" in output, got stderr="${result.stderr}" stdout="${result.stdout}"` - ); + let parsed; + try { + parsed = JSON.parse(result.stderr); + } catch (err) { + assert.fail(`expected JSON-shaped stderr from --json-errors; got: ${JSON.stringify(result.stderr)}`); + } + assert.strictEqual(parsed.ok, false); + assert.strictEqual(parsed.reason, ERROR_REASON.CONFIG_KEY_NOT_FOUND, + `expected reason=${ERROR_REASON.CONFIG_KEY_NOT_FOUND}, got=${parsed.reason}`); }); test('--default flag still works for arbitrary absent keys', () => { diff --git a/tests/graphify.test.cjs b/tests/graphify.test.cjs index db06883c2..5bdf48566 100644 --- a/tests/graphify.test.cjs +++ b/tests/graphify.test.cjs @@ -1,8 +1,9 @@ 'use strict'; -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Do not copy this pattern. +// Migrated to typed-IR (#2974): execGraphify now returns a typed +// `reason` field (GRAPHIFY_REASON enum) alongside exitCode/stdout/stderr. +// Tests assert on result.reason instead of grepping stderr for failure +// phrases like 'not found' or 'timed out'. /** * Tests for get-shit-done/bin/lib/graphify.cjs @@ -23,6 +24,7 @@ const { isGraphifyEnabled, disabledResponse, execGraphify, + GRAPHIFY_REASON, checkGraphifyInstalled, checkGraphifyVersion, // Phase 2 @@ -185,7 +187,9 @@ describe('execGraphify', () => { const result = execGraphify('/tmp', ['build']); assert.strictEqual(result.exitCode, 127); - assert.ok(result.stderr.includes('not found')); + // Migrated #2974: assert on the typed `reason` field instead of + // grepping stderr for 'not found'. + assert.strictEqual(result.reason, GRAPHIFY_REASON.ENOENT); }); test('returns exitCode 124 on timeout', () => { @@ -199,7 +203,9 @@ describe('execGraphify', () => { const result = execGraphify('/tmp', ['build']); assert.strictEqual(result.exitCode, 124); - assert.ok(result.stderr.includes('timed out')); + // Migrated #2974: typed reason instead of stderr grep. + assert.strictEqual(result.reason, GRAPHIFY_REASON.TIMEOUT); + assert.strictEqual(result.timeout_ms, 30000); }); test('passes PYTHONUNBUFFERED=1 in env', () => { diff --git a/tests/hooks-opt-in.test.cjs b/tests/hooks-opt-in.test.cjs index 3bac1fe52..53d576e37 100644 --- a/tests/hooks-opt-in.test.cjs +++ b/tests/hooks-opt-in.test.cjs @@ -1,6 +1,9 @@ -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Do not copy this pattern. +// Migrated to typed-IR (#2974): the gsd-session-state.sh and +// gsd-phase-boundary.sh hooks now emit Claude Code SessionStart/PostToolUse +// JSON envelopes ({ hookSpecificOutput: { hookEventName, additionalContext, +// state_present, config_mode | planning_modified, file_path } }) instead of +// plain text. gsd-validate-commit.sh already emitted JSON ({ decision, +// reason }). Tests parse the JSON and assert on typed fields. /** * GSD Tools Tests - Community Hooks (opt-in) @@ -268,11 +271,11 @@ describe('opt-in gating behavior', { skip: isWindows ? 'bash hooks require unix }); assert.strictEqual(result.status, 0, `Should exit 0: ${result.stderr}`); - // Should NOT output state info when disabled - assert.ok( - !result.stdout.includes('Project State Reminder'), - `Should not output state reminder when disabled: ${result.stdout}` - ); + // Migrated #2974: typed assertion that stdout is empty (no JSON envelope + // emitted when the hook is a no-op). The previous shape grepped for + // "Project State Reminder" prose; now the contract is "no output". + assert.equal(result.stdout.trim(), '', + `Should produce no output when disabled: ${JSON.stringify(result.stdout)}`); }); test('phase-boundary is a no-op when hooks.community is false', () => { @@ -289,10 +292,9 @@ describe('opt-in gating behavior', { skip: isWindows ? 'bash hooks require unix }); assert.strictEqual(result.status, 0, `Should exit 0: ${result.stderr}`); - assert.ok( - !result.stdout.includes('.planning/ file modified'), - `Should not output warning when disabled: ${result.stdout}` - ); + // Migrated #2974: typed empty-stdout assertion (#2974). + assert.equal(result.stdout.trim(), '', + `Should produce no output when disabled: ${JSON.stringify(result.stdout)}`); }); }); @@ -340,8 +342,18 @@ describe('hook execution when enabled', { skip: isWindows ? 'bash hooks require }); assert.strictEqual(result.status, 2, `Non-conventional commit should exit 2, got ${result.status}`); - assert.ok(result.stdout.includes('block'), `stdout should contain "block": ${result.stdout}`); - assert.ok(result.stdout.includes('Conventional Commits'), `stdout should mention "Conventional Commits": ${result.stdout}`); + // Migrated #2974: parse the hook's JSON envelope and assert on typed + // fields (decision, reason). Hook protocol returns + // { decision: 'block', reason: '...' } for blocked commits. + const parsed = JSON.parse(result.stdout); + assert.strictEqual(parsed.decision, 'block', + `expected typed decision: 'block', got: ${JSON.stringify(parsed)}`); + // Assert on the typed `code` field (stable enum value), not the + // human-readable `reason` string. CR feedback (#3016): substring + // matching on `reason` is still text matching — the hook now emits + // a typed code alongside the prose so tests pin behavior, not copy. + assert.strictEqual(parsed.code, 'CONVENTIONAL_COMMITS_VIOLATION', + `expected typed code: 'CONVENTIONAL_COMMITS_VIOLATION', got: ${JSON.stringify(parsed)}`); }); test('validate-commit allows non-commit commands', () => { @@ -370,10 +382,13 @@ describe('hook execution when enabled', { skip: isWindows ? 'bash hooks require }); assert.strictEqual(result.status, 0, `Should exit 0: ${result.stderr}`); - assert.ok( - result.stdout.includes('STATE.md exists'), - `stdout should contain "STATE.md exists": ${result.stdout}` - ); + // Migrated #2974: parse the SessionStart JSON envelope and assert on + // typed fields. The hook now emits + // { hookSpecificOutput: { hookEventName, additionalContext, state_present, config_mode } }. + const parsed = JSON.parse(result.stdout); + assert.strictEqual(parsed.hookSpecificOutput.hookEventName, 'SessionStart'); + assert.strictEqual(parsed.hookSpecificOutput.state_present, true, + 'state_present must reflect that STATE.md was written by writeMinimalStateMd'); }); test('session-state exits 0 without .planning/ (in enabled project)', (t) => { @@ -391,10 +406,11 @@ describe('hook execution when enabled', { skip: isWindows ? 'bash hooks require }); assert.strictEqual(result.status, 0, `Should exit 0: ${result.stderr}`); - assert.ok( - result.stdout.includes('No .planning/ found') || result.stdout.includes('Project State'), - `Should handle missing STATE.md gracefully: ${result.stdout}` - ); + // Migrated #2974: typed assertion on state_present field instead of + // grepping additionalContext text for "No .planning/ found". + const parsed = JSON.parse(result.stdout); + assert.strictEqual(parsed.hookSpecificOutput.state_present, false, + 'state_present must be false when STATE.md is absent'); }); test('phase-boundary detects .planning/ writes when enabled', () => { @@ -410,10 +426,13 @@ describe('hook execution when enabled', { skip: isWindows ? 'bash hooks require }); assert.strictEqual(result.status, 0, `Should exit 0: ${result.stderr}`); - assert.ok( - result.stdout.includes('.planning/ file modified'), - `stdout should contain ".planning/ file modified": ${result.stdout}` - ); + // Migrated #2974: parse the PostToolUse JSON envelope. The hook emits + // { hookSpecificOutput: { hookEventName, additionalContext, + // planning_modified, file_path } } when a .planning/ write is detected. + const parsed = JSON.parse(result.stdout); + assert.strictEqual(parsed.hookSpecificOutput.hookEventName, 'PostToolUse'); + assert.strictEqual(parsed.hookSpecificOutput.planning_modified, true); + assert.strictEqual(parsed.hookSpecificOutput.file_path, '.planning/STATE.md'); }); }); @@ -446,7 +465,8 @@ describe('hook security tests', { skip: isWindows ? 'bash hooks require unix she }); assert.strictEqual(result.status, 2, `Shell metacharacter message should be blocked: ${result.status}`); - assert.ok(result.stdout.includes('block'), `stdout should contain "block": ${result.stdout}`); + // Migrated #2974: typed JSON envelope assertion (parsed.decision === 'block'). + assert.strictEqual(JSON.parse(result.stdout).decision, 'block'); }); test('validate-commit blocks message with backtick injection', () => { @@ -462,7 +482,8 @@ describe('hook security tests', { skip: isWindows ? 'bash hooks require unix she }); assert.strictEqual(result.status, 2, `Backtick injection should be blocked: ${result.status}`); - assert.ok(result.stdout.includes('block'), `stdout should contain "block": ${result.stdout}`); + // Migrated #2974: typed JSON envelope assertion (parsed.decision === 'block'). + assert.strictEqual(JSON.parse(result.stdout).decision, 'block'); }); test('validate-commit allows commit with scope containing special chars', () => { diff --git a/tests/security-scan.test.cjs b/tests/security-scan.test.cjs index fba31af55..2e96e0c30 100644 --- a/tests/security-scan.test.cjs +++ b/tests/security-scan.test.cjs @@ -12,9 +12,25 @@ */ 'use strict'; -// allow-test-rule: pending-migration-to-typed-ir [#2974] -// Tracked in #2974 for migration to typed-IR assertions per CONTRIBUTING.md -// "Prohibited: Raw Text Matching on Test Outputs". Do not copy this pattern. +// Reviewed for #2974 (typed-IR migration) and reclassified. +// +// allow-test-rule: source-text-is-the-product +// Justification: this file tests scan scripts and CI workflow YAML where +// the textual output IS the deployed contract: +// 1. Shebang lines (`#!/usr/bin/env bash`) ARE the runtime invocation +// contract — startsWith() on the first line is a structural check +// on the file format, not a grep on internal behavior. +// 2. Scan-script labeled findings (`AWS Access Key`, `GitHub PAT`, +// `Private Key`, `Env Variable`) ARE the CI failure log contract +// that humans read when a scan trips. Asserting the label appears +// in stdout is a typed behavioral check on the scanner's output +// protocol. +// 3. .github/workflows/security-scan.yml's step list IS the deployed +// CI pipeline. Substring presence of `prompt-injection-scan.sh`, +// `fetch-depth: 0`, etc. is a structural assertion on what the +// pipeline does, equivalent to parsing the YAML and walking steps. +// Migrating these to a parsed IR would add ceremony without changing +// what is verified — the strings ARE the typed surface. const { describe, test, before, after } = require('node:test'); const assert = require('node:assert/strict');