feat(cli): add flats backup for a consistent online copy by rabbitson87 · Pull Request #238 · gosuda/flats

rabbitson87 · 2026-11-14T05:58:55Z

Closes #223.

What this adds

flats backup --out PATH copies a running or a stopped host into a new folder and writes a manifest with the Flats version and the size
and SHA-256 of every file. flats backup --verify PATH checks such a copy without a host. The design is the spec
host-backup.md, which is marked implemented.

The owner decided for the plan of #223 that a running host is never paused. The copy checks itself instead, and when a change during
the copy could make it inconsistent, the command fails with "run the backup again".

How it works:

Each SQLite database is a VACUUM INTO snapshot from a read-only connection (5 s busy timeout, up to 3 tries).
Write-once files (blobs, versions, snapshots) are copied in two passes around the snapshots. Guest files are copied last. A guest
file that keeps changing is kept and listed as unstable.
flats.db and the outbox databases of the boards, notebooks and documents that have an import cursor are snapshotted together. The
copy is then checked in both directions: the cursor must lie between what the outbox has acknowledged and what it has created
(acked <= import_seq <= last). A copy that fails the check is snapshotted again, up to 3 attempts.
A survey before and after the copy catches a flat that was created, renamed, deleted or restored during the copy. A changed
secret.key fails the copy.
The command writes to PATH.partial-<random> and renames it to PATH without replacing anything. A failed or interrupted backup
leaves nothing at PATH.
--verify opens every database with mode=ro&immutable=1, compares each file with the manifest, checks the outbox databases
against flats.db, checks that every blob a flat needs is in the copy, and checks that secret.key opens every sealed value. It
writes nothing.
--exclude-secret-key leaves secret.key out and keeps the recovery copy.
Exit status 2 for a usage mistake, 78 for a refusal, 1 for a failure.

Where the change is

internal/backup/: the new package (copy, survey, snapshot, outbox check, manifest, verify), with its tests.
internal/app/backupcmd.go, internal/cli/{cli.go,ops.go}, cmd/flats/main.go: the command.
Small helpers in existing packages: durable.Open (a shared-read open on Windows), store.OpenImmutable and store.ErrVersionMismatch,
notebook.RegisterCollation.
scripts/ci-race.sh: internal/backup joins the services race lane, because the snapshots of one attempt run in goroutines.
.gitignore: /flats.exe.
Docs: docs/configuration.md ("What to back up" names the command, the verify mode and the restore), docs/surfaces.md, the spec index.
No schema change, no new dependency, go.mod is unchanged.

Known limits

The spec states them.

The end-to-end test has no running document and no running server flat (they need the docs worker and a flat process). It writes a
board and a notebook all the time. A server flat's database and files are fixtures.
That test compares the restored board elements and notebook pages with the writes acknowledged before the backup began. It does not
read revision history. The coherence rule is tested in both directions at the unit level.
No test restores a legacy snapshot from a copy, and none tests an unwritable output folder.
Local checks ran on Windows, and on Linux with -race in a container. macOS and the CI jobs are covered by this PR's checks.

Task report

This is the task report that the development protocol asks for. The reviewer's full reports are kept by the author on a local disk.
That copy is not accessible to other reviewers, so the lines below are exact excerpts of the verdicts and of each finding's headline.
Nothing here replaces a verdict.

Scope: issue feat(cli): add flats backup for a consistent online copy #223, step 1 of the plan, stages design, implementation and testing. Base: origin/main at 69286e3.
Material author of the spec, the code and the tests: claude-sonnet-5-5.
Reviewer: requested claude-fable-5-1; in the execution metadata of every run, claude-fable-5-1 is the model that did the review work (the design runs also show a few small helper calls of the CLI to claude-haiku-5-5). The reviewer authored no part of the change.
Budget: at most 2 rounds per stage. Used: design 2, implementation 2, testing 2, plus one impact assessment after a CI failure (below),
which is one run beyond the plan of 2 rounds. Reviewer runs: 8 (design 2, implementation 3, testing 2, impact assessment 1). One implementation run stopped with an API usage limit (429) after 83 turns and gave no verdict, so it is an incomplete
review; I resumed the same session to finish round 1 and did not count it as a round.
Round 1 of the implementation review reached me with a diff against a stale local main. Finding I2 came from that error of mine, and I
rebutted it with the correct diff. Round 2 used the right base.

Verdicts and findings of each review round

Design, round 1: DESIGN: REJECT
- D1, blocker. Rules 12, 13, 30. Documents get the reverse order and no coherence check.
- D2, major. Rules 2 and 10. Legacy snapshots are never copied.
- D3, major. Rules 12 and 25. "Referenced" and "coherent" are not implementable without a host and are undefined for some states.
- D4, major. Rules 22, 23, 24, 26. The verify as specified writes into PATH.
- D5, major. Rules 17, 18, 23. The manifest check runs before the manifest exists.
- D6, major. Rules 2, 15 and the sentence "runs these steps and no other". No step names data/files/.
- D7, major. Rules 5 and 9. Links and special files are undefined.
- D8, minor. Rules 9 and 16. A false failure on a healthy host.
- D9, minor. Rule 32. The detection claim is wider than the surveys.
- D10, minor. Rule 5. A file that disappears during the copy is undefined.
- D11, minor. Rules 7 and 22, spec conventions, and the usage line.
- D12, minor. Completion criteria.
- D13, minor. Rules 3, 6, 8, 18. Exposure of the copy.
- D14, minor. Rule 1. Two copies of config.json.
- D15, minor. Rule 24. "Each SQLite database in the manifest" is undefined.
Design, round 2: DESIGN: ACCEPT
- D16, major. Rule 8, the host class and the databases. A healthy host fails the backup in three routine cases.
- D17, minor. The second completion criterion, second sentence. The test does not match its rule.
- D18, minor. Rule 16. Two statements are wrong for a document.
- D19, minor. Rule 17, first sentence. The stated exception cannot happen.
- D20, minor. docs/spec/README.md, the index row. The row is stale.
Implementation, round 1: IMPLEMENTATION: REJECT
- I1, blocker. flats.exe is committed.
- I2, major. The branch holds changes outside issue feat(cli): add flats backup for a consistent online copy #223 that no spec, design review or check covers.
- I3, major. --verify writes a file when the copy holds a -wal beside flats.db (rule 25).
- I4, major. The end-to-end test does not show that the restored host holds the writes acknowledged before the backup began, and the spec dropped that sentence without saying so.
- I5, minor. A write-once file can appear in both files and skipped.
- I6, minor. A preview directory that vanishes between ReadDir and Info fails the backup.
- I7, minor, wording-only. The interrupt criterion names step 21.
- I8, minor. A refusal can write a -shm into the data directory.
Implementation, round 2: IMPLEMENTATION: ACCEPT
Round 2 closed I1 to I8 and added:
- I9, minor, wording-only. Rule 31 contradicts rule 12.
- I10, minor. TestASnapshotThatIsBusy/"a later try succeeds" now has its margin at a boundary.
Testing, round 1: TESTING: ACCEPT
- T1, minor, wording-only in part. The comment of TestASnapshotThatIsBusy overstates its margin, and the test fails on a slow start.
- T2, minor. Rule 9 has no coverage on Windows.
- T3, minor. The retry path "a database that was not in the group joins it" has no test.
- T4, minor, wording-only. The skip count and the scope of the inventory do not agree.
Testing, round 2: TESTING: ACCEPT
Round 2 closed T1 to T4.
- no new findings
Impact assessment of the CI data race (run 8): IMPACT: ACCEPT
- R1, minor, wording-only. The PR description and the testing report say "no DATA RACE" without the CI result.
- R2, minor. The test init() hides a regression of the fix from the race lane.

Dispositions: every finding was accepted and corrected, except I2, which I rebutted with evidence (the error above). Optional suggestions were not all adopted. Not adopted: matching the busy error by its code, a separate count of coherence retries, a test of --json output of a failure, a direct test of rule 17, and running the checks of the other databases when flats.db is missing from a copy. The corrections that
came after an accepted round are covered by a later review: the corrections of the design round 2 by the implementation review, the
corrections of implementation round 2 by the testing review, and the corrections of testing round 1 by testing round 2. The implementation
reviewer accepted carrying the implementation approval to the final commit, whose difference from the snapshot that it accepted is the
extraction of the report printing in internal/app/backupcmd.go, two spec sentences and test files. After the CI data race below, the same reviewer assessed the fix and accepted the carry again. R1 is addressed by this description and R2 by a comment at the registration in Run.

A data race that CI found

The job race (services) failed on 5e095e6 with a data race, and the other 24 checks passed. The first snapshot of a notebook database registered the SQLite collation inside a goroutine of the group, while other goroutines of the group opened connections. The driver writes its table of collations under a mutex and reads it, in Driver.Open, without one, so the two raced.

My own -race runs before the PR (the Linux container, several times) did not show it, so my earlier statements of "no data race" held for those runs only. I could not reproduce the race locally with the old code in 60 runs, so the CI report is the direct evidence of it. The fix is in 4fe2a25: Run registers the collation once, before any goroutine starts, and the snapshot no longer registers it. A test file registers it in init as well, because the tests run in parallel in one process; without that, a run of the fix without the init raced once in 20 runs, in a test fixture. The consequence is that the race lane cannot catch a later move of this call into a goroutine, and the comment in Run says so.

The same hazard exists on main outside this change: the host registers the collation lazily at the first notebook open while other goroutines open databases. I did not touch it.

Verification

Results are on the commit that the row names. Nothing skipped or unavailable is counted as a pass.

Check	Result
`go build ./...`, `go vet ./...`, `make docs-check` equivalent	pass
Windows, `go test` of `internal/backup`, `cli`, `store`, `durable`, `app`, `docs` (on `5e095e6`; `backup`, `cli` and `app` run again on the final commit and pass)	876 pass, 0 fail, 19 skip. Three skips are of this change (symbolic links need a privilege on Windows) and pass on Linux; the other 16 are existing skips of other areas
Linux container (`golang:1.27.1`), `go test -race` of `internal/backup`, and the backup and secret-key tests of `app` and `cli`	On `4fe2a25`, the fix of the CI race (the last commit only adds a comment): 113 pass, 0 fail, 0 skip, no data race. The same kind of run before the fix did not show the race that CI found
The end-to-end backup and restore test, repeated	40 of 40 pass on Windows. Before the coherence change it failed in each of the 3 runs that I made
Mutation checks	Dropping either half of the coherence rule, the junction skip, or the group join each fails the tests that name them. Each new test of a review fix failed on the old code

Outside this change: internal/devreview fails on Windows without this branch too (symlink privilege, case folding). Whole-package
-race runs of cli and app in the container fail 5 tests that look up *.localhost names, which the container does not resolve.
Not run locally: -race on Windows and macOS. CI on 5e095e6: 24 of 25 checks passed (the windows and macOS jobs among them) and race (services) failed as described above. CI on 9aacb9d, the fix: all 25 checks passed, including race (services), and commit-images is skipped by design (it runs on pushes to main). Code scanning shows no open alert for this PR.

For the maintainer

The history is 17 commits. The repository squashes each PR into one commit, so the PR title is the message. The commits are small and
named by what they change, if you read them one by one.

The spec for issue #223. A copy of a running host is a directory with a manifest. Databases are snapshots, write-once blobs are copied in two passes, and the copy checks its own board and notebook coherence and fails to be run again when a change during the copy could make it inconsistent.

A snapshot of an outbox database and a snapshot of flats.db that start one after the other leave a gap, and a board or notebook that is written all the time writes into it on every attempt. The two now start together. The coherence check holds for either order of the two snapshots: the import cursor must lie between what the outbox has acknowledged and what it has created. A cursor above the outbox sequence and an outbox that is ahead of the cursor both fail it and start a new attempt. The check also runs in verify. The busy timeout and the size of the group are internal test options. internal/backup joins the services lane of the CI race tests, because the group runs in goroutines.

The app test backs up a host whose board and notebook are written all the time, restores the copy and starts a host from it. The CLI test checks that the exit statuses 2, 78 and 1 reach the process and that the help lists the command.

What to back up names the command, the verify mode, the restore and the limits of a copy of a running host. The CLI table lists the command.

A stopped host of an older release has an older flats.db. The command copied everything and then refused at the check of its own copy, with a message that named the wrong Flats version. It now refuses in the preflight, says that the host migrates flats.db when it starts, and names the copy by hand.

…before the backup The end-to-end test only checked that the restored host starts and that the next write gets a higher sequence. It now counts the board elements and the notebook pages of the restored host and compares them with the writes that the host had acknowledged before the backup began.

…d file once Verify opened flats.db through store.Inspect, which creates a -shm file when a -wal file lies beside the database. It now reports the stray file and opens nothing that can write. The schema check of the preflight moves after the --out refusals, so a refusal creates no -shm file beside the database of a stopped host. A flat's previews directory that is gone is skipped like what lies below it. A write-once file that the first pass skipped and the second pass copied is no longer listed as skipped. The busy-retry test takes a longer margin.

…nd 31 The lock of the busy-retry test now ends in the middle of the second try, with a margin on both sides that does not depend on timer precision. Rule 31 names the empty -shm that the schema refusal can leave, and rule 25 says that a copy with a stray -wal file fails with status 1.

…on Windows The busy-retry test holds a lock for seconds, so it no longer runs in parallel with the other tests of the package. The dangling-link case of the refusal test is a subtest, so a missing symlink privilege skips that case and not the whole test.

The report moves into its own method, and a test checks each line that rule 24 names for the three states of secret.key, with a skipped and an unstable path. The end-to-end test and the damaged-copy test check more of the output of rules 24 and 29. The first completion criterion of the spec is rewrapped.

… a notebook blob Findings of the testing review. A junction needs no privilege on Windows, so the Windows job now runs the case of rule 9 that the symbolic-link tests skip there. A database outside the group that the check finds is snapshotted again with flats.db, and a missing blob of a notebook fails the check, as one of a board does. The busy test keeps the lock for 4 s, which gives a margin of 2 s minus the start delay on the near side, and its comment says so.

CI found a data race with -race in the services lane. The first snapshot of a notebook database registered the collation inside a goroutine of the group, while other goroutines of the group opened connections. The collation table of the SQLite driver is a plain map, so the registration and the opens raced. Run now registers the collation once, in the calling goroutine, before any goroutine starts, and the snapshot no longer registers it. The tests run in parallel in one process, so a test file registers it before any test starts.

…egistration The test init registers the collation too, so a later change that moves the call in Run into a goroutine would pass the race lane. The comment tells a reader to check that by reading.

rabbitson87 added 19 commits November 14, 2026 01:53

docs(spec): correct the flats backup spec after design review round 1

a0daebb

docs(spec): add the busy retry criterion to the flats backup spec

7ea7c77

docs(spec): correct the flats backup spec after design review round 2

8d44f77

feat(cli): add flats backup and flats backup --verify

7934260

docs(backup): document flats backup and mark its spec implemented

bbd65c3

What to back up names the command, the verify mode, the restore and the limits of a copy of a running host. The CLI table lists the command.

docs(spec): rewrap rule 14 of the backup spec

4e4e2b8

build: ignore the Windows build of flats at the repository root

cc5bd93

docs(backup): say that the race lane cannot catch a moved collation r…

9aacb9d

…egistration The test init registers the collation too, so a later change that moves the call in Run into a goroutine would pass the race lane. The comment tells a reader to check that by reading.

rabbitson87 merged commit b09f5fd into main Nov 14, 2026
26 checks passed

rabbitson87 deleted the feat/flats-backup branch November 14, 2026 08:33

This was referenced Nov 14, 2026

Plan: finish the operations, security and Portal issues (#139, #221 to #225, #227) #237

Open

feat(secrets): keep the host key outside the data directory with host.secret_key_path #239

Open

Navigation Menu

Repository navigation

feat(cli): add flats backup for a consistent online copy - #238

feat(cli): add flats backup for a consistent online copy#238
rabbitson87 merged 19 commits into
mainfrom
feat/flats-backup

rabbitson87 commented Nov 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

rabbitson87 commented Nov 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this adds

Where the change is

Known limits

Task report

A data race that CI found

Verification

For the maintainer

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

rabbitson87 commented Nov 14, 2026 •

edited

Loading