fix(config): do not start setup over live Settings - #1175
Conversation
A missing leftover config.yaml is not first-run when state.db already holds settings/config_v1. Load recovers from the sibling database and InitializeStorage rewrites the locator. An edited leftover seed or a wizard document cannot replace live Settings; only an old-Core rollback save is imported. Closes #1153 Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_92598562-9a2f-49af-8c6a-77a2ddff57ad) |
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: 70685528b8
ℹ️ 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".
| // loadSettingsBesideSeed reads live Settings from state.db next to the seed. | ||
| // Schema 2 stores Settings in SQLite; a missing leftover YAML is not first-run. | ||
| func loadSettingsBesideSeed(seedPath string) (*Config, bool) { | ||
| database, err := filepath.Abs(filepath.Join(filepath.Dir(seedPath), "state.db")) |
There was a problem hiding this comment.
On the standard native deployment, deploy/ftw.service runs from /var/lib/ftw while passing /etc/ftw/config.yaml, so the default live database opened by main.go is /var/lib/ftw/state.db, not /etc/ftw/state.db. If that seed is deleted, this hard-coded sibling lookup misses the live Settings, Load returns the missing-file error, and main.go enters the setup wizard instead of recovering; the service cannot resume automatically and may be unable to recreate the root-owned /etc/ftw seed. The recovery lookup must use the deployment's runtime state location rather than assuming it is beside the config file.
Useful? React with 👍 / 👎.
| return false | ||
| } | ||
| // setup.js buildConfig hardcodes these; applyDefaults uses 2 / 3000 / 2. | ||
| return probe.Site.ControlIntervalS == 5 && probe.Site.SlewRateW == 500 && probe.Site.MinDispatchIntervalS == 5 |
There was a problem hiding this comment.
When a site was originally configured through the wizard, its legitimate configuration retains 5/500/5; an older Core that saves settings after rollback removes the locator and header but serializes those same nonzero values along with the operator's changes. This predicate therefore classifies that real rollback save as a wizard document, causing InitializeStorage to reload stale SQLite Settings and overwrite the YAML, silently discarding the changes the rollback-import path is intended to preserve. Wizard detection needs provenance that cannot also occur in an ordinary saved configuration.
Useful? React with 👍 / 👎.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_51f7e2ce-3991-49c5-96d3-9f728aada41f) |
1 participant
Closes #1153.
A missing leftover
config.yamlstarted the setup wizard even whenstate.dbalready held Settings. Finishing the wizard wrote a new YAML, andInitializeStoragetreated the hash mismatch as an old-Core rollback save and committed that document over livesettings/config_v1. The same path fired for an edited leftover seed that no longer carriedconfig_database.YAML is a locator and rollback seed after schema 2. Live Settings stay in SQLite.
Fix
All in
go/internal/config(nomain.go, nostatepackage, no DuckDB files). Open PRs touchconfig.gofield surface andmain.go; none touchstorage.go.Loadonly adds the missing-seed recovery path.Load: if the seed file is missing, readsettings/config_v1from siblingstate.db. First-run (no settings row) still fails, so the wizard still starts on a new box.isConfigMissingis unchanged;main.gonever reaches it when Load succeeds.InitializeStorage: a recovered boot withConfigDatabaseset rewrites the missing locator from SQLite.legacySavenow requires a real old-Core replacement YAML (no locator, no SQLite comments, not a wizard/default document). Wizard output is detected from the hardcodedsetup.jsloop defaults (control_interval_s: 5,slew_rate_w: 500,min_dispatch_interval_s: 5). In that case the locator is rewritten from live Settings instead of keeping the wizard bytes as a rollback seed.Tests
go test ./internal/config -count=1 -timeout 180sconfig.yaml, keepstate.dbwith Settings → Load succeeds, Settings unchanged, locator rewritten.config_databasedoes not override Settings.SaveAtomicdocument does not override Settings.TestUpgradeRetryImportsSettingsSavedByTheRolledBackCore) still imports.go test ./cmd/ftw -run 'TestConfigStorage|TestMissingDatabase' -count=1passes. Full./cmd/ftwhas unrelated nova key-directory mode failures on this host.Not changed
go/cmd/ftw/main.go/isConfigMissing(DuckDB feat(storage): use embedded DuckDB for primary history #1129 also editsmain.go).go/internal/state/**.Note
Medium Risk
Changes config bootstrap and import precedence at every Core start; mistakes could still clobber or block settings, though behavior is heavily regression-tested.
Overview
Fixes a path where a missing or misleading
config.yamlcould treat an already-configured box as first-run and overwrite live Settings in SQLite (e.g. after the setup wizard or an edited leftover seed).Loadnow falls back tosettings/config_v1in siblingstate.dbwhen the seed file is absent; empty DB with no settings still fails so true first-run setup is unchanged.InitializeStoragetreats YAML as a locator/rollback seed after schema 2: SQLite wins unless the on-disk bytes look like a genuine old-Core rollback save (legacyCoreSave). Wizard/default seeds (hardcoded5 / 500 / 5site loop defaults) and edited leftovers no longer re-import;writeSettingsLocatorrewrites the seed from live Settings instead. Recovery with a missing locator file also recreates the pointer YAML without touching stored config.New
storage_testcases cover deleted seed, empty DB, edited leftover, and wizardSaveAtomicoutput.Reviewed by Cursor Bugbot for commit edb727c. Bugbot is set up for automated code reviews on this repo. Configure here.