Skip to content

Commit f09dc99

Browse files
icecrasher321claude
andcommitted
fix(sandboxes): drop the dead row when a re-adopt rebuild cannot be scheduled
Greptile, one layer under the previous fix. Making the post-delete rebuild swallow its own failures kept it from being reported as a failed release, but left the adopter's row claiming a `ready` image whose template is already deleted — the one state resolution cannot repair, since it rebuilds a row that is missing or failed and never one that says ready. So the row is now dropped when the rebuild does not take. That turns the adopter into the missing-row case, which the next execution repairs on its own, instead of a sandbox that stays broken until someone re-saves it by hand. A failure to drop it is logged at error, because at that point two writes in a row have failed and there is nothing further this path can do. Also gives the release tests a default "nothing re-adopted" select. Without it the rebuild threw on an unstubbed mock and the cleanup delete overwrote the predicate the claim assertions read, so two of them were passing on the wrong statement. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 29b7442 commit f09dc99

2 files changed

Lines changed: 40 additions & 4 deletions

File tree

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ beforeEach(() => {
8383
mockProviderStrategy.current = 'prebuilt'
8484
mockDelete.mockReturnValue({ where: () => ({ returning: () => Promise.resolve([]) }) })
8585
mockInsert.mockReturnValue({ values: () => ({ onConflictDoNothing: () => Promise.resolve() }) })
86+
// Default: nothing re-adopted the hash, so the post-delete rebuild is a no-op and
87+
// cases that are not about it stay on one `delete`.
88+
mockSelect.mockReturnValue({
89+
from: () => ({ where: () => ({ limit: () => Promise.resolve([]) }) }),
90+
})
8691
mockDeleteImage.mockResolvedValue(undefined)
8792
})
8893

@@ -243,6 +248,24 @@ describe('releaseSandboxImage', () => {
243248
expect(mockInsert).not.toHaveBeenCalled()
244249
})
245250

251+
/**
252+
* A row claiming `ready` against a deleted template is the state resolution
253+
* cannot repair, so a rebuild that does not take must not leave one behind.
254+
* Dropping it converts the adopter into the missing-row case, which the next
255+
* execution fixes on its own.
256+
*/
257+
it('drops the dead row when the rebuild cannot be scheduled', async () => {
258+
stubClaim([READY_IMAGE])
259+
mockSelect.mockImplementation(() => {
260+
throw new Error('registry unreachable')
261+
})
262+
263+
await releaseSandboxImage('hash-1')
264+
265+
// The claim itself plus the dead-row cleanup.
266+
expect(mockDelete).toHaveBeenCalledTimes(2)
267+
})
268+
246269
it('skips the provider when the claimed row never had an image', async () => {
247270
stubClaim([{ ...READY_IMAGE, imageRef: null }])
248271

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -390,13 +390,26 @@ async function rebuildIfReadopted(
390390
logger.warn('Sandbox image was re-adopted mid-delete; rebuilding it now', { specHash })
391391
await ensureSandboxImage(spec, specHash, { imageKnownGone: true })
392392
} catch (error) {
393-
// Opportunistic: the delete it follows has already succeeded, so failing here
394-
// must not surface as a failed release, and in the sweep must not reject the
395-
// rest of its chunk. The adopter's next run still reaches the repair path.
396-
logger.warn('Failed to rebuild a re-adopted sandbox image', {
393+
// The template is gone and the rebuild did not take, so the adopter's row now
394+
// claims a `ready` image that does not exist — the one state resolution cannot
395+
// repair, because it only rebuilds a row that is missing or `failed`. Dropping
396+
// the row converts that into the missing case, which the next execution fixes
397+
// on its own. Swallowing instead would leave the sandbox broken until someone
398+
// re-saved it by hand.
399+
logger.warn('Failed to rebuild a re-adopted sandbox image; dropping the dead row', {
397400
specHash,
398401
error: getErrorMessage(error),
399402
})
403+
try {
404+
await db
405+
.delete(sandboxImage)
406+
.where(and(eq(sandboxImage.provider, providerId), eq(sandboxImage.specHash, specHash)))
407+
} catch (cleanupError) {
408+
logger.error('Could not drop a sandbox image row pointing at a deleted template', {
409+
specHash,
410+
error: getErrorMessage(cleanupError),
411+
})
412+
}
400413
}
401414
}
402415

0 commit comments

Comments
 (0)