fix: preserve snapshots across repeated and colliding backups

[steipete]

What Problem This Solves

Backups taken within one second could overwrite an earlier snapshot. Archive filenames differing only by case could overwrite each other on the destination filesystem. Unusual crawler IDs could escape the backup root, and failed multi-database runs left partial snapshots.

User Impact

Each backup preserves previous snapshots and every selected archive, uses private directories/files, and removes its own incomplete output on failure. Compatibility: new directory names include a random suffix and encode unusual crawler IDs; callers should use the returned directory and files paths.

Why This Change Was Made

Allocate an exclusive directory per operation, choose filenames against the actual destination filesystem, and keep cleanup scoped to that directory. Archive selection remains unchanged. The regression suite fixes time explicitly and checks completed snapshot contents, path containment, case collisions, permissions, and failure cleanup.

Evidence

• Full warnings-as-errors build and all 54 executable self-tests passed.
• Built CLI backup --app <synthetic-id> --json, real SQLite fixtures, before/after:

Check	Before	After
Repeated backup directories	reused	unique
First snapshot contents	overwritten	preserved
../ crawler ID	escaped backup root	contained
Case-only archive filenames	one archive overwritten	both preserved
Failed multi-database backup	partial directory remained	partial directory removed
Directory/file permissions	0755/0644	0700/0600

• The existing iMessage source-selection fixture now records the source and runs real SQLite, retaining all its assertions and verifying a usable archive snapshot. No tests were removed or weakened.

• Isolated Codex autoreview at P0–P2: scoped-clean.

• Combined backup/CLI candidate: the final packaged helper passes both the CLI contract suite and every backup preservation/containment/permission check; strict signature and icon checks pass.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 12, 2026, 9:12 PM ET / September 13, 2026, 01:12 UTC (Revision 2).

ClawSweeper review

What this changes

Preserves SQLite backup snapshots through unique private directories, filesystem-aware filename selection, crawler-ID encoding, failure cleanup, and regression coverage.

Merge readiness

✅ Ready for maintainer review

The fix remains necessary: current main and v0.4.2 retain the overwrite behavior. The patch is focused, the supplied runtime results cover its intended behavior, and no blocking defect was found.

Priority: P2
Reviewed head: 6c260e1a42d2a3feff5b789266b6c244bcdf77d3

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with relevant runtime observations, meaningful regression tests, and no identified blocking defect.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The captured CLI/real-SQLite before-and-after results exercise the shared backup service and report preserved snapshots, contained traversal IDs, collision survival, cleanup, and private permissions; packaged-helper confirmation covers the same behavior.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The captured CLI/real-SQLite before-and-after results exercise the shared backup service and report preserved snapshots, contained traversal IDs, collision survival, cleanup, and private permissions; packaged-helper confirmation covers the same behavior.
Evidence reviewed	7 items	Current main still needs the fix: The pinned main implementation uses second-resolution directories, raw crawler IDs, a case-sensitive in-memory filename set, and removal of existing destination files before backup. Released behavior: GitHub’s v0.4.2 source also retains timestamp-only directories and destination removal. The release was published on 2026-09-12; it does not contain this fix. Implementation and caller compatibility: The new operation directory contains cleanup and permission changes. The public backup signature and serialized result fields remain unchanged; CLI.swift consumes the returned directory/files, and SettingsModelFileActions.swift consumes the returned file count. No caller constructs timestamp paths.
Findings	None	None.
Security	None	None.

How this fits together

CrawlBar’s CLI and Settings send crawler-reported archive paths to a shared backup service. That service runs SQLite snapshots and returns their directory and file paths to callers.

flowchart TD
 A[Crawler archive status] --> B[CLI or Settings backup]
 B --> C[Select local databases]
 C --> D[Create unique private directory]
 D --> E[Choose filenames and snapshot SQLite]
 E --> F{All snapshots succeeded?}
 F -->|Yes| G[Return backup paths]
 F -->|No| H[Remove incomplete directory]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test growth	production +10 net lines; tests +71 net lines	The small production increase is justified by snapshot isolation and is accompanied by focused regression coverage.

Technical review

Best possible solution:

Keep backup allocation and cleanup in the shared service, preserve completed archives, and retain returned paths as the supported caller contract.

Do we have a high-confidence way to reproduce the issue?

Yes: main’s timestamp-only allocation and destination removal establish how same-second backups overwrite prior snapshots; case-only names also collide on case-insensitive storage. This review inspected source without executing a reproduction.

Is this the best way to solve the issue?

Yes: operation-local allocation and filesystem-aware collision checks repair the existing service without a competing backup path or new user configuration.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against cfe0670b6a10.

Labels

Label justifications:

• P2: This is a bounded repair to explicit backup operations; no widespread active outage is established.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured CLI/real-SQLite before-and-after results exercise the shared backup service and report preserved snapshots, contained traversal IDs, collision survival, cleanup, and private permissions; packaged-helper confirmation covers the same behavior.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured CLI/real-SQLite before-and-after results exercise the shared backup service and report preserved snapshots, contained traversal IDs, collision survival, cleanup, and private permissions; packaged-helper confirmation covers the same behavior.

Evidence

What I checked:

• Current main still needs the fix: The pinned main implementation uses second-resolution directories, raw crawler IDs, a case-sensitive in-memory filename set, and removal of existing destination files before backup. (Sources/CrawlBarCore/DatabaseBackup.swift:85, cfe0670b6a10)
• Released behavior: GitHub’s v0.4.2 source also retains timestamp-only directories and destination removal. The release was published on 2026-09-12; it does not contain this fix. (Sources/CrawlBarCore/DatabaseBackup.swift:85, 9bea3e2e00d6)
• Implementation and caller compatibility: The new operation directory contains cleanup and permission changes. The public backup signature and serialized result fields remain unchanged; CLI.swift consumes the returned directory/files, and SettingsModelFileActions.swift consumes the returned file count. No caller constructs timestamp paths. (Sources/CrawlBarCore/DatabaseBackup.swift:90, 6c260e1a42d2)
• Supplied production-path proof: The complete captured PR body reports built-CLI backup runs using real SQLite fixtures, with before/after observations for repeated snapshots, case collisions, traversal IDs, failed multi-database cleanup, and 0700/0600 permissions. It also reports the packaged helper passing the backup checks. Captured sourceRevision: 0c524c722803486c710eb942fab17dadf3d80ae23757b8e1b6f53c81d461b215. These observations were assessed separately from the supplemental build/self-test claims; no target code was executed during this review. (6c260e1a42d2)
• Regression coverage: The added test fixes the clock, verifies earlier and later database contents, exercises unusual crawler IDs and case-only filenames, checks permissions, and verifies failed operations preserve completed snapshots. The source-selection fixture now delegates to real SQLite and verifies usable output. (Sources/CrawlBarSelfTest/SelfTestBackupIsolation.swift:21, 6c260e1a42d2)
• Related merged work is distinct: fix: keep backups archive-only and coordinate crawler actions #29 is merged and addresses archive-only selection and action coordination. Its body and current source establish that it does not resolve repeated snapshot or destination filename preservation.

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-09-13T01:05:24.338Z sha 6c260e1 :: needs maintainer review before merge. :: none