fix(config): boot with duplicate site meters demoted instead of crash-looping - #991
Conversation
…-looping A config with two is_site_meter: true drivers crash-looped Core at boot since #844, before the boot-phase HTTP listener binds — the operator loses the very UI they would fix the config with. Field incident 2026-08-29: a driver install on v1.15.0 left two site meters, and the update to v2.3.0 took the box dark until SSH. Parse now keeps the first declared site meter — the same driver older versions silently dispatched against — clears the flag on the rest, and records a LoadWarning that main logs at ERROR so it reaches the log ring and the help report. Validate keeps rejecting the ambiguity, so Settings save and bootstrap stay strict; only a file already on disk is repaired. Closes #988. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C7xyXX1HYi2i1aax8EsCFx
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d33fb18718
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| c.AppLink = &AppLink{Enabled: false} | ||
| } | ||
| applyDefaults(&c) | ||
| c.demoteExtraSiteMeters() |
There was a problem hiding this comment.
When a duplicate is introduced while the service is already running, configreload.Watcher.reload calls config.Load and immediately applies the returned config without inspecting LoadWarnings; the only warning loop added by this commit runs once during startup in main. Consequently, a hot edit can be silently repaired—and, if the new duplicate precedes the old meter, can switch the active site-meter driver—without the promised error naming the ambiguity ever reaching the log ring or support report. Log these warnings in the watcher/apply load path as well.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d33fb18. Configure here.
| // support report, without stopping a boot the repair made safe. | ||
| for _, w := range cfg.LoadWarnings { | ||
| slog.Error(w) | ||
| } |
There was a problem hiding this comment.
Hot reload swallows site-meter warnings
Medium Severity
LoadWarnings are only logged in main after the initial config.Load. Watcher.reload also calls config.Load, which demotes extra is_site_meter flags, then applies the result with no ERROR. A live config.yaml edit that leaves two flags now hot-reloads the first driver silently, so the log ring and help report never see the ambiguity after boot.
Reviewed by Cursor Bugbot for commit d33fb18. Configure here.
There was a problem hiding this comment.
Left a non-blocking comment: Cursor Bugbot’s check skipped and its review still has one unresolved finding (hot-reload LoadWarnings). Not approving; assigned HuggeK and Sanjin-Maker for human review.
Sent by Cursor Approval Agent: Pull Request Router and Approver
1 participant
Closes #988.
What
A config with two
is_site_meter: truedrivers has crash-looped Core at boot since #844 — and theos.Exit(1)fires before the boot-phase HTTP listener binds, so the box goes completely dark: host pings, no UI, no Settings to fix the config with. Today's field incident (10 minutes after v2.3.0 stable published): a driver install on v1.15.0 had leftis_site_meter: trueon bothpixiiand the neweasee-cloud; v1.15.0 ran fine (first match silently won), the update to v2.3.0 bricked the box until SSH.Validation strictness now depends on the door:
Parse— boot and anything re-reading the file):demoteExtraSiteMeterskeeps the first declared site meter — the identical driver older versions silently dispatched against — clears the flag on the rest, and records aLoadWarning.mainlogs each warning at ERROR, so it lands in the log ring and the help report instead of being silent (fix(config): reject >3 fuse phases and duplicate site meters #844's original complaint) or fatal (fix(config): reject >3 fuse phases and duplicate site meters #844's overshoot).api.go, bootstrapPOST /api/config): unchanged — both callValidate()directly, which still rejects the duplicate with the same message. An operator present at a form gets stopped; a box booting an old file does not.Zero site meters still fails at load: that severity predates 1.0 and no working box ever booted without one.
Behavior, before → after
is_site_meter: trueTesting
TestLoadDemotesDuplicateSiteMetersAndWarns— Göran-shaped config loads, first declared wins, flag cleared on the second, warning names both drivers.TestValidateRejectsDuplicateSiteMeters— the write path stays strict.make verifygreen.Follow-ups filed from the same incident: #989 (rollback does not survive a Restart — this is why the box came back up on the failed version at all) and #990 (CI boots the candidate against data fixtures from every published stable, so an old box's config meets the new version before a tag exists).
🤖 Generated with Claude Code
https://claude.ai/code/session_01C7xyXX1HYi2i1aax8EsCFx
Note
Medium Risk
Changes which driver is treated as the site meter at boot when duplicates exist (first wins); wrong meter could affect dispatch until config is fixed, but avoids total UI loss from crash-loops.
Overview
Boot no longer fails when
config.yamlhas more than one driver withis_site_meter: true. On load,demoteExtraSiteMeterskeeps the first declared site meter (matching older “first match wins” behavior), clears the flag on the rest, and appends human-readable messages toLoadWarnings.mainlogs each warning at ERROR so it shows up in the log ring and support bundle without aborting startup.Write path is unchanged:
Validate()still rejects duplicate site meters, so Settings save and bootstrap cannot persist the bad state.Tests now expect load-time repair plus warnings, with a separate test that
Validatealone still errors on duplicates.Reviewed by Cursor Bugbot for commit d33fb18. Bugbot is set up for automated code reviews on this repo. Configure here.