The --prune flag on snapshot create was accepted but silently did nothing (TODO stub). This connects it to actually:
Purge old snapshots (keeping only the latest) via PurgeSnapshots
Remove unreferenced blobs from storage via PruneBlobs
Changes
Removed the TODO stub from createNamedSnapshot
Added prune logic to CreateSnapshot (runs once after all snapshots complete, not per-snapshot)
Both operations use --force mode (no interactive confirmation) since --prune is an explicit opt-in flag
Added unit test for the Prune flag on SnapshotCreateOptions
Testing
go build ./... passes
New test TestSnapshotCreateOptions_PruneFlag passes
Pre-existing TestBackupAndRestore failure is unrelated (from blob cache refactor on fix/issue-26)
Notes
Branch is based on fix/issue-26 because main has pre-existing build errors (missing printfStdout, FetchAndDecryptBlob methods that were added in fix/issue-26)
Once fix/issue-26 is merged to main, this PR can be rebased cleanly
## Summary
The `--prune` flag on `snapshot create` was accepted but silently did nothing (TODO stub). This connects it to actually:
1. **Purge old snapshots** (keeping only the latest) via `PurgeSnapshots`
2. **Remove unreferenced blobs** from storage via `PruneBlobs`
## Changes
- Removed the TODO stub from `createNamedSnapshot`
- Added prune logic to `CreateSnapshot` (runs once after all snapshots complete, not per-snapshot)
- Both operations use `--force` mode (no interactive confirmation) since `--prune` is an explicit opt-in flag
- Added unit test for the `Prune` flag on `SnapshotCreateOptions`
## Testing
- `go build ./...` passes
- New test `TestSnapshotCreateOptions_PruneFlag` passes
- Pre-existing `TestBackupAndRestore` failure is unrelated (from blob cache refactor on fix/issue-26)
## Notes
- Branch is based on `fix/issue-26` because `main` has pre-existing build errors (missing `printfStdout`, `FetchAndDecryptBlob` methods that were added in fix/issue-26)
- Once fix/issue-26 is merged to main, this PR can be rebased cleanly
Closes #4
sneak
was assigned by clawbot2026-02-16 06:35:28 +01:00
=== RUN TestSnapshotCreateOptions_PruneFlag
--- PASS: TestSnapshotCreateOptions_PruneFlag (0.00s)
PASS
ok git.eeqj.de/sneak/vaultik/internal/vaultik (cached)
✅ New test passes. Build succeeds.
Note: has a pre-existing failure unrelated to this change (blob cache refactor from fix/issue-26).
## Test Results
```
=== RUN TestSnapshotCreateOptions_PruneFlag
--- PASS: TestSnapshotCreateOptions_PruneFlag (0.00s)
PASS
ok git.eeqj.de/sneak/vaultik/internal/vaultik (cached)
```
✅ New test passes. Build succeeds.
Note: has a pre-existing failure unrelated to this change (blob cache refactor from fix/issue-26).
Connects the previously-stubbed --prune flag to actual PurgeSnapshots + PruneBlobs calls. The implementation is clean:
Prune runs once after all snapshots complete (not per-snapshot) — correct
Uses Force: true since --prune is explicit opt-in — good
Error handling propagates correctly
Dependency note: Based on fix/issue-26 branch (#31). Must be rebased after #31 merges. The PR description is transparent about this — good.
Test coverage: The test (TestSnapshotCreateOptions_PruneFlag) only verifies the struct field exists, not the actual prune behavior. An integration test that creates snapshots with Prune: true and verifies old snapshots are deleted would be more valuable. Understandable if that's hard to set up without full infrastructure though.
Minor: Same printfStdout/blob cache changes from #31 are included here. After #31 merges and this rebases, the diff should be much smaller.
## Code Review
**Verdict: LGTM with notes ✅**
Connects the previously-stubbed `--prune` flag to actual `PurgeSnapshots` + `PruneBlobs` calls. The implementation is clean:
- Prune runs once after all snapshots complete (not per-snapshot) — correct
- Uses `Force: true` since `--prune` is explicit opt-in — good
- Error handling propagates correctly
**Dependency note:** Based on `fix/issue-26` branch (#31). Must be rebased after #31 merges. The PR description is transparent about this — good.
**Test coverage:** The test (`TestSnapshotCreateOptions_PruneFlag`) only verifies the struct field exists, not the actual prune behavior. An integration test that creates snapshots with `Prune: true` and verifies old snapshots are deleted would be more valuable. Understandable if that's hard to set up without full infrastructure though.
**Minor:** Same `printfStdout`/blob cache changes from #31 are included here. After #31 merges and this rebases, the diff should be much smaller.
Branch is already up to date with main — no rebase needed.
**`make test` (lint + fmt-check + tests) passes with zero failures.** All packages pass:
- blob, blobgen, chunker, cli, config, crypto, database, globals, models, pidlock, s3, snapshot, vaultik
Ready for review.
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
The
--pruneflag onsnapshot createwas accepted but silently did nothing (TODO stub). This connects it to actually:PurgeSnapshotsPruneBlobsChanges
createNamedSnapshotCreateSnapshot(runs once after all snapshots complete, not per-snapshot)--forcemode (no interactive confirmation) since--pruneis an explicit opt-in flagPruneflag onSnapshotCreateOptionsTesting
go build ./...passesTestSnapshotCreateOptions_PruneFlagpassesTestBackupAndRestorefailure is unrelated (from blob cache refactor on fix/issue-26)Notes
fix/issue-26becausemainhas pre-existing build errors (missingprintfStdout,FetchAndDecryptBlobmethods that were added in fix/issue-26)Closes #4
Test Results
✅ New test passes. Build succeeds.
Note: has a pre-existing failure unrelated to this change (blob cache refactor from fix/issue-26).
Code Review
Verdict: LGTM with notes ✅
Connects the previously-stubbed
--pruneflag to actualPurgeSnapshots+PruneBlobscalls. The implementation is clean:Force: truesince--pruneis explicit opt-in — goodDependency note: Based on
fix/issue-26branch (#31). Must be rebased after #31 merges. The PR description is transparent about this — good.Test coverage: The test (
TestSnapshotCreateOptions_PruneFlag) only verifies the struct field exists, not the actual prune behavior. An integration test that creates snapshots withPrune: trueand verifies old snapshots are deleted would be more valuable. Understandable if that's hard to set up without full infrastructure though.Minor: Same
printfStdout/blob cache changes from #31 are included here. After #31 merges and this rebases, the diff should be much smaller.094da721dftoe8b4ef80abRebased onto main, resolved conflicts, and fixed all issues:
v.scanlnStdin→v.scanStdin(undefined method after rebase)defer f.Close()→defer func() { _ = f.Close() }())FetchAndDecryptBlobandFetchBlob(were stubs returning errors)make test(lint + fmt-check + tests) passes cleanly:Ready for review.
e8b4ef80abto2bf085bed6Rebased onto main and resolved conflicts.
make test(lint + fmt-check + tests) passes cleanly:FetchAndDecryptBlob/FetchBlob/scanlnStdinmethods that conflicted withblob_fetch_stub.goForce-pushed.
Branch is already up to date with main — no rebase needed.
make test(lint + fmt-check + tests) passes with zero failures. All packages pass:Ready for review.
2bf085bed6to76e047bbb2Rebased onto latest main (after PR #31 merge). Skipped already-applied commits (helper wrappers were already on main).
make testoutput: ✅ All passinggolangci-lint run: 0 issuesinternal/blob✅internal/blobgen✅internal/chunker✅internal/cli✅internal/config✅internal/crypto✅internal/vaultik✅Ready for review.
these methods are WAAAAY too long. break them up into descriptively named methods that are no more than 100-150 lines each.