snapshot remove could not remove a remote-only snapshot by the remote key that snapshot list prints for it. It passed the argument to RemoteSnapshotKey, so a remote key was hashed again and never matched; the
only working identifier was the plain snapshot ID, which a host with no local
index does not have.
findSnapshotToRemove now resolves the identifier with resolveSnapshotRemoteKey,
the same function verify and restore use, and returns the resolved key. That key
is used both to confirm the snapshot exists on the destination store and to
delete its metadata. A human snapshot ID still hashes without consulting the
store; a remote key or an unambiguous leading part of one is resolved against
the store's metadata listing.
A note for the reviewer: for a remote key that matches nothing on the store, the
error is now errSnapshotKeyNotFound ("no snapshot on the destination store
matches this identifier") rather than the previous "snapshot not found in the
local index or on the destination store". The human-ID path is unchanged.
Run usage: DeepSeek V4 Pro through OpenRouter, 287,246 input, 11,110 output, 10,296 reasoning and 5,107,456 cache-read tokens; cost $0.70.
Model: deepseek-v4-pro
Closes #400.
snapshot remove could not remove a remote-only snapshot by the remote key that
`snapshot list` prints for it. It passed the argument to
`RemoteSnapshotKey`, so a remote key was hashed again and never matched; the
only working identifier was the plain snapshot ID, which a host with no local
index does not have.
`findSnapshotToRemove` now resolves the identifier with `resolveSnapshotRemoteKey`,
the same function verify and restore use, and returns the resolved key. That key
is used both to confirm the snapshot exists on the destination store and to
delete its metadata. A human snapshot ID still hashes without consulting the
store; a remote key or an unambiguous leading part of one is resolved against
the store's metadata listing.
A note for the reviewer: for a remote key that matches nothing on the store, the
error is now `errSnapshotKeyNotFound` ("no snapshot on the destination store
matches this identifier") rather than the previous "snapshot not found in the
local index or on the destination store". The human-ID path is unchanged.
Run usage: DeepSeek V4 Pro through OpenRouter, 287,246 input, 11,110 output, 10,296 reasoning and 5,107,456 cache-read tokens; cost $0.70.
Model: deepseek-v4-pro
A host with no local index could remove a remote-only snapshot only by
its plain snapshot ID, which it does not have. snapshot remove passed
its argument through RemoteSnapshotKey, rehashing a remote key so it
never matched. It now resolves the identifier with the same
resolveSnapshotRemoteKey that verify and restore use, so a remote key
or an unambiguous leading part of it works.
Model: deepseek-v4-pro
For the reviewer (an Opus review, by sneak's order #400 (comment)): the bar is the definition of done in #400. After a passing review this PR goes to sneak for his acceptance review; it is not merged.
Model: opus-5-5
**For the reviewer** (an Opus review, by sneak's order https://git.eeqj.de/sneak/vaultik/issues/400#issuecomment-170531): the bar is the definition of done in https://git.eeqj.de/sneak/vaultik/issues/400. After a passing review this PR goes to sneak for his acceptance review; it is not merged.
Model: opus-5-5
internal/vaultik/snapshot.go, findSnapshotToRemove: a remote key is looked up in the local index only as a literal snapshot ID, so it never matches there. snapshot list --json prints a remote_key for every snapshot, including ones the local index holds. Given such a key, snapshot remove deletes the snapshot's metadata from the destination store, keeps its row in the local index, and reports success. Its confirmation prompt had said it would remove the snapshot from the local database. Before this change, that key was refused. Acceptable: when the resolved key equals snapshot.RemoteSnapshotKey of a snapshot in the local index, remove that row too and report it as removed from the local database (RemoveAllSnapshots already matches keys to local IDs this way). Refusing with an error that names the snapshot ID is also acceptable. Add a test for this case. Also correct the function's doc comment: it says a remote key always names a remote-only snapshot, and that errSnapshotNotFound is the only not-found error.
PR title: it lacks (closes #400). The PR lands as a squash, and the squash commit's subject is the PR title. Add it to the title.
internal/vaultik/snapshot.go, doc comment of removeSnapshotRemote: the rewrap breaks a line after "checkDestinationMarker; when", in the middle of a sentence. Rewrap the paragraph.
Model: opus-5-5 (review); deepseek-v4-pro (change under review)
**FAIL** (`needs-rework`)
1. `internal/vaultik/snapshot.go`, `findSnapshotToRemove`: a remote key is looked up in the local index only as a literal snapshot ID, so it never matches there. `snapshot list --json` prints a `remote_key` for every snapshot, including ones the local index holds. Given such a key, `snapshot remove` deletes the snapshot's metadata from the destination store, keeps its row in the local index, and reports success. Its confirmation prompt had said it would remove the snapshot from the local database. Before this change, that key was refused. Acceptable: when the resolved key equals `snapshot.RemoteSnapshotKey` of a snapshot in the local index, remove that row too and report it as removed from the local database (`RemoveAllSnapshots` already matches keys to local IDs this way). Refusing with an error that names the snapshot ID is also acceptable. Add a test for this case. Also correct the function's doc comment: it says a remote key always names a remote-only snapshot, and that `errSnapshotNotFound` is the only not-found error.
2. PR title: it lacks ` (closes #400)`. The PR lands as a squash, and the squash commit's subject is the PR title. Add it to the title.
3. `internal/vaultik/snapshot.go`, doc comment of `removeSnapshotRemote`: the rewrap breaks a line after "checkDestinationMarker; when", in the middle of a sentence. Rewrap the paragraph.
Model: opus-5-5 (review); deepseek-v4-pro (change under review)
clawbot
changed title from snapshot remove accepts a remote-only snapshot's remote key to snapshot remove accepts a remote-only snapshot's remote key (closes #400)2026-11-08 15:21:25 +01:00
For the reworker (a DeepSeek V4 Pro run, within sneak's order #400 (comment)): fix the review's findings (#404 (comment)), and nothing more.
When the resolved remote key equals snapshot.RemoteSnapshotKey of a snapshot in the local index, remove that local row too, and report it as removed from the local database. Match keys to local IDs the way RemoveAllSnapshots already does. Add a test for this case. Correct findSnapshotToRemove's doc comment: a remote key does not always name a remote-only snapshot, and errSnapshotNotFound is not the only not-found error.
The title already ends in (closes #400), so there is nothing to do for this point.
Rewrap the doc comment of removeSnapshotRemote so that no line breaks in the middle of a sentence.
Keep one commit, run make check, and push with --force-with-lease. Don't merge. The next review checks these points and anything the change breaks.
Model: opus-5-5
**For the reworker** (a DeepSeek V4 Pro run, within sneak's order https://git.eeqj.de/sneak/vaultik/issues/400#issuecomment-170531): fix the review's findings (https://git.eeqj.de/sneak/vaultik/pulls/404#issuecomment-170573), and nothing more.
1. When the resolved remote key equals `snapshot.RemoteSnapshotKey` of a snapshot in the local index, remove that local row too, and report it as removed from the local database. Match keys to local IDs the way `RemoveAllSnapshots` already does. Add a test for this case. Correct `findSnapshotToRemove`'s doc comment: a remote key does not always name a remote-only snapshot, and `errSnapshotNotFound` is not the only not-found error.
2. The title already ends in `(closes #400)`, so there is nothing to do for this point.
3. Rewrap the doc comment of `removeSnapshotRemote` so that no line breaks in the middle of a sentence.
Keep one commit, run `make check`, and push with `--force-with-lease`. Don't merge. The next review checks these points and anything the change breaks.
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.
Closes #400.
snapshot remove could not remove a remote-only snapshot by the remote key that
snapshot listprints for it. It passed the argument toRemoteSnapshotKey, so a remote key was hashed again and never matched; theonly working identifier was the plain snapshot ID, which a host with no local
index does not have.
findSnapshotToRemovenow resolves the identifier withresolveSnapshotRemoteKey,the same function verify and restore use, and returns the resolved key. That key
is used both to confirm the snapshot exists on the destination store and to
delete its metadata. A human snapshot ID still hashes without consulting the
store; a remote key or an unambiguous leading part of one is resolved against
the store's metadata listing.
A note for the reviewer: for a remote key that matches nothing on the store, the
error is now
errSnapshotKeyNotFound("no snapshot on the destination storematches this identifier") rather than the previous "snapshot not found in the
local index or on the destination store". The human-ID path is unchanged.
Run usage: DeepSeek V4 Pro through OpenRouter, 287,246 input, 11,110 output, 10,296 reasoning and 5,107,456 cache-read tokens; cost $0.70.
Model: deepseek-v4-pro
For the reviewer (an Opus review, by sneak's order #400 (comment)): the bar is the definition of done in #400. After a passing review this PR goes to sneak for his acceptance review; it is not merged.
Model: opus-5-5
FAIL (
needs-rework)internal/vaultik/snapshot.go,findSnapshotToRemove: a remote key is looked up in the local index only as a literal snapshot ID, so it never matches there.snapshot list --jsonprints aremote_keyfor every snapshot, including ones the local index holds. Given such a key,snapshot removedeletes the snapshot's metadata from the destination store, keeps its row in the local index, and reports success. Its confirmation prompt had said it would remove the snapshot from the local database. Before this change, that key was refused. Acceptable: when the resolved key equalssnapshot.RemoteSnapshotKeyof a snapshot in the local index, remove that row too and report it as removed from the local database (RemoveAllSnapshotsalready matches keys to local IDs this way). Refusing with an error that names the snapshot ID is also acceptable. Add a test for this case. Also correct the function's doc comment: it says a remote key always names a remote-only snapshot, and thaterrSnapshotNotFoundis the only not-found error.(closes #400). The PR lands as a squash, and the squash commit's subject is the PR title. Add it to the title.internal/vaultik/snapshot.go, doc comment ofremoveSnapshotRemote: the rewrap breaks a line after "checkDestinationMarker; when", in the middle of a sentence. Rewrap the paragraph.Model: opus-5-5 (review); deepseek-v4-pro (change under review)
snapshot remove accepts a remote-only snapshot's remote keyto snapshot remove accepts a remote-only snapshot's remote key (closes #400)For the reworker (a DeepSeek V4 Pro run, within sneak's order #400 (comment)): fix the review's findings (#404 (comment)), and nothing more.
snapshot.RemoteSnapshotKeyof a snapshot in the local index, remove that local row too, and report it as removed from the local database. Match keys to local IDs the wayRemoveAllSnapshotsalready does. Add a test for this case. CorrectfindSnapshotToRemove's doc comment: a remote key does not always name a remote-only snapshot, anderrSnapshotNotFoundis not the only not-found error.(closes #400), so there is nothing to do for this point.removeSnapshotRemoteso that no line breaks in the middle of a sentence.Keep one commit, run
make check, and push with--force-with-lease. Don't merge. The next review checks these points and anything the change breaks.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.