Skip to content

fix(kiloclaw): adopt an existing volume when the DO loses flyVolumeId - #6271

Merged
iscekic merged 2 commits into
mainfrom
iscekic/kiloclaw-adopt-existing-volume-on-start
Sep 17, 2026
Merged

iscekic merged 2 commits into
mainfrom
iscekic/kiloclaw-adopt-existing-volume-on-start

Conversation

@iscekic

@iscekic iscekic commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

An unexpected-stop recovery can leave the Instance DO with no usable volume reference. Start then created a brand-new empty volume and mounted it over data that still existed on Fly. This is Pylon 28311, and a recurrence of 28216.

ensureVolume (services/kiloclaw/src/durable-objects/kiloclaw-instance/fly-machines.ts:131) now adopts an existing volume before creating one:

  • Lists the app's volumes, keeps exact name matches (volumeNameFromSandboxId), skips pending_destroy / destroying / destroyed, and adopts the newest usable volume.
  • Recovery forks and stranded-volume replacements keep the same volume name and are created last, so the newest usable volume holds the latest data.
  • If the lookup fails on an instance that has started before, start fails closed instead of creating an empty volume.
  • A never-started instance keeps the previous create behavior.
  • Logs adopt_volume with the adopted id, state and created_at, and warns when several usable volumes match.

Verification

  • No manual tests. This is a backend-only Durable Object lifecycle change with no UI path.
    Automated checks in services/kiloclaw: pnpm test (108 files, 2348 tests), pnpm typecheck (tsgo), and pnpm lint (oxlint) all pass.
  • New tests in services/kiloclaw/src/durable-objects/kiloclaw-instance.test.ts cover adoption of the newest usable volume, skipping non-adoptable states, the start path after a lost flyVolumeId, and fail-closed on a failed lookup.

Visual Changes

N/A

Reviewer Notes

  • The selection rule is "newest usable volume with the instance's volume name". The codebase already assumes one sandbox per Fly app (tryDeleteOrphanVolumes), so a name match is unambiguous for ownership; multiple matches only come from retained recovery artifacts. The newest usable volume is the recovery fork that holds the latest data.
  • The fail-closed branch triggers only when lastStartedAt !== null. A never-started instance has no data to protect, so a transient listVolumes failure keeps the old create path.

An unexpected-stop recovery can leave the Instance DO without a usable
volume reference. Start then created a brand-new empty volume and mounted
it over data that still existed on Fly (Pylon 28311, a recurrence of
28216).

ensureVolume now lists the app's volumes, keeps the ones that match the
instance's volume name, skips volumes Fly is reaping, and adopts the
newest usable one. Recovery forks and stranded-volume replacements keep
the same name and are created last, so the newest volume holds the latest
data. If the lookup fails on an instance that has started before, start
fails closed instead of creating an empty volume.

Fresh instances that never started keep the previous create behavior.
@iscekic iscekic self-assigned this Sep 17, 2026
@kilo-code-bot

kilo-code-bot Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (4 files)
  • services/kiloclaw/src/durable-objects/kiloclaw-instance/fly-machines.ts
  • services/kiloclaw/src/durable-objects/kiloclaw-instance/reconcile.ts
  • services/kiloclaw/src/providers/fly/index.ts
  • services/kiloclaw/src/durable-objects/kiloclaw-instance.test.ts
Previous Review Summary (commit 94ef580)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 94ef580)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
services/kiloclaw/src/durable-objects/kiloclaw-instance/fly-machines.ts 101 Adoption ignores attached_machine_id; a volume Fly still considers attached cannot be mounted by createNewMachine, so start fails instead of recovering
services/kiloclaw/src/durable-objects/kiloclaw-instance/fly-machines.ts 154 replace_lost_volume still logs data_loss: true when adoption returns a surviving volume
Files Reviewed (2 files)
  • services/kiloclaw/src/durable-objects/kiloclaw-instance/fly-machines.ts - 2 issues
  • services/kiloclaw/src/durable-objects/kiloclaw-instance.test.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

Address review remarks on volume adoption:

- Skip matching volumes that another machine still holds. A Fly volume
  attaches to one machine, so adopting an attached volume made the
  follow-up createNewMachine fail with an opaque 400. Adopt only
  unattached volumes or ones the DO's known machine already holds, and
  warn when the only candidates are held elsewhere.
- Return whether ensureVolume adopted a volume, and set
  replace_lost_volume.data_loss from it. A repaired pointer that adopted
  a surviving fork no longer reports data loss.
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 17, 2026
@iscekic
iscekic enabled auto-merge (squash) September 17, 2026 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants