fix(sandboxes): keep the row restore to a refused delete only

Cursor and Greptile, independently, on the same code. `deleteImage` and
`rebuildIfReadopted` shared one try/catch, so a rebuild failure after a *successful*
provider delete was handled as if the provider had refused: the catch put the
claimed row back, `ready` status and all, pointing at a template that no longer
exists.

That is the one state resolution cannot repair — it fixes a row that is missing or
failed, never one claiming to be ready — so it reintroduced the permanent breakage
an earlier commit had just closed, through the error path rather than the happy one.

Restoring now belongs strictly to a refused delete. Once the template is gone the
row stays gone, and the rebuild runs past that catch. The rebuild also swallows its
own failures: it follows a delete that already succeeded, so it must not be reported
as a failed release, and inside the sweep it must not reject the rest of its chunk.
The adopter's next run still reaches the normal repair path.

The regression test drives a rebuild failure and asserts no row is restored. It
fails against the original shape — rebuild inside the shared try, no inner catch —
which is what the two reviewers were describing.

Co-Authored-By: Claude <noreply@anthropic.com>

Author: icecrasher321

apps/sim/lib/execution/remote-sandbox/image-registry.test.ts

@@ -226,6 +226,23 @@ describe('releaseSandboxImage', () => {
     expect(enqueued()).toBe(false)
   })
 
+  /**
+   * Restoring belongs to a refused delete and nothing else. Once the template is
+   * gone, putting the row back would recreate a `ready` row pointing at nothing —
+   * the one state resolution cannot repair.
+   */
+  it('does not restore the row when the post-delete rebuild fails', async () => {
+    stubClaim([READY_IMAGE])
+    mockSelect.mockImplementation(() => {
+      throw new Error('registry unreachable')
+    })
+
+    await releaseSandboxImage('hash-1')
+
+    expect(mockDeleteImage).toHaveBeenCalledTimes(1)
+    expect(mockInsert).not.toHaveBeenCalled()
+  })
+
   it('skips the provider when the claimed row never had an image', async () => {
     stubClaim([{ ...READY_IMAGE, imageRef: null }])
 

apps/sim/lib/execution/remote-sandbox/image-registry.ts

@@ -379,15 +379,25 @@ async function rebuildIfReadopted(
   specHash: string,
   spec: SandboxSpec
 ): Promise<void> {
-  const [readopted] = await db
-    .select({ id: sandboxImage.id })
-    .from(sandboxImage)
-    .where(and(eq(sandboxImage.provider, providerId), eq(sandboxImage.specHash, specHash)))
-    .limit(1)
-  if (!readopted) return
-
-  logger.warn('Sandbox image was re-adopted mid-delete; rebuilding it now', { specHash })
-  await ensureSandboxImage(spec, specHash, { imageKnownGone: true })
+  try {
+    const [readopted] = await db
+      .select({ id: sandboxImage.id })
+      .from(sandboxImage)
+      .where(and(eq(sandboxImage.provider, providerId), eq(sandboxImage.specHash, specHash)))
+      .limit(1)
+    if (!readopted) return
+
+    logger.warn('Sandbox image was re-adopted mid-delete; rebuilding it now', { specHash })
+    await ensureSandboxImage(spec, specHash, { imageKnownGone: true })
+  } catch (error) {
+    // Opportunistic: the delete it follows has already succeeded, so failing here
+    // must not surface as a failed release, and in the sweep must not reject the
+    // rest of its chunk. The adopter's next run still reaches the repair path.
+    logger.warn('Failed to rebuild a re-adopted sandbox image', {
+      specHash,
+      error: getErrorMessage(error),
+    })
+  }
 }
 
 /**
@@ -444,8 +454,6 @@ async function claimAndDeleteImage(
       buildId: claimed.buildId ?? '',
       providerImageId: claimed.providerImageId ?? undefined,
     })
-    await rebuildIfReadopted(providerId, specHash, claimed.spec as SandboxSpec)
-    return 'released'
   } catch (error) {
     await db
       .insert(sandboxImage)
@@ -466,6 +474,12 @@ async function claimAndDeleteImage(
     })
     return 'failed'
   }
+
+  // Deliberately past the catch above. The template is gone for good by now, so a
+  // failure here must not restore the row: that path puts back a `ready` row whose
+  // imageRef points at nothing, which is the one state resolution cannot repair.
+  await rebuildIfReadopted(providerId, specHash, claimed.spec as SandboxSpec)
+  return 'released'
 }
 
 /** Most rows one sweep will touch. The next run picks up whatever is left. */