Mark a snapshot complete only after its metadata export succeeds #200

Merged
clawbot merged 1 commits from issue-177-complete-after-export into next 2026-09-22 20:00:41 +02:00
Collaborator

Summary

finalizeSnapshotMetadata marked a snapshot complete and then exported its metadata. A crash after completion but before/during the export left the local index showing the snapshot complete while the destination held no manifest or database. PruneDatabase drops only rows with a NULL completion time, so it survived — a snapshot silently treated as good but unrestorable.

Fix

Reorder finalization so completion is recorded last. CompleteSnapshot is split into two halves:

  • PopulateSnapshotBlobs — runs before the export (the manifest and trimmed database are built from snapshot_blobs).
  • MarkSnapshotComplete — runs only after the export succeeds.

An interrupted export now leaves the snapshot incomplete, so the next run's PruneDatabase drops it and re-backs-up the data. The reverse, tiny window (crash after export, before the completion write) leaves a fully restorable snapshot at the destination that the local index reports honestly as remote-only — the safe direction. CompleteSnapshot is kept as a convenience (populate, then mark) for callers that do not interleave an export.

Docs

Updated REPOSTRUCTURE.md consistency guarantee 4 and the ARCHITECTURE.md backup flow (both documented the old order under #171) to describe the new ordering.

Test

Added TestBackupCompletesOnlyAfterMetadataExport: it drives the full CreateSnapshot path through the faultstore seam (#72), fails the manifest upload once, and asserts the interrupted snapshot is left incomplete (not complete), then that a retry prunes it, leaves the destination and local index in agreement, and restores. It fails against the old complete-before-export order.

Verification

make check green; full make test green.

Model: opus-4-8

## Summary `finalizeSnapshotMetadata` marked a snapshot complete and *then* exported its metadata. A crash after completion but before/during the export left the local index showing the snapshot complete while the destination held no manifest or database. `PruneDatabase` drops only rows with a NULL completion time, so it survived — a snapshot silently treated as good but unrestorable. ## Fix Reorder finalization so completion is recorded last. `CompleteSnapshot` is split into two halves: - `PopulateSnapshotBlobs` — runs before the export (the manifest and trimmed database are built from `snapshot_blobs`). - `MarkSnapshotComplete` — runs only after the export succeeds. An interrupted export now leaves the snapshot incomplete, so the next run's `PruneDatabase` drops it and re-backs-up the data. The reverse, tiny window (crash after export, before the completion write) leaves a fully restorable snapshot at the destination that the local index reports honestly as remote-only — the safe direction. `CompleteSnapshot` is kept as a convenience (populate, then mark) for callers that do not interleave an export. ## Docs Updated REPOSTRUCTURE.md consistency guarantee 4 and the ARCHITECTURE.md backup flow (both documented the old order under https://git.eeqj.de/sneak/vaultik/issues/171) to describe the new ordering. ## Test Added `TestBackupCompletesOnlyAfterMetadataExport`: it drives the full `CreateSnapshot` path through the faultstore seam (https://git.eeqj.de/sneak/vaultik/issues/72), fails the manifest upload once, and asserts the interrupted snapshot is left incomplete (not complete), then that a retry prunes it, leaves the destination and local index in agreement, and restores. It fails against the old complete-before-export order. ## Verification `make check` green; full `make test` green. Model: opus-4-8
clawbot added 1 commit 2026-09-22 19:44:29 +02:00
finalizeSnapshotMetadata marked the snapshot complete and then exported its
metadata. A crash after completion but before/during the export left the
local index showing the snapshot as complete while the destination had no
manifest or database, and PruneDatabase (which drops only NULL completed_at
rows) kept it: a silently unrestorable snapshot.

Reorder so completion is recorded last. CompleteSnapshot is split into
PopulateSnapshotBlobs (before the export, which reads snapshot_blobs) and
MarkSnapshotComplete (after it). An interrupted export now leaves the
snapshot incomplete, so the next run's PruneDatabase drops it and re-backs-up
the data. The reverse tiny window leaves a fully restorable snapshot at the
destination that the index reports honestly as remote-only.

Update REPOSTRUCTURE.md guarantee 4 and the ARCHITECTURE.md flow to the new
order. Add a fault-injection test driving the full create path.

Model: opus-4-8
clawbot added the needs-review label 2026-09-22 19:44:36 +02:00
clawbot self-assigned this 2026-09-22 19:44:37 +02:00
Author
Collaborator

PASS

Model: opus-4-8

PASS Model: opus-4-8
clawbot merged commit dd7a610c23 into next 2026-09-22 20:00:41 +02:00
clawbot deleted branch issue-177-complete-after-export 2026-09-22 20:00:41 +02:00
Sign in to join this conversation.