config-merge: a malformed config file silently reverts all settings to defaults (indistinguishable from absent) #39

New issue

Closed

opened 2026-07-23 14:26:05 +00:00 by crenshawdev · 0 comments

crenshawdev commented

2026-07-23 14:26:05 +00:00

(Migrated from github.com)

Severity: medium · Area: config · surfaced in the post-v1.2.0 cross-model review sweep. Independently flagged by two reviewers (highest-confidence finding of the sweep).

What happens

The shared config reader cannot distinguish an absent config layer from a malformed one — both collapse to a dropped layer with no signal. A syntax error in .planning/config.json (a stray trailing comma, say) silently removes the entire repo config layer, and every setting reverts to its default: routing tier back to balanced, git rails back to built-ins, memory backend back to builtin. The only trace is the source field reading defaults / global instead of repo — which no consumer treats as an error.

Its most serious consequence chains into #38: a malformed config silently reverts the git-guard safety rail to defaults as well.

Mechanism (verified against source)

cadence-core/bin/lib/config-merge.mjs:21-24:

export function readJSON(file) {
  try { return JSON.parse(readFileSync(file, 'utf8')); }
  catch { return null; }
}

IO error (absent) and SyntaxError (malformed) both return null.

mergeLayers (config-merge.mjs:47-57) pushes a layer name only when the layer is truthy, so a malformed repo layer is indistinguishable from a missing one; both yield the merged result of whatever remains, and source never reflects that a layer failed to parse.

Design note

The swallow is deliberate and defensible — "a bad layer is skipped, never fatal (the spine must not block on config)" — and this issue is not a request to make it fatal. The defect is the total silence: a broken config should be visible, not invisibly ignored.

Fix direction

Keep the fail-safe, but distinguish a parse failure from absence and surface it — e.g. a one-line stderr warning naming the file, and/or a distinguishable source value such as defaults (repo config failed to parse) — so the operator can tell "settings not applied because the file is broken" from "no repo config present."

**Severity:** medium · **Area:** config · surfaced in the post-v1.2.0 cross-model review sweep. Independently flagged by two reviewers (highest-confidence finding of the sweep). ## What happens The shared config reader cannot distinguish an **absent** config layer from a **malformed** one — both collapse to a dropped layer with no signal. A syntax error in `.planning/config.json` (a stray trailing comma, say) silently removes the entire repo config layer, and every setting reverts to its default: routing tier back to `balanced`, git rails back to built-ins, memory backend back to `builtin`. The only trace is the `source` field reading `defaults` / `global` instead of `repo` — which no consumer treats as an error. Its most serious consequence chains into #38: a malformed config silently reverts the `git-guard` safety rail to defaults as well. ## Mechanism (verified against source) - `cadence-core/bin/lib/config-merge.mjs:21-24`: ```js export function readJSON(file) { try { return JSON.parse(readFileSync(file, 'utf8')); } catch { return null; } } ``` IO error (absent) and `SyntaxError` (malformed) both return `null`. - `mergeLayers` (`config-merge.mjs:47-57`) pushes a layer name only when the layer is truthy, so a malformed repo layer is indistinguishable from a missing one; both yield the merged result of whatever remains, and `source` never reflects that a layer failed to parse. ## Design note The swallow is deliberate and defensible — "a bad layer is skipped, never fatal (the spine must not block on config)" — and this issue is **not** a request to make it fatal. The defect is the total silence: a broken config should be visible, not invisibly ignored. ## Fix direction Keep the fail-safe, but distinguish a parse failure from absence and surface it — e.g. a one-line stderr warning naming the file, and/or a distinguishable `source` value such as `defaults (repo config failed to parse)` — so the operator can tell "settings not applied because the file is broken" from "no repo config present."