Skip to content

fix(config): warn instead of throw on validation failure#949

Merged
gfargo merged 1 commit into
mainfrom
fix/config-validation-non-fatal
May 14, 2026
Merged

fix(config): warn instead of throw on validation failure#949
gfargo merged 1 commit into
mainfrom
fix/config-validation-non-fatal

Conversation

@gfargo
Copy link
Copy Markdown
Owner

@gfargo gfargo commented May 14, 2026

User report: every coco command crashes with `Invalid project config` when the merged config (defaults + XDG + git + project + env) fails schema validation. The throw originated from `loadProjectJsonConfig` but the offending fields are often inherited from EARLIER layers (XDG, git config) — so the error pinned blame on the wrong source AND a stale field anywhere in the chain takes down the whole tool.

The error

```
Error: Invalid project config
cause: data/service must NOT have additional properties,
data/service/requestOptions/timeout must be number,
data/service/requestOptions/maxRetries must be number,
data/service must have required property 'endpoint',
data/service must match a schema in anyOf
```

Hit during `coco ui --repo /tmp/scenario` testing this morning. Reproducible against any real-world XDG config whose service shape has drifted from the strict schema. Adding ANY local `.coco.config.json` re-triggers validation of the merged result — inherits invalid XDG service — crashes.

Fix

`loadProjectJsonConfig` no longer throws. Instead:

  1. Logs a clear warning to stderr with:
    • The path of the local file that triggered validation
    • The specific schema errors
    • A note that validation runs against the MERGED config — the issue may be in any layer
  2. Applies the merge anyway so the user's intent flows through
  3. Lets downstream components guard locally against shapes they can't handle

Why this is the right answer

  • Throwing turns "you have a stale field somewhere" into "the tool fails to boot." Brutal cost for a recoverable situation.
  • Coco has sane defaults. A drifted service shape can fall back gracefully.
  • A downstream consumer that GENUINELY needs a valid service shape can throw a localized error at use-time with concrete next steps, rather than burying the user in opaque "Invalid project config" at boot.

Verified

Local repro of the user's exact error now produces:

```
[coco] Warning: config validation issues detected (continuing with merged config).
Local file: .coco.config.json
Schema issues: data/service must NOT have additional properties, ...
Fix the offending fields to silence this warning. The validation runs against
the merged result of every config source (defaults, XDG, git, project, env) —
the issue may be in any of those, not necessarily this file.
Graph Commit Date Author Message

  •  f13c3f2    2026-05-14  Coco Test  chore: baseline app scaffold  [HEAD -> main]
    
  •  a817234    2026-05-14  Coco Test  chore: initial commit
    

```

Warning fires (so the user knows to fix it eventually) but the tool keeps going.

Out of scope

The drifted schema itself. The error suggests the service-config schema has gotten stricter than real configs in the wild — worth an audit follow-up to either loosen the schema or document the new required shape. This PR just stops the bleed.

Test plan

  • `npm run lint` clean
  • `tsc --noEmit` clean
  • `npm run test:jest` — 1666 tests pass (+1 new regression test)
  • Manual: the user's blocked workflow (`coco ui --repo /tmp/conv-test`) now boots

User report: every coco command crashes with "Invalid project config" when the merged config (defaults + XDG + git + project + env) fails schema validation. The throw originated from `loadProjectJsonConfig` but the offending fields are often inherited from EARLIER layers (most commonly XDG / git config), so the error pinned the blame on the wrong source — and a stale field anywhere in the chain takes down the whole tool.

## What was broken

\`\`\`
Error: Invalid project config
  cause: data/service must NOT have additional properties,
         data/service/requestOptions/timeout must be number,
         data/service/requestOptions/maxRetries must be number,
         data/service must have required property 'endpoint',
         data/service must match a schema in anyOf
\`\`\`

Reproducible against the user's own XDG config (the service shape has drifted from the strict schema — extra fields + wrong number types). Adding ANY local \`.coco.config.json\` triggered re-validation of the merged result, which inherited the invalid XDG service. Every \`coco ui\`, \`coco log\`, \`coco commit\` ... crashed during boot.

## What this PR changes

\`loadProjectJsonConfig\` no longer throws on validation failure. Instead:

1. Logs a clear warning to stderr with:
   - The path of the local file that triggered the validation
   - The specific schema errors
   - A note that the validation runs against the MERGED config — the issue may be in any of: defaults, XDG, git, project, env (not necessarily the local file)
2. Applies the merge anyway so the user's intent flows through
3. Lets downstream components guard locally against shapes they can't handle (clearer error than "tool failed to boot")

## Why this is the right answer

- Throwing turns "you have a stale field somewhere in your config" into "the whole tool fails to boot." That's a brutal regression cost for a recoverable situation.
- Coco has sane defaults at every layer. A drifted service shape can fall back to default service behavior and the rest of the tool keeps working — commits, changelogs, PR flows, etc.
- If a future consumer GENUINELY can't operate without a valid service shape, it can throw a localized error at the moment of use with concrete next steps, rather than burying the user in an opaque "Invalid project config" at boot.

## Verified

- Local repro of the user's exact error: \`coco log --repo /tmp/conv-test\` (with their config that triggered the bug) now warns + outputs the log instead of crashing.
- New regression test: \`warns instead of throwing when the merged config fails validation\`.

## Out of scope

The drifted schema itself — the user's working config genuinely has fields the schema rejects. Could be the schema is too strict (recent service-config additions not yet reflected) OR genuine drift in the user's file. Worth a follow-up audit of the service schema vs what real configs look like in the wild. This PR just stops the bleed.

## Tests

- 1666 tests pass (one flaky parallel-execution test unrelated)
- New regression test covers the warn-not-throw path
@gfargo gfargo merged commit 7dfcb58 into main May 14, 2026
7 checks passed
@gfargo gfargo deleted the fix/config-validation-non-fatal branch May 14, 2026 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant