Repository navigation
fix(updater): restore rollback onto the configured database path - #1171
Conversation
Soft rollback copied every snapshot to /app/data/state.db and only deleted that file's WAL, so a custom State.Path or config_database kept its live sidecars. Decompress also discarded gzip.Reader.Close, which is how a truncated snapshot passed as valid. Closes #1158 Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com>
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. |
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_0ea3cfdb-7067-44af-ad52-b40317d6f770) |
![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: 061ce33736
ℹ️ 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".
| if err := s.runner(ctx, nil, "cp", "-a", containerID+":/app/data/config.yaml", tmpPath); err != nil { | ||
| return "state.db", nil |
There was a problem hiding this comment.
For a state-only rollback on a box using a custom database such as config_database: site.db, any error copying the live config is treated as proof that the default state.db is in use. The restore consequently writes the snapshot and removes WAL sidecars for state.db, then starts a healthy Core that is still using site.db and reports the rollback complete even though no live state was restored. Only a confirmed absent config should select the default; transport, permission, or nonstandard config-location failures need to abort or otherwise resolve the actual database path.
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_2e5799df-d4bc-417b-9fba-8bbc9bd5e0a1) |
1 participant
Summary
Soft rollback always copied
state.db.gzto/app/data/state.dband only deletedstate.db-wal/state.db-shm. A box whoseconfig_databaseorstate.pathis another filename kept the live WAL, and SQLite could replay old pages over the restored file.decompressGzipFilealso discardedgzip.Reader.Close(), so a truncated snapshot could be installed.This restore now:
config.yaml, or the live seed when the snapshot is state-only)gzip.Reader.Close()after consuming the stream and refuses a truncated or checksum-mismatched gzipSidecar-only in
go/cmd/ftw-updater. Distinct from #989 (compose pin) and #1140 (mutex).Closes #1158
Test plan
cd go && go test ./cmd/ftw-updater -count=1 -timeout 60sNote
Medium Risk
Changes rollback data-path and SQLite WAL handling in the updater sidecar; mistakes could corrupt or mis-target the live database, though safety backup recovery and new tests mitigate that.
Overview
Soft rollback in
ftw-updaterno longer always writesstate.db.gzto/app/data/state.dband only clearsstate.db-wal/state.db-shm. It now resolves the target filename from snapshotconfig.yaml(config_databaseoverstate.path, same as Core), or from the liveconfig.yamlwhen the rollback is state-only, then deletes that file’s WAL/SHM so SQLite cannot replay stale pages over the restored DB.restoreSnapshotFilesstages all snapshot files first, picks the DB name, then copies; invalid or non–/app/datapaths are rejected.Gzip restore now calls
gzip.Reader.Close()after the copy so truncated or CRC-badstate.db.gzfails before install (with safety rollback unchanged on failure).Tests and a patch changeset cover custom DB names, live-config fallback, corrupt gzip, and path validation.
Reviewed by Cursor Bugbot for commit 469b69f. Bugbot is set up for automated code reviews on this repo. Configure here.