diff --git a/.changeset/proud-otters-howl.md b/.changeset/proud-otters-howl.md new file mode 100644 index 000000000..dc20aa038 --- /dev/null +++ b/.changeset/proud-otters-howl.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2688 +--- +**A corrupt `.planning/config.json` no longer silently discards your entire configuration** — a single trailing comma used to fall back to built-in defaults with no signal, indistinguishable from having no config file at all, so a project could run for weeks on defaults while its model profile, workflow toggles and branching strategy sat unread on disk. GSD now tells you the file could not be used and that its settings were not applied, and reports the cause (`config_unparseable` / `config_unreadable`) distinctly from genuine absence. The same applies to an unreadable file and to the global `~/.gsd/defaults.json`. (#1880) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index bf9e3944e..5f8c432c6 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -140,6 +140,28 @@ GSD stores project settings in `.planning/config.json`. Created during `/gsd-new --- +## When your config file cannot be read + +GSD distinguishes a config file that is **absent** from one that is **present but unusable**. The +two used to be indistinguishable: a single trailing comma in `.planning/config.json` silently +replaced your entire configuration with built-in defaults, and nothing said so (#1880). + +| Situation | What GSD does | Diagnostic | +|---|---|---| +| No `.planning/config.json` | Uses built-in defaults. This is normal. | none | +| File present, valid, has settings | Uses your settings. | none | +| File present, valid, but empty (`{}`) | Uses built-in defaults. | none | +| **File present but not valid JSON** | Uses built-in defaults — **your settings are not applied** | `warning: is not valid JSON — its settings were NOT applied` | +| **File present but unreadable** (e.g. permissions) | Uses built-in defaults — **your settings are not applied** | `warning: could not be read (EACCES) — its settings were NOT applied` | + +The warning is printed once per file per run, so a repeated command will not spam it. + +The same applies to the global `~/.gsd/defaults.json`. If the project config is also unusable, the +project one is reported, since that is the file you are most likely able to fix. + +**If you see this warning:** your config was not applied. Validate the file, for example with +`node -e "JSON.parse(require('fs').readFileSync('.planning/config.json','utf8'))"`, then re-run. + ## Core Settings | Setting | Type | Options | Default | Description | diff --git a/scripts/lint-resolution-provenance.cjs b/scripts/lint-resolution-provenance.cjs index a7920c757..0d988e7a8 100644 --- a/scripts/lint-resolution-provenance.cjs +++ b/scripts/lint-resolution-provenance.cjs @@ -56,6 +56,15 @@ const REGISTRY = [ sourceFile: 'src/init.cts', testFile: 'tests/agent-skills.test.cjs', }, + { + // Registered by #1880 (ADR-1411 amendment "corrupt is not absent"). Until + // this entry existed the config-loader seam carried the provenance contract + // with nothing guarding it, so a regression that collapsed configured_empty + // back into not_configured would have shipped silently. + verb: 'config-loader', + sourceFile: 'src/config-loader.cts', + testFile: 'tests/config-loader.test.cjs', + }, ]; // Markers that MUST appear in every registered verb's test file. diff --git a/src/config-loader.cts b/src/config-loader.cts index bce884b14..749fdae7b 100644 --- a/src/config-loader.cts +++ b/src/config-loader.cts @@ -316,6 +316,7 @@ function _warnUnknownProfileOverrides(parsed: Record, configLab function _resetRuntimeWarningCacheForTests(): void { _warnedConfigKeys.clear(); _warnedUnknownConfigKeys.clear(); + _warnedUnusableConfig.clear(); } // ─── FIX 2: Federated overlay helpers ──────────────────────────────────────── @@ -425,13 +426,119 @@ type ConfigSource = 'workstream' | 'root' | 'builtin-defaults' | 'global-default /** * Result of loadConfigResolved — wraps the config object with provenance metadata. * - source: which layer supplied the config - * - degraded: true when a workstream was requested but its config.json was absent - * (fell back to root config); false otherwise + * - degraded: true when the resolution did not deliver the configuration it + * should have — either a workstream was requested but its + * config.json was absent (fell back to root), or a file on the + * resolution path exists but is unusable (#1880). `reason` says which. */ +/** + * Machine-readable outcome of a config resolution (#1880, ADR-1411 amendment + * "corrupt is not absent"). `Resolution`'s four documented values all + * describe a resolution *miss*; the two `config_un*` values below are the + * unusable-input class that amendment introduced, and they are what makes a + * corrupt file distinguishable from an absent one. + * + * Frozen enum rather than bare strings so tests assert on the typed surface + * instead of diagnostic prose (CONTRIBUTING.md — Prohibited: Raw Text Matching + * on Test Outputs). + */ +const CONFIG_REASON = Object.freeze({ + /** A config file was found, parsed, and supplied at least one setting. */ + RESOLVED: 'resolved', + /** No config file exists at the resolved path. Genuine absence — NOT degraded. */ + NOT_CONFIGURED: 'not_configured', + /** A config file exists and parsed, but carried no settings (`{}`). */ + CONFIGURED_EMPTY: 'configured_empty', + /** A workstream was requested but had no config; fell back to root. */ + WORKSTREAM_FALLBACK: 'workstream_fallback', + /** The file exists but is not valid JSON — settings were NOT applied. */ + CONFIG_UNPARSEABLE: 'config_unparseable', + /** The file exists but could not be read (EACCES/EIO/…) — NOT applied. */ + CONFIG_UNREADABLE: 'config_unreadable', +} as const); + +type ConfigReason = (typeof CONFIG_REASON)[keyof typeof CONFIG_REASON]; + +/** A config file that exists but cannot be used. Absence is NOT a fault. */ +interface ConfigFault { + reason: typeof CONFIG_REASON.CONFIG_UNPARSEABLE | typeof CONFIG_REASON.CONFIG_UNREADABLE; + /** Resolved path of the offending file — half of the diagnostic dedup key. */ + path: string; + /** errno for an unreadable file; '' for a parse failure. The other half. */ + code: string; +} + interface ConfigResolution { config: Record; source: ConfigSource; degraded: boolean; + /** + * Why this resolution produced what it did. `degraded` alone cannot separate + * "no config here" from "your config is corrupt and was discarded" — both + * previously returned identical objects (#1880). + */ + reason: ConfigReason; +} + +/** + * Read + JSON-parse a config file, keeping *absent* distinguishable from + * *unusable*. `platformReadSync` returns null on ENOENT and re-throws every + * other errno, which is the seam that makes this separable at all. + */ +function _readConfigFile(filePath: string): + | { kind: 'ok'; data: Record } + | { kind: 'absent' } + | { kind: 'fault'; fault: ConfigFault } { + let raw: string | null; + try { + raw = platformReadSync(filePath); + } catch (err) { + const code = (err as NodeJS.ErrnoException).code ?? 'EUNKNOWN'; + return { kind: 'fault', fault: { reason: CONFIG_REASON.CONFIG_UNREADABLE, path: filePath, code } }; + } + if (raw === null) return { kind: 'absent' }; + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return { kind: 'fault', fault: { reason: CONFIG_REASON.CONFIG_UNPARSEABLE, path: filePath, code: '' } }; + } + // Shape, not just parseability (ADR-227). `0`, `"x"`, `[]` and `null` are all + // valid JSON but are not a config object. Accepting them let a PRESENT file + // parse "ok", then throw downstream, and be reported not_configured by the + // outer catch — a corrupt file indistinguishable from an absent one, which is + // the exact defect this change closes. Caught by the fast-check property. + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + return { kind: 'fault', fault: { reason: CONFIG_REASON.CONFIG_UNPARSEABLE, path: filePath, code: '' } }; + } + return { kind: 'ok', data: parsed as Record }; +} + +/** + * Dedup set for the unusable-config diagnostic. Keyed on resolved path + errno + * per the ADR-1411 amendment — never on message text, which would couple the + * guard to wording, and never on the errno alone, which would suppress a + * genuine second failure in a different file. + */ +const _warnedUnusableConfig = new Set(); + +/** + * The wiring clause (ADR-1411 amendment). `reason` lives on `ConfigResolution`, + * but `loadConfig` — the wrapper roughly fifty call sites use — returns + * `.config` alone and would never surface it. Without this diagnostic the field + * is unreachable to almost every consumer, and the user whose config was + * silently discarded still gets no signal. That was the whole defect in #1880. + */ +function _warnUnusableConfig(fault: ConfigFault): void { + const key = `${fault.path}${fault.reason}${fault.code}`; + if (_warnedUnusableConfig.has(key)) return; + _warnedUnusableConfig.add(key); + const what = fault.reason === CONFIG_REASON.CONFIG_UNPARSEABLE + ? 'is not valid JSON' + : `could not be read (${fault.code})`; + process.stderr.write( + `gsd-tools: warning: ${fault.path} ${what} — its settings were NOT applied; using defaults instead\n`, + ); } /** @@ -441,13 +548,21 @@ interface ConfigResolution { * { config, source, degraded } instead of just the config object. * loadConfig now delegates to this function (byte-identical back-compat). * - * Branch → source/degraded mapping: - * A1: ws set + ws config.json found → source:'workstream', degraded:false - * A2: ws null + config.json found → source:'root', degraded:false - * B: catch + .planning/ + rootParsed set (ws fallback) → source:'root', degraded:true - * C: catch + .planning/ + rootParsed null (federated defaults) → source:'builtin-defaults', degraded:false - * D: catch + no .planning/ + ~/.gsd/defaults.json readable → source:'global-defaults', degraded:false - * E: catch + no .planning/ + no global → source:'builtin-defaults', degraded:false + * Branch → source/degraded/reason mapping: + * A1: ws set + ws config.json found → source:'workstream', degraded:false, reason:'resolved'|'configured_empty' + * A2: ws null + config.json found → source:'root', degraded:false, reason:'resolved'|'configured_empty' + * B: catch + .planning/ + rootParsed set (ws fallback) → source:'root', degraded:true, reason:'workstream_fallback' + * C: catch + .planning/ + rootParsed null (federated defaults) → source:'builtin-defaults', degraded:false, reason:'not_configured' + * D: catch + no .planning/ + ~/.gsd/defaults.json readable → source:'global-defaults', degraded:false, reason:'not_configured' + * E: catch + no .planning/ + no global → source:'builtin-defaults', degraded:false, reason:'not_configured' + * + * ORTHOGONAL to all of the above (#1880, ADR-1411 "corrupt is not absent"): if + * any config file on the resolution path exists but is UNUSABLE — invalid JSON, + * or an errno such as EACCES — every branch instead returns degraded:true with + * reason:'config_unparseable'|'config_unreadable', and a deduplicated stderr + * diagnostic names the file. Before this, a trailing comma in config.json was + * byte-identical to the file not existing: builtin defaults, degraded:false, + * and the user's entire configuration silently discarded. */ function loadConfigResolved(cwd: string, options: Record = {}): ConfigResolution { // NOTE: loadConfigResolved resolves from cwd AS-IS (no walk-up). @@ -470,13 +585,39 @@ function loadConfigResolved(cwd: string, options: Record = {}): if (cachedSubRepos === undefined) cachedSubRepos = detectSubRepos(cwd); return cachedSubRepos.slice(); }; + // Faults are captured, not thrown: the existing control flow (one broad catch + // that falls back to defaults) is preserved exactly — see #1880. All that is + // added is knowing WHY the fallback fired, which is the whole defect. + let configFault: ConfigFault | null = null; + + /** + * Stamp a fallback return with its reason. Every branch below reaches defaults + * (or the root config) — what differs is WHY, and before #1880 that was + * unrecoverable: a corrupt file and an absent one produced identical objects. + * + * An unusable file always wins and always sets `degraded:true`; genuine + * absence keeps whatever `degraded` the branch already decided, so the + * existing #1366 workstream-fallback semantics are untouched. + */ + const fallback = (r: Omit): ConfigResolution => { + if (configFault) return { ...r, degraded: true, reason: configFault.reason }; + return { + ...r, + reason: r.degraded ? CONFIG_REASON.WORKSTREAM_FALLBACK : CONFIG_REASON.NOT_CONFIGURED, + }; + }; + let rootParsed: ParsedConfig | null = null; if (ws) { const rootConfigPath = path.join(planningRoot(cwd), 'config.json'); try { - const raw = platformReadSync(rootConfigPath); - if (raw === null) throw new Error('missing'); - rootParsed = JSON.parse(raw) as ParsedConfig; + const rootRead = _readConfigFile(rootConfigPath); + if (rootRead.kind === 'fault') { + configFault = rootRead.fault; + _warnUnusableConfig(rootRead.fault); + } + if (rootRead.kind !== 'ok') throw new Error('root config absent or unusable'); + rootParsed = rootRead.data; const { parsed: rootNormalized, normalizations: rootNorms } = normalizeLegacyKeys(rootParsed); if (rootNorms.length > 0) { for (const norm of rootNorms as unknown as NormalizationEntry[]) { @@ -503,9 +644,17 @@ function loadConfigResolved(cwd: string, options: Record = {}): const defaults = CONFIG_DEFAULTS; try { - const raw = platformReadSync(configPath); - if (raw === null) throw new Error('missing'); - const fileData: ParsedConfig = JSON.parse(raw) as ParsedConfig; + const read = _readConfigFile(configPath); + if (read.kind === 'fault') { + // The workstream/root config that ACTUALLY governs this resolution is + // unusable. This outranks any earlier root-config fault for reporting. + configFault = read.fault; + _warnUnusableConfig(read.fault); + } + if (read.kind !== 'ok') throw new Error('config absent or unusable'); + const fileData: ParsedConfig = read.data; + // Snapshot BEFORE normalizeLegacyKeys mutates fileData in place. + const fileHadKeys = Object.keys(read.data).length > 0; let configDirty = false; { @@ -679,7 +828,26 @@ function loadConfigResolved(cwd: string, options: Record = {}): // A1 vs A2: disambiguate by whether a real workstream was requested. // Fix 4: empty-string ws ('') resolves the root path → source:'root'. const source: ConfigSource = wsRequested ? 'workstream' : 'root'; - return { config: _baseConfig, source, degraded: false }; + + // This config parsed — but a DIFFERENT file on the resolution path may not + // have. A workstream config that loads cleanly while the root config it + // inherits from is corrupt is still a degraded resolution: the root's + // settings were silently dropped. Reporting `resolved` here would reopen + // the exact hole this change closes, for the common case of a project that + // uses workstreams at all. + if (configFault) { + return { config: _baseConfig, source, degraded: true, reason: configFault.reason }; + } + + // Emptiness is judged on the FILE THAT WAS READ, not on `parsed` (the + // root+workstream merge). An empty workstream file inheriting a non-empty + // root would otherwise report `resolved` while carrying no settings of its + // own — the opposite of the not-configured/configured-empty distinction + // ADR-1411 rule 3 requires. + const reason = fileHadKeys + ? CONFIG_REASON.RESOLVED + : CONFIG_REASON.CONFIGURED_EMPTY; + return { config: _baseConfig, source, degraded: false, reason }; } catch { // Fix 2: Early intercept — workstream requested but ws config.json absent (or dir absent) @@ -687,7 +855,7 @@ function loadConfigResolved(cwd: string, options: Record = {}): // This delivers the #1366 acceptance criterion: nonexistent GSD_WORKSTREAM yields root, degraded. if (wsRequested && rootParsed) { const fb = loadConfigResolved(cwd, { workstream: null }); - return { config: fb.config, source: 'root', degraded: true }; + return fallback({ config: fb.config, source: 'root', degraded: true }); } // Branch B, C, D, E @@ -696,22 +864,29 @@ function loadConfigResolved(cwd: string, options: Record = {}): // Branch B: workstream requested but ws config.json absent; root config present. // (Only reached when wsRequested is false — e.g. ws='' with .planning/workstreams//config.json) const fb = loadConfigResolved(cwd, { workstream: null }); - return { config: fb.config, source: 'root', degraded: true }; + return fallback({ config: fb.config, source: 'root', degraded: true }); } // Branch C: .planning/ exists but no config.json and no root config — federated/builtin defaults try { - return { config: _applyFederatedOverlay(defaults, {}, cwd), source: 'builtin-defaults', degraded: false }; + return fallback({ config: _applyFederatedOverlay(defaults, {}, cwd), source: 'builtin-defaults', degraded: false }); } catch { - return { config: defaults, source: 'builtin-defaults', degraded: false }; + return fallback({ config: defaults, source: 'builtin-defaults', degraded: false }); } } // Branch D or E: no .planning/ try { const home = process.env['GSD_HOME'] || os.homedir(); const globalDefaultsPath = path.join(home, '.gsd', 'defaults.json'); - const raw = platformReadSync(globalDefaultsPath); - if (raw === null) throw new Error('missing'); - const globalDefaults = JSON.parse(raw) as Record; + const globalRead = _readConfigFile(globalDefaultsPath); + if (globalRead.kind === 'fault') { + // ~/.gsd/defaults.json is present but unusable. Only report it when the + // project config did not already fail — the nearer file is the one the + // user is most likely to be able to act on. + if (!configFault) configFault = globalRead.fault; + _warnUnusableConfig(globalRead.fault); + } + if (globalRead.kind !== 'ok') throw new Error('global defaults absent or unusable'); + const globalDefaults = globalRead.data; const _globalBaseCfg: Record = { ...defaults, model_profile: (globalDefaults['model_profile']) ?? defaults.model_profile, @@ -749,16 +924,16 @@ function loadConfigResolved(cwd: string, options: Record = {}): }; // Branch D: global-defaults try { - return { config: _applyFederatedOverlay(_globalBaseCfg, globalDefaults, cwd), source: 'global-defaults', degraded: false }; + return fallback({ config: _applyFederatedOverlay(_globalBaseCfg, globalDefaults, cwd), source: 'global-defaults', degraded: false }); } catch { - return { config: _globalBaseCfg, source: 'global-defaults', degraded: false }; + return fallback({ config: _globalBaseCfg, source: 'global-defaults', degraded: false }); } } catch { // Branch E: no global defaults try { - return { config: _applyFederatedOverlay(defaults, {}, cwd), source: 'builtin-defaults', degraded: false }; + return fallback({ config: _applyFederatedOverlay(defaults, {}, cwd), source: 'builtin-defaults', degraded: false }); } catch { - return { config: defaults, source: 'builtin-defaults', degraded: false }; + return fallback({ config: defaults, source: 'builtin-defaults', degraded: false }); } } } @@ -775,6 +950,8 @@ function loadConfig(cwd: string, options: Record = {}): Record< export = { loadConfig, loadConfigResolved, + CONFIG_REASON, + _warnedUnusableConfig, isGitIgnored, CONFIG_DEFAULTS, _getConfigDefault, diff --git a/tests/config-loader.test.cjs b/tests/config-loader.test.cjs index e1335c499..e0a545528 100644 --- a/tests/config-loader.test.cjs +++ b/tests/config-loader.test.cjs @@ -23,6 +23,7 @@ const fs = require('node:fs'); const path = require('node:path'); const os = require('node:os'); const { cleanup } = require('./helpers.cjs'); +const fc = require('fast-check'); // ─── module under test ──────────────────────────────────────────────────────── @@ -1092,3 +1093,165 @@ describe('bug-3523 — CJS↔SDK contract: both agree on legacy branching_strate }); }); } + +// ─── #1880: corrupt is not absent (ADR-1411 amendment) ──────────────────────── + +describe("loadConfigResolved — corrupt config is distinguishable from absent", () => { + let tmpDir; + let stderrLines; + let originalStderrWrite; + + beforeEach(() => { + tmpDir = makeTempProject(); + stderrLines = []; + originalStderrWrite = process.stderr.write.bind(process.stderr); + process.stderr.write = (chunk) => { stderrLines.push(String(chunk)); return true; }; + configLoader._resetRuntimeWarningCacheForTests(); + }); + + afterEach(() => { + process.stderr.write = originalStderrWrite; + if (tmpDir) cleanup(tmpDir); + tmpDir = null; + }); + + const configPath = (d) => path.join(d, ".planning", "config.json"); + const R = configLoader.CONFIG_REASON; + + // The repro from the issue: absent and malformed were byte-identical. + test("absent config resolves not_configured and is NOT degraded", () => { + const res = configLoader.loadConfigResolved(tmpDir); + assert.equal(res.degraded, false, "a missing config is legitimate absence"); + assert.equal(res.reason, R.NOT_CONFIGURED); + assert.equal(res.reason, "not_configured", "enum value is the wire contract"); + }); + + test("malformed config is degraded with reason config_unparseable", () => { + fs.writeFileSync(configPath(tmpDir), '{"model_profile":"budget",}', "utf-8"); + const res = configLoader.loadConfigResolved(tmpDir); + assert.equal(res.reason, R.CONFIG_UNPARSEABLE, + "a trailing comma must not read as \"no config here\""); + assert.equal(res.degraded, true, "corruption is a degraded resolution"); + }); + + test("absent and malformed no longer produce the same resolution", () => { + const absent = configLoader.loadConfigResolved(tmpDir); + configLoader._resetRuntimeWarningCacheForTests(); + fs.writeFileSync(configPath(tmpDir), '{"model_profile":"budget",}', "utf-8"); + const corrupt = configLoader.loadConfigResolved(tmpDir); + assert.notEqual(absent.reason, corrupt.reason, + "the whole defect: these two were indistinguishable"); + assert.notEqual(absent.degraded, corrupt.degraded); + }); + + test("unreadable config is degraded with reason config_unreadable", (t) => { + fs.writeFileSync(configPath(tmpDir), '{"model_profile":"budget"}', "utf-8"); + // Deterministic IO fault via fs monkeypatch, restored in t.after() — never + // chmod 0o000, which root bypasses (CLAUDE.md cross-platform IO rule). + const realRead = fs.readFileSync; + t.after(() => { fs.readFileSync = realRead; }); + fs.readFileSync = (f, ...rest) => { + if (String(f).endsWith("config.json")) { + const e = new Error("EACCES: permission denied"); e.code = "EACCES"; throw e; + } + return realRead(f, ...rest); + }; + const res = configLoader.loadConfigResolved(tmpDir); + assert.equal(res.reason, R.CONFIG_UNREADABLE); + assert.equal(res.degraded, true); + }); + + // The wiring clause: loadConfig returns .config alone to ~51 call sites, so + // without a diagnostic the reason field is unreachable to nearly every consumer. + test("the plain loadConfig path still surfaces the cause on stderr", () => { + fs.writeFileSync(configPath(tmpDir), '{"model_profile":"budget",}', "utf-8"); + configLoader.loadConfig(tmpDir); + assert.equal(configLoader._warnedUnusableConfig.size, 1, + "a loadConfig caller must still get a signal it can act on"); + }); + + test("the diagnostic is deduplicated across repeat loads", () => { + fs.writeFileSync(configPath(tmpDir), '{"model_profile":"budget",}', "utf-8"); + configLoader.loadConfig(tmpDir); + configLoader.loadConfig(tmpDir); + configLoader.loadConfig(tmpDir); + assert.equal(configLoader._warnedUnusableConfig.size, 1, "keyed on path+errno, warned once"); + }); + + // Regression: found by isolated review of the first cut of this fix. The + // success path returned early WITHOUT consulting configFault, so a corrupt + // ROOT config whose workstream override happened to parse reported + // degraded:false / resolved — the same silent-discard defect this issue + // exists to close, reappearing for any project that uses workstreams. + test("a corrupt ROOT config still degrades when the workstream config parses", () => { + const wsDir = path.join(tmpDir, ".planning", "workstreams", "ws-a"); + fs.mkdirSync(wsDir, { recursive: true }); + fs.writeFileSync(configPath(tmpDir), '{"model_profile":"budget",}', "utf-8"); + fs.writeFileSync(path.join(wsDir, "config.json"), '{"mode":"autonomous"}', "utf-8"); + const res = configLoader.loadConfigResolved(tmpDir, { workstream: "ws-a" }); + assert.equal(res.reason, R.CONFIG_UNPARSEABLE, + "the root config was discarded — that must not report as a clean resolve"); + assert.equal(res.degraded, true); + }); + + // Regression: reason was computed from the root+workstream MERGE, so an empty + // workstream file inheriting a non-empty root reported "resolved" despite + // carrying no settings of its own. + test("an empty workstream file inheriting a non-empty root is configured_empty", () => { + const wsDir = path.join(tmpDir, ".planning", "workstreams", "ws-b"); + fs.mkdirSync(wsDir, { recursive: true }); + fs.writeFileSync(configPath(tmpDir), '{"model_profile":"quality"}', "utf-8"); + fs.writeFileSync(path.join(wsDir, "config.json"), "{}", "utf-8"); + const res = configLoader.loadConfigResolved(tmpDir, { workstream: "ws-b" }); + assert.equal(res.reason, R.CONFIGURED_EMPTY, + "emptiness is a property of the file read, not of the merged result"); + assert.equal(res.degraded, false, "an empty file is not corruption"); + }); + + // Found by the property test below: valid JSON that is not an OBJECT parsed + // "ok", then threw downstream, and the outer catch reported not_configured — + // a present file indistinguishable from an absent one, the exact defect this + // issue closes. Shape is now validated at the read seam (ADR-227). + for (const body of ["0", '"a string"', "[]", "null", "true"]) { + test(`valid JSON that is not an object is unusable, not absent: ${body}`, () => { + fs.writeFileSync(configPath(tmpDir), body, "utf-8"); + const res = configLoader.loadConfigResolved(tmpDir); + assert.equal(res.reason, R.CONFIG_UNPARSEABLE, + "a present-but-unusable file must never report as not_configured"); + assert.equal(res.degraded, true); + }); + } + + // Property test (CONTRIBUTING.md: parsers require >=1 fast-check property). + // The classification is total and mutually exclusive: any byte string is + // exactly one of resolved/configured_empty (parses) or config_unparseable. + test("classification is total and never reports a corrupt file as absent", () => { + fc.assert( + fc.property(fc.string(), (body) => { + configLoader._resetRuntimeWarningCacheForTests(); + fs.writeFileSync(configPath(tmpDir), body, "utf-8"); + const res = configLoader.loadConfigResolved(tmpDir); + let parses = true; + try { const v = JSON.parse(body); parses = v !== null && typeof v === "object" && !Array.isArray(v); } + catch { parses = false; } + // The invariant that matters: a file that is PRESENT is never reported + // as not_configured, whatever its bytes. + assert.notEqual(res.reason, R.NOT_CONFIGURED); + if (!parses) assert.equal(res.reason, R.CONFIG_UNPARSEABLE); + return true; + }), + { numRuns: 200, seed: 1880 }, + ); + }); + + // Contract markers required by scripts/lint-resolution-provenance.cjs: + // configured_empty and not_configured must stay distinguishable. + test("an empty config object is configured_empty, not not_configured", () => { + fs.writeFileSync(configPath(tmpDir), "{}", "utf-8"); + const res = configLoader.loadConfigResolved(tmpDir); + assert.equal(res.reason, R.CONFIGURED_EMPTY, + "configured_empty and not_configured must be distinguishable (ADR-1411 rule 3)"); + assert.equal(res.reason, "configured_empty", "enum value is the wire contract"); + assert.equal(res.degraded, false, "an empty file is not corruption"); + }); +}); diff --git a/tests/mutation-workflow-base-ref.test.cjs b/tests/mutation-workflow-base-ref.test.cjs index 5cf0de997..8418f807b 100644 --- a/tests/mutation-workflow-base-ref.test.cjs +++ b/tests/mutation-workflow-base-ref.test.cjs @@ -193,8 +193,17 @@ describe('#2452 CI gates: base-ref fetch must preserve ancestry', () => { git(dir, ['remote', 'add', 'origin', origin]); git(dir, ['fetch', '--quiet', 'origin', 'feature']); git(dir, ['checkout', '--quiet', 'FETCH_HEAD']); - git(dir, ['fetch', '--quiet', 'origin', 'base', ...baseFetchArgs]); + // The FETCH is inside the try, not before it. A base fetch whose shallow + // boundary lands short of the merge base can fail during the FETCH itself + // ("unable to parse commit" — the boundary commit's parent is unavailable) + // rather than succeeding and leaving the DIFF to fail with "no merge base". + // Which of the two git picks is version/transport dependent: this test + // passed on ubuntu-22 and windows-24 and failed on ubuntu-24 for the same + // commit. Both outcomes mean the same thing for what this guard protects — + // a shallow base ref cannot resolve the three-dot diff — so both are + // recorded as ok:false instead of one of them escaping as a crash. try { + git(dir, ['fetch', '--quiet', 'origin', 'base', ...baseFetchArgs]); return { ok: true, out: git(dir, ['diff', '--name-only', 'origin/base...HEAD']).trim() }; } catch (err) { return { ok: false, err: String(err.stderr || err.message) }; @@ -210,11 +219,12 @@ describe('#2452 CI gates: base-ref fetch must preserve ancestry', () => { 'If this stops holding, the #2452 mechanism no longer reproduces and ' + 'this guard needs revisiting.', ); - assert.match( - depth1.err, - /no merge base/i, - 'Expected git to report "no merge base" for a depth-1 base ref', - ); + // Asserting git's exact wording couples this guard to a git version: + // "no merge base" (diff-time) and "unable to parse commit" (fetch-time) + // are the same condition reported at different stages. CONTRIBUTING also + // prohibits raw text matching on subprocess output — the typed outcome + // above (`ok === false`) IS the contract this test exists to pin. + assert.ok(depth1.err.length > 0, 'a failed shallow diff must report a cause'); // (b) BOUNDARY, just below: the merge base is one commit out of reach. // This is the changeset-required.yml / docs-required.yml --depth=50 case @@ -226,7 +236,7 @@ describe('#2452 CI gates: base-ref fetch must preserve ancestry', () => { `Expected FAIL at --depth=${MERGE_BASE_DEPTH - 1} (merge base one commit ` + 'beyond the shallow boundary) — this is why a bounded cushion is not a fix', ); - assert.match(below.err, /no merge base/i); + assert.ok(below.err.length > 0, 'a failed shallow diff must report a cause'); // (c) BOUNDARY, exactly deep enough: the merge base is the last commit in. const atDepth = runnerDiff('runner-depth-at', [`--depth=${MERGE_BASE_DEPTH}`]);