ListSnapshots() silently deleted local snapshot records not found in remote storage. A list/read operation should not have destructive side effects.
Changes
Removed destructive sync from ListSnapshots() — the inline loop that deleted local snapshots not present in remote storage has been removed entirely. ListSnapshots() now only reads and displays data.
Improved syncWithRemote() cascade cleanup — updated syncWithRemote() to use deleteSnapshotFromLocalDB() instead of directly calling Repositories.Snapshots.Delete(). This ensures proper cascade deletion of related records (snapshot_files, snapshot_blobs, snapshot_uploads) before deleting the snapshot record itself, matching the thorough cleanup that the removed ListSnapshots code was doing.
The explicit sync behavior remains available via syncWithRemote(), which is called by PurgeSnapshots().
Testing
docker build . passes (lint, fmt-check, all tests, compilation)
## Summary
`ListSnapshots()` silently deleted local snapshot records not found in remote storage. A list/read operation should not have destructive side effects.
## Changes
1. **Removed destructive sync from `ListSnapshots()`** — the inline loop that deleted local snapshots not present in remote storage has been removed entirely. `ListSnapshots()` now only reads and displays data.
2. **Improved `syncWithRemote()` cascade cleanup** — updated `syncWithRemote()` to use `deleteSnapshotFromLocalDB()` instead of directly calling `Repositories.Snapshots.Delete()`. This ensures proper cascade deletion of related records (`snapshot_files`, `snapshot_blobs`, `snapshot_uploads`) before deleting the snapshot record itself, matching the thorough cleanup that the removed `ListSnapshots` code was doing.
The explicit sync behavior remains available via `syncWithRemote()`, which is called by `PurgeSnapshots()`.
## Testing
- `docker build .` passes (lint, fmt-check, all tests, compilation)
closes https://git.eeqj.de/sneak/vaultik/issues/15
ListSnapshots() silently deleted local snapshot records not found in
remote storage. A list/read operation should not have destructive side
effects.
Changes:
- Remove the inline deletion loop from ListSnapshots() that deleted
local snapshots not present in remote storage
- Update syncWithRemote() to use deleteSnapshotFromLocalDB() for
proper cascade cleanup (deleting snapshot_files, snapshot_blobs,
and snapshot_uploads before the snapshot record itself)
The sync behavior remains available via syncWithRemote(), which is
called explicitly by PurgeSnapshots().
closes #15
Destructive sync loop fully removed from ListSnapshots() — the 27-line block that silently deleted local snapshot records not found in remote storage is completely gone. Grep confirms zero delete/remove operations remain in the function. ListSnapshots() is now purely read-only.
syncWithRemote() now uses deleteSnapshotFromLocalDB() — the old code called Repositories.Snapshots.Delete() directly, which only deleted the snapshot record and left orphaned snapshot_files, snapshot_blobs, and snapshot_uploads records. The new call to deleteSnapshotFromLocalDB() does proper cascade deletion (files → blobs → uploads → snapshot record), fixing a pre-existing bug.
deleteSnapshotFromLocalDB() is a pre-existing function with correct cascade logic and proper fmt.Errorf wrapping.
docker build . passes — lint, fmt-check, tests, and compilation all green.
## Review: PASS ✅
**Verified:**
1. **Destructive sync loop fully removed from `ListSnapshots()`** — the 27-line block that silently deleted local snapshot records not found in remote storage is completely gone. Grep confirms zero delete/remove operations remain in the function. `ListSnapshots()` is now purely read-only.
2. **`syncWithRemote()` now uses `deleteSnapshotFromLocalDB()`** — the old code called `Repositories.Snapshots.Delete()` directly, which only deleted the snapshot record and left orphaned `snapshot_files`, `snapshot_blobs`, and `snapshot_uploads` records. The new call to [`deleteSnapshotFromLocalDB()`](https://git.eeqj.de/sneak/vaultik/src/branch/fix/list-snapshots-no-destructive-side-effect/internal/vaultik/snapshot.go#L986) does proper cascade deletion (files → blobs → uploads → snapshot record), fixing a pre-existing bug.
3. **`deleteSnapshotFromLocalDB()`** is a pre-existing function with correct cascade logic and proper `fmt.Errorf` wrapping.
4. **`docker build .` passes** — lint, fmt-check, tests, and compilation all green.
5. **Minimal, focused diff** — 1 file, +1/-28 lines. Exactly addresses [issue #15](https://git.eeqj.de/sneak/vaultik/issues/15).
Clean fix. No issues found.
Conflict was in reconcileLocalWithRemote — the refactor in #41 extracted this function, which still contained the destructive sync loop. Resolved by keeping the refactored structure but removing the deletion loop, consistent with the intent of issue #15. Also updated the function's doc comment to match its new behavior.
Rebased onto `main` (on top of [PR #39](https://git.eeqj.de/sneak/vaultik/pulls/39) and [PR #41](https://git.eeqj.de/sneak/vaultik/pulls/41)).
Conflict was in `reconcileLocalWithRemote` — the refactor in #41 extracted this function, which still contained the destructive sync loop. Resolved by keeping the refactored structure but removing the deletion loop, consistent with the intent of [issue #15](https://git.eeqj.de/sneak/vaultik/issues/15). Also updated the function's doc comment to match its new behavior.
`docker build .` passes: lint (0 issues), all tests pass, binary builds.
Verified after rebase onto main (incorporating PR #39 and PR #41):
Rebase is clean — single commit a36b314 on top of main, no conflict markers, no merge commits.
reconcileLocalWithRemote() is now read-only — the 13-line deletion loop that removed local snapshots not found in remote has been completely removed. The function now only builds and returns a map[string]*database.Snapshot for cross-referencing. Doc comment updated to match.
ListSnapshots() call chain is entirely non-destructive — listRemoteSnapshotIDs() → reconcileLocalWithRemote() → buildSnapshotInfoList() → sort → printSnapshotTable(). Zero write or delete operations anywhere in this path.
syncWithRemote() now uses deleteSnapshotFromLocalDB() — the old direct Repositories.Snapshots.Delete() call has been replaced with the cascade helper, which properly deletes snapshot_files → snapshot_blobs → snapshot_uploads → snapshot record. This fixes a pre-existing orphaned-records bug.
docker build . passes — lint (0 issues), fmt-check, all tests, compilation all green.
## Review: PASS ✅ (post-rebase)
**Verified after rebase onto main (incorporating [PR #39](https://git.eeqj.de/sneak/vaultik/pulls/39) and [PR #41](https://git.eeqj.de/sneak/vaultik/pulls/41)):**
1. **Rebase is clean** — single commit `a36b314` on top of `main`, no conflict markers, no merge commits.
2. **`reconcileLocalWithRemote()` is now read-only** — the 13-line deletion loop that removed local snapshots not found in remote has been completely removed. The function now only builds and returns a `map[string]*database.Snapshot` for cross-referencing. Doc comment updated to match.
3. **`ListSnapshots()` call chain is entirely non-destructive** — `listRemoteSnapshotIDs()` → `reconcileLocalWithRemote()` → `buildSnapshotInfoList()` → sort → `printSnapshotTable()`. Zero write or delete operations anywhere in this path.
4. **`syncWithRemote()` now uses `deleteSnapshotFromLocalDB()`** — the old direct `Repositories.Snapshots.Delete()` call has been replaced with the cascade helper, which properly deletes `snapshot_files` → `snapshot_blobs` → `snapshot_uploads` → snapshot record. This fixes a pre-existing orphaned-records bug.
5. **`docker build .` passes** — lint (0 issues), fmt-check, all tests, compilation all green.
Minimal diff: 1 file, +2/−15 lines. Correctly addresses [issue #15](https://git.eeqj.de/sneak/vaultik/issues/15).
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.
clawbot
Summary
ListSnapshots()silently deleted local snapshot records not found in remote storage. A list/read operation should not have destructive side effects.Changes
Removed destructive sync from
ListSnapshots()— the inline loop that deleted local snapshots not present in remote storage has been removed entirely.ListSnapshots()now only reads and displays data.Improved
syncWithRemote()cascade cleanup — updatedsyncWithRemote()to usedeleteSnapshotFromLocalDB()instead of directly callingRepositories.Snapshots.Delete(). This ensures proper cascade deletion of related records (snapshot_files,snapshot_blobs,snapshot_uploads) before deleting the snapshot record itself, matching the thorough cleanup that the removedListSnapshotscode was doing.The explicit sync behavior remains available via
syncWithRemote(), which is called byPurgeSnapshots().Testing
docker build .passes (lint, fmt-check, all tests, compilation)closes #15
Review: PASS ✅
Verified:
Destructive sync loop fully removed from
ListSnapshots()— the 27-line block that silently deleted local snapshot records not found in remote storage is completely gone. Grep confirms zero delete/remove operations remain in the function.ListSnapshots()is now purely read-only.syncWithRemote()now usesdeleteSnapshotFromLocalDB()— the old code calledRepositories.Snapshots.Delete()directly, which only deleted the snapshot record and left orphanedsnapshot_files,snapshot_blobs, andsnapshot_uploadsrecords. The new call todeleteSnapshotFromLocalDB()does proper cascade deletion (files → blobs → uploads → snapshot record), fixing a pre-existing bug.deleteSnapshotFromLocalDB()is a pre-existing function with correct cascade logic and properfmt.Errorfwrapping.docker build .passes — lint, fmt-check, tests, and compilation all green.Minimal, focused diff — 1 file, +1/-28 lines. Exactly addresses issue #15.
Clean fix. No issues found.
@clawbot conflict
4bcca47ce6toa36b314c79Rebased onto
main(on top of PR #39 and PR #41).Conflict was in
reconcileLocalWithRemote— the refactor in #41 extracted this function, which still contained the destructive sync loop. Resolved by keeping the refactored structure but removing the deletion loop, consistent with the intent of issue #15. Also updated the function's doc comment to match its new behavior.docker build .passes: lint (0 issues), all tests pass, binary builds.Review: PASS ✅ (post-rebase)
Verified after rebase onto main (incorporating PR #39 and PR #41):
Rebase is clean — single commit
a36b314on top ofmain, no conflict markers, no merge commits.reconcileLocalWithRemote()is now read-only — the 13-line deletion loop that removed local snapshots not found in remote has been completely removed. The function now only builds and returns amap[string]*database.Snapshotfor cross-referencing. Doc comment updated to match.ListSnapshots()call chain is entirely non-destructive —listRemoteSnapshotIDs()→reconcileLocalWithRemote()→buildSnapshotInfoList()→ sort →printSnapshotTable(). Zero write or delete operations anywhere in this path.syncWithRemote()now usesdeleteSnapshotFromLocalDB()— the old directRepositories.Snapshots.Delete()call has been replaced with the cascade helper, which properly deletessnapshot_files→snapshot_blobs→snapshot_uploads→ snapshot record. This fixes a pre-existing orphaned-records bug.docker build .passes — lint (0 issues), fmt-check, all tests, compilation all green.Minimal diff: 1 file, +2/−15 lines. Correctly addresses issue #15.