* test(#2974): migrate 8 test files to typed-IR assertions Replaces raw stdout/stderr substring matching with structured-field assertions per CONTRIBUTING.md "Prohibited: Raw Text Matching on Test Outputs". Adds shared infrastructure for typed error emission so this pattern is the easy path going forward. Shared infrastructure: - core.cjs: ERROR_REASON frozen enum + setJsonErrorMode/getJsonErrorMode - gsd-tools.cjs: --json-errors CLI flag, parsed before subcommand dispatch - config.cjs: typed reasons at all 7 error sites - graphify.cjs: GRAPHIFY_REASON enum + reason/timeout_ms in execGraphify result - bin/install.js: pure buildSdkFailFastReport() IR builder + renderer - hooks/gsd-session-state.sh, gsd-phase-boundary.sh: emit Claude Code hookSpecificOutput JSON envelope with typed state_present/config_mode/ planning_modified/file_path fields (no-op when hooks.community is off) Test migrations (all pass, 171 tests across the 8 files): - bug-2649-sdk-fail-fast: assert on ir.reason / ir.context / ir.fix_command - bug-2687-config-read-warning-parity: assert.equal stderr === '' - bug-2796-arg-parsing-regression: assert on result.json.updated/.phase - bug-2838-summary-rescue: parse rescue footer, assert mtime invariant - bug-2943-config-get-context-window: parse JSON, assert ERROR_REASON.CONFIG_KEY_NOT_FOUND - graphify: assert reason === GRAPHIFY_REASON.ENOENT/TIMEOUT - hooks-opt-in: parse hookSpecificOutput, assert typed fields - security-scan: reclassified as source-text-is-the-product (scan label output and CI workflow YAML ARE the deployed contract) Verification: lint-no-source-grep clean (0 violations), full suite 6741/6741 pass. Closes #2974 * test(#2974): address CR feedback — typed code field, robust idempotency Two CodeRabbit findings on #3016 addressed: 1. tests/hooks-opt-in.test.cjs:355 (Minor, inline) — parsed.reason.includes('Conventional Commits') was still substring matching after the typed-IR migration. Fixed at the source: the gsd-validate-commit hook now emits a typed `code` field ('CONVENTIONAL_COMMITS_VIOLATION', 'COMMIT_SUBJECT_TOO_LONG') alongside the human-readable `reason`. Test asserts strictEqual on the code; the prose copy is no longer part of the test contract. 2. tests/bug-2838-summary-rescue-gitignored-planning.test.cjs:224-250 (Outside-diff) — mtimeMs alone can stay unchanged on coarse-grained filesystems (HFS+, FAT) when two rewrites land within the same timestamp tick, falsely passing the idempotency assertion. Replaced with a full snapshot (mtimeMs, ctimeMs, size, ino, sha256 of contents) compared via assert.deepStrictEqual — the hash catches any rewrite the timestamp would miss. Verification: 30/30 pass on the two affected files; lint-no-source-grep clean (0 violations across 368 test files).
This commit is contained in:
@@ -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: <ERROR_REASON code>, message }) instead of plain
|
||||
// "Error: <text>". 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 <name>: 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]).
|
||||
|
||||
@@ -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.<agent-type>, features.<feature_name>`);
|
||||
error(`Unknown config key: "${keyPath}". Valid keys: ${[...VALID_CONFIG_KEYS].sort().join(', ')}, agent_skills.<agent-type>, features.<feature_name>`, 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
|
||||
|
||||
@@ -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: <message>" 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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user