Skip to content

[CTX-1339] fix(config): fall back to built-in config on first-start failure (#2225) - #2228

Merged
Xuepoo merged 2 commits into
mainfrom
ctx-1339/fix-first-start-config-fallback
Oct 18, 2026
Merged

Xuepoo merged 2 commits into
mainfrom
ctx-1339/fix-first-start-config-fallback

Conversation

@Xuepoo

@Xuepoo Xuepoo commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Priority: P1 | Area: area:config | Labels: fix,P1,area:config | Milestone: v0.1.0 | RFC: OQ-010 | Task: CTX-1339

Closes #2225

What

Implements the accepted configuration-model clause: on first start with a present-but-broken user config, proceed with the minimal built-in configuration and report diagnostics; on later failures retain the last good plan.

Behavior

  • First start + broken user file → unmodified CoreDefaults plan (source=default), original error plus first-start diagnostic, startup proceeds.
  • Later reload failure → last good plan retained (applied=false).
  • Explicitly missing --config/BITTY_CONFIG → exit 2 (unchanged).
  • Invalid --theme or invalid profile with a valid file → exit 2, attributed to CLI/profile, no fallback (merge errors are classified causally, not by file presence).
  • Safe mode, consent, CLI surface, and schema unchanged.

Verification

  • fmt, clippy -D warnings (bitty-terminal/bitty-config, all targets), Windows GNU all-targets: pass
  • cargo nextest run -p bitty-terminal -p bitty-config --locked: 1651 passed
  • Mutation: reverting the classifier to file-presence-only makes the invalid-profile regression exit 0 with fallback text and fail
  • Rebased on e1f6920d; merged ghost tests preserved

Non-goals

Profiles D1-D5 remain open; automatic watcher polling for a broken file after fallback is a restart/ctl config reload concern, tracked separately.

Summary by CodeRabbit

  • Bug Fixes
    • On first startup, an unusable default config now produces a diagnostic and falls back to built-in defaults when recovery is safe.
    • Missing explicitly specified config files, invalid themes, and profile errors still stop startup with an error instead of silently using defaults.
    • Invalid CLI theme values are reported at appearance.theme before configuration layers are merged.
    • Config reload errors preserve the last successfully applied settings.

@Xuepoo Xuepoo added this to the v0.1.0 milestone Oct 18, 2026
@Xuepoo Xuepoo added fix Bug fix P1 Priority: high area:config Area: configuration labels Oct 18, 2026
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI (base), Organization UI (inherited)
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2c695d23-4027-4968-9664-afa690f63f69
📥 Commits

Reviewing files that changed from the base of the PR and between e1f6920 and 1fda0eb.

📒 Files selected for processing: 7
  • crates/bitty-config/src/file.rs
  • crates/bitty-config/src/plan.rs
  • crates/bitty-terminal/src/config_cli.rs
  • crates/bitty-terminal/src/config_reload.rs
  • crates/bitty-terminal/src/main.rs
  • crates/bitty-terminal/src/tests.rs
  • crates/bitty-terminal/tests/cli_config_fallback.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Startup now falls back to built-in defaults with diagnostics when a recoverable user-config failure occurs on first start. Fatal configuration errors remain errors. Reload failures retain the last successfully committed plan. CLI theme overrides are validated before configuration merging.

Changes: Startup configuration handling

Layer / File(s) Summary
Validate overrides and classify load failures
crates/bitty-config/src/file.rs, crates/bitty-terminal/src/config_cli.rs, crates/bitty-terminal/src/tests.rs
CLI themes are validated before merge, and invalid values report the appearance.theme path. Configuration loading classifies failures by cause, including whether the user file caused a merge error.
Apply first-start fallback and retain last-good plans
crates/bitty-terminal/src/config_cli.rs, crates/bitty-terminal/src/main.rs, crates/bitty-terminal/src/config_reload.rs, crates/bitty-terminal/src/tests.rs, crates/bitty-terminal/tests/cli_config_fallback.rs, crates/bitty-config/src/plan.rs
On first start, recoverable user-file failures load built-in defaults and produce diagnostics. Fatal errors still fail startup. Reload errors leave the committed plan unchanged. Tests cover these cases and the existing valid-config and missing-default-file behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant main
  participant load_startup_config
  participant builtin_app_config
  main->>load_startup_config: Load startup configuration
  load_startup_config->>builtin_app_config: Build defaults after a recoverable first-start failure
  builtin_app_config-->>load_startup_config: Return built-in configuration
  load_startup_config-->>main: Return configuration and diagnostic
Loading
Merge Risk: ⚪ Minimal · up to 1fda0

This change makes startup fall back to built-in defaults with diagnostics when the user config is broken on first start. Missing explicit config paths, invalid themes and invalid profiles still fail with exit 2. No concrete merge-blocking risk was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: first-start fallback to the built-in configuration after a configuration failure.
Linked Issues check Passed Issue #2225 requires first-start recovery for a present broken config, diagnostics, and retention of the last good plan on later failure. load_startup_config falls back only for a classified present…
Out of Scope Changes check Passed The changes stay within issue #2225 and the stated PR intent. Failure classification for CLI and profile layers prevents unrelated errors from being absorbed by the first-start fallback. The related v…
Docstring Coverage Passed Docstring coverage is 91.67% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 48 functions across 7 files.
Full details: Linked Issues check

Explanation

Issue #2225 requires first-start recovery for a present broken config, diagnostics, and retention of the last good plan on later failure. load_startup_config falls back only for a classified present-but-unusable user file and builds the unmodified built-in plan with source=default. The binary test covers successful startup, the original error, and the fallback diagnostic. The reload test verifies load-error, applied=false, and preservation of the committed plan. Explicitly missing paths, invalid themes, and invalid profiles remain fail-closed in the added tests.

Full details: Out of Scope Changes check

Explanation

The changes stay within issue #2225 and the stated PR intent. Failure classification for CLI and profile layers prevents unrelated errors from being absorbed by the first-start fallback. The related validation and regression tests verify that boundary. No changes target consent, safe mode, schema, or profile selection behavior.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Xuepoo
Xuepoo merged commit 42535e2 into main Oct 18, 2026
17 checks passed
@Xuepoo
Xuepoo deleted the ctx-1339/fix-first-start-config-fallback branch October 18, 2026 20:22

@Xuepoo Xuepoo left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independent review verdict (separate agent, guard cron hermes-planner).

Verdict: APPROVE.

Checks performed:

  • git show 1fda0ebc --stat + cumulative gh pr diff 2228
  • LoadFailure classified broken (present-but-unusable user file -> fallback) vs fatal (missing --config/BITTY_CONFIG, missing/invalid profile, CLI usage error, invalid built-in plan -> fail-closed).
  • Follow-up commit tightens causality: merge error blamed on the file only when re-merging without the user layer succeeds; --theme pre-validated against preset registry.
  • No silent swallowing: the without-user re-merge probe discards its error via .is_ok() deliberately and the original err is preserved in every branch.
  • Tests: unit tests for classify_merge_failure, rewritten fail-closed test, --theme CLI-overrides case, cli_config_fallback.rs integration growth.

Diff scoped to bitty-config + bitty-terminal; no unrelated churn. CI green on head 1fda0eb. Merge gate satisfied (comment-LGTM + green CI).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:config Area: configuration fix Bug fix P1 Priority: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(config): first start with a broken config exits 2 instead of proceeding with built-in configuration

1 participant