Repository navigation
fix(kiloclaw): size snapshot restore volume from snapshot source - #6239
Conversation
Fly requires an explicit size_gb for a snapshot restore volume and rejects a size smaller than the snapshot's source volume. The queue worker sized the new volume from the current volume, so restoring a snapshot whose source volume is larger failed with a 400 and reverted the instance to stopped. Resolve the snapshot's source volume size by scanning the app volumes and create the restore volume at the larger of the current and source sizes. Add queue worker tests for both sizing directions and the not-found fallback.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe incremental changes skip volumes too small to raise the restore size and treat 400/404/422 vanished-volume responses as a non-match during the snapshot scan, with test coverage for both; the previously raised findings are resolved and no new issues were found. Files Reviewed (2 files)
Previous Review Summary (commit 17a5eb5)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 17a5eb5)Status: 2 Issues Found | Recommendation: Address before merge Executive SummaryThe snapshot source-volume sizing fix is correct in its core logic, but the mid-scan missing-volume handling only catches HTTP 404 and can abort the whole restore, and the per-volume snapshot scan fans out without bound. Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (2 files)
Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
Address review remarks on the snapshot restore sizing fix: - Match vanished volumes with isFlyMissingVolume so the scan also tolerates the 400 and 422 responses Fly uses, not only 404. - Skip volumes smaller than the current volume, which cannot raise the restore size, to bound the scan as retained volumes accumulate. Add tests for a volume that vanishes mid-scan and for skipped small volumes.
isFlyMissingVolume requires both a missing-volume status code and a known marker, so a bare 404 without a marker would have thrown. Match the platform route pattern and accept either check.
| currentSizeGb: number | ||
| ): Promise<number | null> { | ||
| const volumes = await fly.listVolumes(flyConfig); | ||
| const candidates = volumes.filter(volume => volume.size_gb >= currentSizeGb); |
There was a problem hiding this comment.
bot: Same-sized retained volumes are still scanned even though they cannot affect the restore size.
Suggested fix: Use volume.size_gb > currentSizeGb rather than >=. A snapshot from an equal-sized source is already safely covered by the existingVolume.size_gb fallback in Math.max, so querying it cannot change the create request. Add a test with a same-sized retained volume and assert listVolumeSnapshots is not called for it. This prevents the concurrent Fly request fan-out from growing for the common case where each retained restore volume has the same size.
2 participants
Summary
The KiloClaw snapshot-restore queue worker sized the new volume from the instance's
current volume. Fly rejects a restore volume smaller than the snapshot's source
volume, so restoring a snapshot from a larger previous or recovery volume failed
with
volume size (10 GiB) is smaller than the snapshot size (11 GiB), exhaustedthe queue retries, and reset the instance to
stopped.This change resolves the snapshot's source volume size by scanning the app volumes
(there is no Fly endpoint to fetch a snapshot by id) and creates the restore volume
at the larger of the current and source sizes. When the snapshot cannot be found,
the current volume size is used, which preserves the previous behavior.
Found while recovering a customer instance whose bot came back on a fresh empty
volume: recovering their data required restoring a snapshot from the retained
previous volume, which hit this exact failure.
Verification
Automated checks:
pnpm exec vitest runinservices/kiloclaw(108 files, 2342 tests),pnpm exec tsgo --noEmit, andoxlinton the changed files all pass.Visual Changes
N/A
Reviewer Notes
services/kiloclaw/src/queue/snapshot-restore.test.tscover a largersnapshot source, a smaller snapshot source, the not-found fallback, the happy
path swap/start, and the idempotency skip.
resolveSnapshotVolumeSizeGbscans the app's volumes in parallel and treats a404 from a volume that disappeared mid-scan as "no match".