Skip to content

fix(config): surface configuration errors in TUI and CLI - #481

Merged
shuv1337 merged 12 commits into
integration-v2from
fix-surface-config-errors
Nov 10, 2026
Merged

shuv1337 merged 12 commits into
integration-v2from
fix-surface-config-errors

Conversation

@shuv1337

@shuv1337 shuv1337 commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

Closes #476.

  • Retain rejected configuration documents and invalid-setting diagnostics in the existing config pipeline, exposed through config.get as document.experimental.diagnostics. Rejected files contribute no settings; valid siblings still load.
  • Include source path, JSON path, parse/schema explanation, and line/column when available. Positions refer to the original file before substitutions, including array items. Schema diagnostics omit input values so credentials are not printed.
  • Show a TUI error toast at startup and after changes, with an action opening the persistent Status diagnostics. Existing config.updated refreshes replace/clear diagnostics after repair or deletion; unchanged errors do not repeatedly toast. debug config also retains the diagnostics.
  • Config-consuming CLI paths always report diagnostics on stderr. mcp list, models, and debug config exit 1 while preserving available output; run only refuses a whole-file diagnostic (`## Summary

Closes #476.

  • Retain rejected configuration documents and invalid-setting diagnostics in the existing config pipeline, exposed through config.get as document.experimental.diagnostics. Rejected files contribute no settings; valid siblings still load.
  • Include source path, JSON path, parse/schema explanation, and line/column when available. Positions refer to the original file before substitutions, including array items. Schema diagnostics omit input values so credentials are not printed.
  • Show a TUI error toast at startup and after changes, with an action opening the persistent Status diagnostics. Existing config.updated refreshes replace/clear diagnostics after repair or deletion; unchanged errors do not repeatedly toast. debug config also retains the diagnostics.
    ). Auth, logout, MCP auth/logout, and debug agents warn and continue.
  • Fix the independent false-empty mcp list result by waiting on the client for initial plugin activation before listing. A timeout, disabled MCP config loader, or failed activation is reported explicitly. Server reads remain nonblocking.

Root causes

  1. Config.parseInfo logged normalization warnings and discarded rejected files; neither startup clients nor hot-reload subscribers had a diagnostic in their config read model. Schema normalization also discarded the validation explanation.
  2. mcp list immediately read a cold Location's catalog, before the asynchronous config MCP plugin registered its servers. The empty snapshot was indistinguishable from truly unconfigured MCP. The existing config plugin remains the sole server-registration path.

Feature flag / API

No feature flag: these are bug fixes. The additive diagnostics metadata is experimental. Client types were regenerated with bun run generate in packages/client; no generated files were edited by hand.

Verification

  • bun run check — passes (existing advisory lint warnings remain).
  • git diff --check — passes.
  • Core config + MCP suites — 406 passed.
  • Full CLI suite — 458 passed, 7 skipped.
  • Full TUI suite — 1,738 passed, 2 skipped.
  • Full client suite — 271 passed.
  • Focused config/normalization, CLI diagnostics/MCP, and TUI regression tests also passed 10 repeated runs.
  • Coverage: invalid JSON and JSONC; schema-invalid values; redaction; original-file and array positions; diagnostic-only hot reload; repair/deletion; startup and repeated TUI warnings; persistent status at 100 and 44 columns; real cold standalone MCP lists (configured and truly empty); stderr/exit status for invalid config; plugin startup timeout; bounded run config checks.

Visual

<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72" fill="none">
  <style>@keyframes notice{0%,100%{opacity:1}50%{opacity:.45}}.notice{animation:notice 2.4s ease-in-out infinite}@media(prefers-reduced-motion:reduce){.notice{animation:none}}</style>
  <rect x="1" y="1" width="238" height="70" rx="6" stroke="currentColor" opacity=".25"/>
  <path d="M12 17h54M12 27h37M12 37h46" stroke="currentColor" opacity=".35"/>
  <path d="M77 32h18m-5-5 5 5-5 5" stroke="currentColor"/>
  <g class="notice">
    <rect x="104" y="12" width="124" height="48" rx="4" stroke="currentColor"/>
    <text x="113" y="29" fill="currentColor" font-family="monospace" font-size="10">Config error</text>
    <text x="113" y="45" fill="currentColor" font-family="monospace" font-size="9" opacity=".75">opencode.json:3:2</text>
  </g>
</svg>

Review fixes

Nine follow-up commits, one per review item:

  1. 5adcf28b7a — Warn-and-continue by default; only MCP/model/config listings and whole-file run failures set exit 1. Real standalone tests prove a broken repository .opencode/opencode.json does not block auth list or auth logout. A valid disabled MCP server is listed even alongside an invalid setting, then the command exits 1.
  2. feb05d7532 — Exit cleanly after printing configuration errors, without a JavaScript stack trace.
  3. f75856dd19 — EOF errors identify the innermost unclosed object/array and its opening line, including unclosed mcp.servers.
  4. fc84dc7c8c — Parse error code and source offset come from the same error list; omit position when substitution changes the error.
  5. a87cda6472 — Report disabled MCP configuration loading as soon as the nonempty plugin list proves it absent.
  6. 705be1a075 — Zero-byte and whitespace-only configuration files behave as {}, with no diagnostic/toast.
  7. 9ec3c07966 — Remove empty $: prefixes and use readable permission expectations.
  8. 7d5290fc36 — Test unchanged diagnostics on config.updated do not toast again, and that a skipped setting still permits a successful run. Auth regression coverage is in item 1.
  9. 9ac8fd1c96 — Revert unrelated well-known block formatting.

Review-fix verification

  • bun run check: passed (39 typecheck tasks; advisory lint warnings remain).
  • git diff 9f50f426b0..HEAD --check: passed.
  • Core test/config plus test/permission.test.ts: 439 passed.
  • CLI auth/config, auth, MCP list, debug config, and all test/run suites: 71 passed.
  • TUI config diagnostics: 1 passed, plus 10/10 repeated runs.
  • Core config/normalization repeated 10 times: 1,130 passed.
  • CLI auth/config and MCP list repeated 10 times: 110 passed.
  • Tests used disposable state and local fixtures; no live credentials or service were changed. No merge or release.

@shuv1337
shuv1337 merged commit adaf309 into integration-v2 Nov 10, 2026
5 checks passed
@shuv1337
shuv1337 deleted the fix-surface-config-errors branch November 10, 2026 00:36
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.

Surface malformed configuration errors in the TUI and CLI

1 participant