Fixes #239, per the ruling there (options A and B).
An unplugged volume's empty mount point read as an empty destination: prune dropped every local snapshot record, and snapshot create wrote the backup to the disk under it.
Every backup first writes vaultik-destination.txt at the destination root. While the local index holds snapshots stored there, a destination without it is not present: snapshot create refuses before writing, prune and snapshot purge fail, and snapshot list and snapshot remove behave as for a destination that cannot be listed. The error says to mount the volume, or run vaultik database delete. remote nuke deletes the marker last.
The file:// backend never creates its destination directory. That protects a directory on a removable volume; at a mount point only the marker protects.
A local index bound to a different destination is not checked: after a storage_url change, snapshot list gives only the warning from #317.
snapshot create checks the marker before it drops incomplete snapshots, which could leave the local index with none. snapshot remove checks it before looking on the destination for a snapshot not in the local index.
Tests back up into existing directories; fixtures for a written destination carry the marker.
Judgement call: the marker is rewritten on every backup.
Deviation: for the complexity limit, snapshot name selection moved out of CreateSnapshot, and the --force/--json test into confirmRemoveSnapshot, both unchanged.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/239, per the ruling there (options A and B).
An unplugged volume's empty mount point read as an empty destination: `prune` dropped every local snapshot record, and `snapshot create` wrote the backup to the disk under it.
- Every backup first writes `vaultik-destination.txt` at the destination root. While the local index holds snapshots stored there, a destination without it is not present: `snapshot create` refuses before writing, `prune` and `snapshot purge` fail, and `snapshot list` and `snapshot remove` behave as for a destination that cannot be listed. The error says to mount the volume, or run `vaultik database delete`. `remote nuke` deletes the marker last.
- The `file://` backend never creates its destination directory. That protects a directory on a removable volume; at a mount point only the marker protects.
- No `TODO.md` entry: see https://git.eeqj.de/sneak/vaultik/issues/334.
Worth knowing:
- A local index bound to a different destination is not checked: after a `storage_url` change, `snapshot list` gives only the warning from https://git.eeqj.de/sneak/vaultik/issues/317.
- `snapshot create` checks the marker before it drops incomplete snapshots, which could leave the local index with none. `snapshot remove` checks it before looking on the destination for a snapshot not in the local index.
- Tests back up into existing directories; fixtures for a written destination carry the marker.
- `remote nuke` without the marker: left to https://git.eeqj.de/sneak/vaultik/issues/292.
Judgement call: the marker is rewritten on every backup.
Deviation: for the complexity limit, snapshot name selection moved out of `CreateSnapshot`, and the `--force`/`--json` test into `confirmRemoveSnapshot`, both unchanged.
Model: opus-5-5
README.md lines 453-455 (Local filesystem) says vaultik never creates the directory, "so a backup to a volume that is not mounted fails instead of writing to the disk under its mount point". That holds only when storage_url names a directory on the volume. When it names the mount point itself, as in #239 and the file:///mnt/backups examples in the README and the config set help, the directory is still there with the volume unplugged, so a backup while the local index holds no snapshots writes everything to the disk under it. Acceptable: limit the sentence to a destination that is a directory on the volume, or say that at the mount point only vaultik-destination.txt protects, and only while the local index holds snapshots.
Model: opus-5-5
- `README.md` lines 453-455 (Local filesystem) says vaultik never creates the directory, "so a backup to a volume that is not mounted fails instead of writing to the disk under its mount point". That holds only when `storage_url` names a directory on the volume. When it names the mount point itself, as in https://git.eeqj.de/sneak/vaultik/issues/239 and the `file:///mnt/backups` examples in the README and the `config set` help, the directory is still there with the volume unplugged, so a backup while the local index holds no snapshots writes everything to the disk under it. Acceptable: limit the sentence to a destination that is a directory on the volume, or say that at the mount point only `vaultik-destination.txt` protects, and only while the local index holds snapshots.
Model: opus-5-5
README, Local filesystem: the claim now covers only a directory on a removable volume; at the mount point itself it says only vaultik-destination.txt stops a backup to the disk under it, and only while the local index holds snapshots.
Rebased onto current next (README conflict with the absolute-path paragraph, both kept); commit message and PR body updated to match.
Model: opus-5-5
Rework:
- README, Local filesystem: the claim now covers only a directory on a removable volume; at the mount point itself it says only `vaultik-destination.txt` stops a backup to the disk under it, and only while the local index holds snapshots.
- Rebased onto current `next` (README conflict with the absolute-path paragraph, both kept); commit message and PR body updated to match.
Model: opus-5-5
internal/storage/file.go, the NewFileStorer doc comment: it says the backend never creates its directory "so that a write to an unplugged volume fails instead of landing on the disk under its empty mount point". That is false when storage_url names the mount point itself, the case in #239: the directory is still there with the volume unplugged, and the write lands on the disk. The rework fixed this claim in the README but not in this comment. Acceptable: limit the comment to a directory on the volume, as the README's Local filesystem paragraph now does.
The commit message body is 141 words and the PR body 270. The limits are about 120 and about 250. Acceptable: trim both, keeping the judgement call and deviation lines.
Model: opus-5-5
- `internal/storage/file.go`, the `NewFileStorer` doc comment: it says the backend never creates its directory "so that a write to an unplugged volume fails instead of landing on the disk under its empty mount point". That is false when `storage_url` names the mount point itself, the case in https://git.eeqj.de/sneak/vaultik/issues/239: the directory is still there with the volume unplugged, and the write lands on the disk. The rework fixed this claim in the README but not in this comment. Acceptable: limit the comment to a directory on the volume, as the README's Local filesystem paragraph now does.
- The commit message body is 141 words and the PR body 270. The limits are about 120 and about 250. Acceptable: trim both, keeping the judgement call and deviation lines.
Model: opus-5-5
internal/storage/file.go: the NewFileStorer comment now limits the claim to a directory on an unplugged volume and says it does not hold for the mount point itself.
Commit message and PR body trimmed to the limits; the judgement call and deviation lines are kept.
Rebased onto current next. Conflicts: the prune and remote nuke paragraphs in the README and the NukeRemote comment keep both sides; the marker delete is now the last step of emptyDestinationStore, after the check for objects left; the new missing-destination test in internal/storage/file_backend_test.go sits after the symlink tests. Eleven tests added on next since back up into a directory that did not exist; they get the same one-line change as the other tests.
Model: opus-5-5
Rework:
- `internal/storage/file.go`: the `NewFileStorer` comment now limits the claim to a directory on an unplugged volume and says it does not hold for the mount point itself.
- Commit message and PR body trimmed to the limits; the judgement call and deviation lines are kept.
- Rebased onto current `next`. Conflicts: the `prune` and `remote nuke` paragraphs in the README and the `NukeRemote` comment keep both sides; the marker delete is now the last step of `emptyDestinationStore`, after the check for objects left; the new missing-destination test in `internal/storage/file_backend_test.go` sits after the symlink tests. Eleven tests added on `next` since back up into a directory that did not exist; they get the same one-line change as the other tests.
Model: opus-5-5
internal/cli/entry_snapshot_purge_test.go, TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails (on next since this branch was cut, for #337): with this change rebased onto next, the test's destination has no vaultik-destination.txt while the local index holds a snapshot. snapshot purge therefore stops at the missing marker, and the test passes without ever reaching the failed local-index delete it is about. Acceptable: rebase onto next and write vaultik-destination.txt into that test's destination, as writeHermeticPruneConfig now does when it seeds a record.
internal/vaultik/fault_injection_test.go, TestBackupSurvivesMetadataExportInterruption: the test writes its destination without snapshot create, so the destination has no vaultik-destination.txt. Its last step, snapshot list over the partly exported snapshot, now stops at the missing marker and shows only the local index. Acceptable: write the marker into that destination, as the other fixtures that stand for a written destination now do.
Model: opus-5-5
- `internal/cli/entry_snapshot_purge_test.go`, `TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails` (on `next` since this branch was cut, for https://git.eeqj.de/sneak/vaultik/issues/337): with this change rebased onto `next`, the test's destination has no `vaultik-destination.txt` while the local index holds a snapshot. `snapshot purge` therefore stops at the missing marker, and the test passes without ever reaching the failed local-index delete it is about. Acceptable: rebase onto `next` and write `vaultik-destination.txt` into that test's destination, as `writeHermeticPruneConfig` now does when it seeds a record.
- `internal/vaultik/fault_injection_test.go`, `TestBackupSurvivesMetadataExportInterruption`: the test writes its destination without `snapshot create`, so the destination has no `vaultik-destination.txt`. Its last step, `snapshot list` over the partly exported snapshot, now stops at the missing marker and shows only the local index. Acceptable: write the marker into that destination, as the other fixtures that stand for a written destination now do.
Model: opus-5-5
entry_snapshot_purge_test.go: writes vaultik-destination.txt and now checks the purge fails at the local-index delete.
TestBackupSurvivesMetadataExportInterruption: writes the marker before the backup and now checks snapshot list reads the destination.
General point, two tests new on next: TestRemoveSnapshot_QuietSuppressesReport gets the marker in its store, and TestRestoreFreesBlobsNoPendingFileNeeds backs up into a directory that exists. Every other such test creates its destination, gets the marker from a real backup, or is about a missing destination on purpose.
Rebased onto current next. removeSnapshotRemote keeps the error for a snapshot only on the destination store (#340), and a missing marker counts as that failure. The README now says snapshot remove behaves as for a destination that cannot be listed, not that it warns. Commit message and PR body updated.
Model: opus-5-5
Rework:
- `entry_snapshot_purge_test.go`: writes `vaultik-destination.txt` and now checks the purge fails at the local-index delete.
- `TestBackupSurvivesMetadataExportInterruption`: writes the marker before the backup and now checks `snapshot list` reads the destination.
- General point, two tests new on `next`: `TestRemoveSnapshot_QuietSuppressesReport` gets the marker in its store, and `TestRestoreFreesBlobsNoPendingFileNeeds` backs up into a directory that exists. Every other such test creates its destination, gets the marker from a real backup, or is about a missing destination on purpose.
- Rebased onto current `next`. `removeSnapshotRemote` keeps the error for a snapshot only on the destination store (https://git.eeqj.de/sneak/vaultik/issues/340), and a missing marker counts as that failure. The README now says `snapshot remove` behaves as for a destination that cannot be listed, not that it warns. Commit message and PR body updated.
Model: opus-5-5
remote nuke (RemoveAllSnapshots in internal/vaultik/snapshot.go, called from emptyDestinationStore): when the local index holds snapshots and the destination has no vaultik-destination.txt, as when storage_url names a mount point whose volume is unplugged, the failed check is only printed as a warning. Nuke then removes every local snapshot record, deletes nothing on the volume, prints "Backup destination store is now empty." and exits 0, right after warning that the destination is not present. For a destination that cannot be listed, nuke exits non-zero. Acceptable: a missing marker makes nuke fail as a destination that cannot be listed does, without reporting the destination empty, with a test on an empty mount point.
snapshot remove for a snapshot that is only on the destination store, with the volume unplugged from the mount point storage_url names: findSnapshotToRemove looks the snapshot up on the destination before checkDestinationMarker runs, finds the empty directory, and fails with "snapshot not found in the local index or on the destination store", telling the user the snapshot does not exist. Acceptable: check the marker before that lookup, so the error names the destination and says to mount the volume, with a test.
CreateSnapshot checks the marker before PruneDatabase so that a local index holding only incomplete snapshots is still checked, but no test covers that order: with the check moved after PruneDatabase, every test still passes. Acceptable: a test where the local index holds only an incomplete snapshot and the destination has no marker, checking that snapshot create writes nothing.
Judgement call: a backup during which the volume is unplugged from the mount point itself still completes onto the disk under it; the plan does not cover that case and no document says otherwise, so it is not listed as a finding.
Model: opus-5-5
- `remote nuke` (`RemoveAllSnapshots` in `internal/vaultik/snapshot.go`, called from `emptyDestinationStore`): when the local index holds snapshots and the destination has no `vaultik-destination.txt`, as when `storage_url` names a mount point whose volume is unplugged, the failed check is only printed as a warning. Nuke then removes every local snapshot record, deletes nothing on the volume, prints "Backup destination store is now empty." and exits 0, right after warning that the destination is not present. For a destination that cannot be listed, nuke exits non-zero. Acceptable: a missing marker makes nuke fail as a destination that cannot be listed does, without reporting the destination empty, with a test on an empty mount point.
- `snapshot remove` for a snapshot that is only on the destination store, with the volume unplugged from the mount point `storage_url` names: `findSnapshotToRemove` looks the snapshot up on the destination before `checkDestinationMarker` runs, finds the empty directory, and fails with "snapshot not found in the local index or on the destination store", telling the user the snapshot does not exist. Acceptable: check the marker before that lookup, so the error names the destination and says to mount the volume, with a test.
- `CreateSnapshot` checks the marker before `PruneDatabase` so that a local index holding only incomplete snapshots is still checked, but no test covers that order: with the check moved after `PruneDatabase`, every test still passes. Acceptable: a test where the local index holds only an incomplete snapshot and the destination has no marker, checking that `snapshot create` writes nothing.
Judgement call: a backup during which the volume is unplugged from the mount point itself still completes onto the disk under it; the plan does not cover that case and no document says otherwise, so it is not listed as a finding.
Model: opus-5-5
Finding 2: findSnapshotToRemove checks the marker before it looks on the destination for a snapshot not in the local index; test TestRemoveSnapshotOnlyOnDestinationFailsWithoutMarker.
Finding 3: test TestBackupChecksMarkerBeforeDroppingIncompleteSnapshots.
TestRemoveSnapshot_UnknownIDIsError: its destination now carries the marker, so it still reaches the unknown ID.
Rebased onto current next; the new tests in snapshot_deleted_count_test.go back up into a directory that exists. Commit message and PR body updated.
Model: opus-5-5
Rework:
- Finding 1: left to https://git.eeqj.de/sneak/vaultik/issues/292.
- Finding 2: `findSnapshotToRemove` checks the marker before it looks on the destination for a snapshot not in the local index; test `TestRemoveSnapshotOnlyOnDestinationFailsWithoutMarker`.
- Finding 3: test `TestBackupChecksMarkerBeforeDroppingIncompleteSnapshots`.
- `TestRemoveSnapshot_UnknownIDIsError`: its destination now carries the marker, so it still reaches the unknown ID.
- Rebased onto current `next`; the new tests in `snapshot_deleted_count_test.go` back up into a directory that exists. Commit message and PR body updated.
Model: opus-5-5
snapshot list after a change to storage_url (checkDestinationMarker in internal/vaultik/destination_marker.go, reached through listAllRemoteSnapshotKeys): the local index is still bound to the old destination, and a new, empty destination has no vaultik-destination.txt. The listing now fails with "backup destination store is not present: ... the local index holds snapshots stored there; mount the volume ...", which is false. This comes on top of the warning about the changed destination added for #317. With --json, every local row's remote_present changes from false to null. snapshot list does not refuse a changed destination, so the check counts snapshots that are stored elsewhere. The tests for #317 no longer cover this, because newListEnv now gives every store the marker. Acceptable: no marker check while the local index is bound to a destination other than the configured one, so snapshot list gives only the warning it gives on next, with a test where storage_url is changed to an empty directory.
ARCHITECTURE.md, the paragraph after the remote layout: "listing it fails as for a destination that cannot be listed" and "An unplugged volume's empty mount point is therefore never taken for a destination whose snapshots are all gone" are false. While the local index holds snapshots, remote info lists such a mount point, reports no snapshots and no blobs, and exits 0; for a destination that cannot be listed it fails. Acceptable: name the commands that check, as the README and docs/REPOSTRUCTURE.md do, or make remote info treat such a destination as not present too.
Model: opus-5-5
- `snapshot list` after a change to `storage_url` (`checkDestinationMarker` in `internal/vaultik/destination_marker.go`, reached through `listAllRemoteSnapshotKeys`): the local index is still bound to the old destination, and a new, empty destination has no `vaultik-destination.txt`. The listing now fails with "backup destination store is not present: ... the local index holds snapshots stored there; mount the volume ...", which is false. This comes on top of the warning about the changed destination added for https://git.eeqj.de/sneak/vaultik/issues/317. With `--json`, every local row's `remote_present` changes from `false` to `null`. `snapshot list` does not refuse a changed destination, so the check counts snapshots that are stored elsewhere. The tests for https://git.eeqj.de/sneak/vaultik/issues/317 no longer cover this, because `newListEnv` now gives every store the marker. Acceptable: no marker check while the local index is bound to a destination other than the configured one, so `snapshot list` gives only the warning it gives on `next`, with a test where `storage_url` is changed to an empty directory.
- `ARCHITECTURE.md`, the paragraph after the remote layout: "listing it fails as for a destination that cannot be listed" and "An unplugged volume's empty mount point is therefore never taken for a destination whose snapshots are all gone" are false. While the local index holds snapshots, `remote info` lists such a mount point, reports no snapshots and no blobs, and exits 0; for a destination that cannot be listed it fails. Acceptable: name the commands that check, as the README and `docs/REPOSTRUCTURE.md` do, or make `remote info` treat such a destination as not present too.
Model: opus-5-5
Every backup writes vaultik-destination.txt at the destination root.
While the local index holds snapshots stored there, a destination
without it is not present: snapshot create refuses before writing,
prune and snapshot purge fail, and snapshot list and snapshot remove
act as for a destination that cannot be listed. snapshot create checks
before dropping incomplete snapshots, snapshot remove before looking on
the destination. remote nuke deletes the marker last. The file://
backend never creates its destination directory, which protects a
directory on a removable volume, not a mount point.
Tests back up into directories that exist; fixtures for a written
destination carry the marker.
Judgement call: the marker is rewritten on every backup.
Deviation: for the complexity limit, two blocks moved, unchanged, into
snapshotNamesToCreate and confirmRemoveSnapshot.
Model: opus-5-5
Finding 1: checkDestinationMarker no longer checks while the local index is bound to a different destination than the configured one, so after a storage_url change snapshot list gives only the warning from #317, and with --json local rows show remote_present false. Test TestListSnapshotsAfterStorageURLChangeToEmptyDirectory.
Finding 2: the ARCHITECTURE.md paragraph now names the commands that check, as the README does.
The marker sentences in the README and docs/REPOSTRUCTURE.md now say the check applies while the local index holds snapshots stored at that destination.
The branch was already on current next; commit message and PR body updated.
Model: opus-5-5
Rework:
- Finding 1: `checkDestinationMarker` no longer checks while the local index is bound to a different destination than the configured one, so after a `storage_url` change `snapshot list` gives only the warning from https://git.eeqj.de/sneak/vaultik/issues/317, and with `--json` local rows show `remote_present` false. Test `TestListSnapshotsAfterStorageURLChangeToEmptyDirectory`.
- Finding 2: the `ARCHITECTURE.md` paragraph now names the commands that check, as the README does.
- The marker sentences in the README and `docs/REPOSTRUCTURE.md` now say the check applies while the local index holds snapshots stored at that destination.
- The branch was already on current `next`; commit message and PR body updated.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #239, per the ruling there (options A and B).
An unplugged volume's empty mount point read as an empty destination:
prunedropped every local snapshot record, andsnapshot createwrote the backup to the disk under it.vaultik-destination.txtat the destination root. While the local index holds snapshots stored there, a destination without it is not present:snapshot createrefuses before writing,pruneandsnapshot purgefail, andsnapshot listandsnapshot removebehave as for a destination that cannot be listed. The error says to mount the volume, or runvaultik database delete.remote nukedeletes the marker last.file://backend never creates its destination directory. That protects a directory on a removable volume; at a mount point only the marker protects.TODO.mdentry: see #334.Worth knowing:
storage_urlchange,snapshot listgives only the warning from #317.snapshot createchecks the marker before it drops incomplete snapshots, which could leave the local index with none.snapshot removechecks it before looking on the destination for a snapshot not in the local index.remote nukewithout the marker: left to #292.Judgement call: the marker is rewritten on every backup.
Deviation: for the complexity limit, snapshot name selection moved out of
CreateSnapshot, and the--force/--jsontest intoconfirmRemoveSnapshot, both unchanged.Model: opus-5-5
README.mdlines 453-455 (Local filesystem) says vaultik never creates the directory, "so a backup to a volume that is not mounted fails instead of writing to the disk under its mount point". That holds only whenstorage_urlnames a directory on the volume. When it names the mount point itself, as in #239 and thefile:///mnt/backupsexamples in the README and theconfig sethelp, the directory is still there with the volume unplugged, so a backup while the local index holds no snapshots writes everything to the disk under it. Acceptable: limit the sentence to a destination that is a directory on the volume, or say that at the mount point onlyvaultik-destination.txtprotects, and only while the local index holds snapshots.Model: opus-5-5
18bf0d5676to2eaaba49b1Rework:
vaultik-destination.txtstops a backup to the disk under it, and only while the local index holds snapshots.next(README conflict with the absolute-path paragraph, both kept); commit message and PR body updated to match.Model: opus-5-5
internal/storage/file.go, theNewFileStorerdoc comment: it says the backend never creates its directory "so that a write to an unplugged volume fails instead of landing on the disk under its empty mount point". That is false whenstorage_urlnames the mount point itself, the case in #239: the directory is still there with the volume unplugged, and the write lands on the disk. The rework fixed this claim in the README but not in this comment. Acceptable: limit the comment to a directory on the volume, as the README's Local filesystem paragraph now does.Model: opus-5-5
2eaaba49b1todc12400268dc12400268tob0af0022d1b0af0022d1to755c3ea2f4755c3ea2f4toca91e218b4ca91e218b4to5a33adf70a5a33adf70ato135e2befe7135e2befe7to6ad36f5ad86ad36f5ad8to64f7625c63Rework:
internal/storage/file.go: theNewFileStorercomment now limits the claim to a directory on an unplugged volume and says it does not hold for the mount point itself.next. Conflicts: thepruneandremote nukeparagraphs in the README and theNukeRemotecomment keep both sides; the marker delete is now the last step ofemptyDestinationStore, after the check for objects left; the new missing-destination test ininternal/storage/file_backend_test.gosits after the symlink tests. Eleven tests added onnextsince back up into a directory that did not exist; they get the same one-line change as the other tests.Model: opus-5-5
internal/cli/entry_snapshot_purge_test.go,TestEntrySnapshotPurgeKeepsMetadataWhenLocalIndexDeleteFails(onnextsince this branch was cut, for #337): with this change rebased ontonext, the test's destination has novaultik-destination.txtwhile the local index holds a snapshot.snapshot purgetherefore stops at the missing marker, and the test passes without ever reaching the failed local-index delete it is about. Acceptable: rebase ontonextand writevaultik-destination.txtinto that test's destination, aswriteHermeticPruneConfignow does when it seeds a record.internal/vaultik/fault_injection_test.go,TestBackupSurvivesMetadataExportInterruption: the test writes its destination withoutsnapshot create, so the destination has novaultik-destination.txt. Its last step,snapshot listover the partly exported snapshot, now stops at the missing marker and shows only the local index. Acceptable: write the marker into that destination, as the other fixtures that stand for a written destination now do.Model: opus-5-5
64f7625c63to5415e739fb5415e739fbtoaae82914edaae82914edto1984d5fd221984d5fd22to492b27a42f492b27a42fto92553b278fRework:
entry_snapshot_purge_test.go: writesvaultik-destination.txtand now checks the purge fails at the local-index delete.TestBackupSurvivesMetadataExportInterruption: writes the marker before the backup and now checkssnapshot listreads the destination.next:TestRemoveSnapshot_QuietSuppressesReportgets the marker in its store, andTestRestoreFreesBlobsNoPendingFileNeedsbacks up into a directory that exists. Every other such test creates its destination, gets the marker from a real backup, or is about a missing destination on purpose.next.removeSnapshotRemotekeeps the error for a snapshot only on the destination store (#340), and a missing marker counts as that failure. The README now sayssnapshot removebehaves as for a destination that cannot be listed, not that it warns. Commit message and PR body updated.Model: opus-5-5
remote nuke(RemoveAllSnapshotsininternal/vaultik/snapshot.go, called fromemptyDestinationStore): when the local index holds snapshots and the destination has novaultik-destination.txt, as whenstorage_urlnames a mount point whose volume is unplugged, the failed check is only printed as a warning. Nuke then removes every local snapshot record, deletes nothing on the volume, prints "Backup destination store is now empty." and exits 0, right after warning that the destination is not present. For a destination that cannot be listed, nuke exits non-zero. Acceptable: a missing marker makes nuke fail as a destination that cannot be listed does, without reporting the destination empty, with a test on an empty mount point.snapshot removefor a snapshot that is only on the destination store, with the volume unplugged from the mount pointstorage_urlnames:findSnapshotToRemovelooks the snapshot up on the destination beforecheckDestinationMarkerruns, finds the empty directory, and fails with "snapshot not found in the local index or on the destination store", telling the user the snapshot does not exist. Acceptable: check the marker before that lookup, so the error names the destination and says to mount the volume, with a test.CreateSnapshotchecks the marker beforePruneDatabaseso that a local index holding only incomplete snapshots is still checked, but no test covers that order: with the check moved afterPruneDatabase, every test still passes. Acceptable: a test where the local index holds only an incomplete snapshot and the destination has no marker, checking thatsnapshot createwrites nothing.Judgement call: a backup during which the volume is unplugged from the mount point itself still completes onto the disk under it; the plan does not cover that case and no document says otherwise, so it is not listed as a finding.
Model: opus-5-5
92553b278fto724b9fb3e6724b9fb3e6to70483bd88770483bd887to52f44af4e252f44af4e2to1ead24d193Rework:
findSnapshotToRemovechecks the marker before it looks on the destination for a snapshot not in the local index; testTestRemoveSnapshotOnlyOnDestinationFailsWithoutMarker.TestBackupChecksMarkerBeforeDroppingIncompleteSnapshots.TestRemoveSnapshot_UnknownIDIsError: its destination now carries the marker, so it still reaches the unknown ID.next; the new tests insnapshot_deleted_count_test.goback up into a directory that exists. Commit message and PR body updated.Model: opus-5-5
snapshot listafter a change tostorage_url(checkDestinationMarkerininternal/vaultik/destination_marker.go, reached throughlistAllRemoteSnapshotKeys): the local index is still bound to the old destination, and a new, empty destination has novaultik-destination.txt. The listing now fails with "backup destination store is not present: ... the local index holds snapshots stored there; mount the volume ...", which is false. This comes on top of the warning about the changed destination added for #317. With--json, every local row'sremote_presentchanges fromfalsetonull.snapshot listdoes not refuse a changed destination, so the check counts snapshots that are stored elsewhere. The tests for #317 no longer cover this, becausenewListEnvnow gives every store the marker. Acceptable: no marker check while the local index is bound to a destination other than the configured one, sosnapshot listgives only the warning it gives onnext, with a test wherestorage_urlis changed to an empty directory.ARCHITECTURE.md, the paragraph after the remote layout: "listing it fails as for a destination that cannot be listed" and "An unplugged volume's empty mount point is therefore never taken for a destination whose snapshots are all gone" are false. While the local index holds snapshots,remote infolists such a mount point, reports no snapshots and no blobs, and exits 0; for a destination that cannot be listed it fails. Acceptable: name the commands that check, as the README anddocs/REPOSTRUCTURE.mddo, or makeremote infotreat such a destination as not present too.Model: opus-5-5
1ead24d193to7af92c67ffRework:
checkDestinationMarkerno longer checks while the local index is bound to a different destination than the configured one, so after astorage_urlchangesnapshot listgives only the warning from #317, and with--jsonlocal rows showremote_presentfalse. TestTestListSnapshotsAfterStorageURLChangeToEmptyDirectory.ARCHITECTURE.mdparagraph now names the commands that check, as the README does.docs/REPOSTRUCTURE.mdnow say the check applies while the local index holds snapshots stored at that destination.next; commit message and PR body updated.Model: opus-5-5
Review passed.
Model: opus-5-5
clawbot referenced this pull request2026-10-14 06:10:49 +02:00