From 6b286bc99f30cfed7a1f55b1eb991c24df3147f9 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 21 Aug 2026 22:44:16 +0200 Subject: [PATCH 1/3] fix(daytona): bound refreshData before replacement with lookup deadline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit recreateAfterFailedStart calls refreshData() on the pre-restart sandbox before reading its network/env/labels for replacementCreateParams. That call was unbounded: if the SDK stalled on the refresh (rate limit, transient 5xx, network stall), start() hung forever without ever reaching the replacement create — the exact hang the recreate path exists to avoid. Wrap the refresh with the same `awaitLookupOperation` + `lookupDeadline` pattern already used five other places in this file (rehydrate after start, initial getById lookup, cleanup guards). Same default timeout, same error message shape, no new abstractions. No dedicated unit-test regression is added: the default lookup deadline is 10s, which would make a "hangs forever without the fix" assertion a 10s+ test, and the awaitLookupOperation utility already has a fast unit test at findByLabels (`lookup exceeded 20ms` at runtime.test.ts:325). Existing recreate-path fake-sandbox tests continue to pass (fakeSandbox exposes no refreshData, so the optional call is a no-op both before and after). Addresses cubic-dev-ai review thread on PR #14 (src/daytona/runtime.ts line 772). Suite unchanged: 220 tests / 215 passed / 5 skipped / 0 failed (skips are DAYTONA_API_KEY-gated smoke, unrelated to this change). Co-Authored-By: Claude Opus 4.7 Session-Id: 34c847c1-1a32-4cba-b341-311f694f1145 --- src/daytona/runtime.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/daytona/runtime.ts b/src/daytona/runtime.ts index fc78edb..c597042 100644 --- a/src/daytona/runtime.ts +++ b/src/daytona/runtime.ts @@ -778,8 +778,19 @@ export class DaytonaRuntime implements WorkflowRuntime { // without ever going through get(), which leaves env, volumes, and // network settings unpopulated until refreshData() runs. Hydrate before // reading it for replacementCreateParams so those settings are not - // silently dropped from the replacement. - await (originalSandbox as unknown as { refreshData?: () => Promise }).refreshData?.(); + // silently dropped from the replacement. Bound the refresh with the same + // lookup deadline used elsewhere so a hanging SDK call cannot leave + // recreateAfterFailedStart wedged before the replacement is ever created. + const refreshableSandbox = originalSandbox as unknown as { + refreshData?: () => Promise; + }; + if (typeof refreshableSandbox.refreshData === 'function') { + await awaitLookupOperation( + Promise.resolve(refreshableSandbox.refreshData.call(originalSandbox)), + lookupDeadline(undefined), + `refreshing sandbox ${originalId} before replacement`, + ); + } try { // Do not copy the name: Daytona requires names to be unique while the From 7142b149837f82aaa34e616bf2355c5dc1f87ce2 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 21 Aug 2026 22:45:16 +0200 Subject: [PATCH 2/3] test(daytona): tolerate 404 on smoke-suite cleanup delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The smoke suite's `after` hook loops over createdSandboxIds and calls daytona.get(id) followed by daytona.delete(sandbox). daytona get/delete is eventually consistent: after runtime.destroy(handle) already removes the sandbox, get can still resolve briefly, and the subsequent delete then rejects 404. Only the get was guarded by isTestDaytonaNotFound; the delete rejection landed in cleanupFailures and failed the entire after hook even though cleanup succeeded. Extend the same isTestDaytonaNotFound guard to the delete call so a 404 there is treated as "already gone" rather than a cleanup failure. assertDaytonaSandboxGone still runs afterward and confirms absence in either path, so a real never-deleted sandbox continues to fail the hook. Any non-404 delete error still bubbles. Blast radius: DAYTONA_API_KEY-gated smoke suite only, skipped in CI and in this environment (skip count unchanged: 5 before, 5 after). Worst case if the classifier misclassifies a real delete failure as 404: one cleanupFailures entry silently dropped in the smoke `after` hook — bounded, non-production. Addresses cubic-dev-ai review thread on PR #14 (src/daytona/runtime.test.ts line 1890). Pattern mirrors the adjacent get-guard in the same cleanup loop. Co-Authored-By: Claude Opus 4.7 Session-Id: 34c847c1-1a32-4cba-b341-311f694f1145 --- src/daytona/runtime.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/daytona/runtime.test.ts b/src/daytona/runtime.test.ts index 1dd26af..f0ebe08 100644 --- a/src/daytona/runtime.test.ts +++ b/src/daytona/runtime.test.ts @@ -1920,7 +1920,16 @@ describe('DaytonaRuntime smoke', { concurrency: false }, () => { if (isTestDaytonaNotFound(error)) continue; throw error; } - await daytona.delete(sandbox); + try { + await daytona.delete(sandbox); + } catch (deleteError) { + // Daytona get/delete is eventually consistent: get can still resolve + // for a sandbox that runtime.destroy() already removed, and the + // subsequent delete then rejects 404. That case is cleanup success, + // not failure — assertDaytonaSandboxGone below confirms absence + // regardless. Any other delete error is real and must still bubble. + if (!isTestDaytonaNotFound(deleteError)) throw deleteError; + } await assertDaytonaSandboxGone(daytona, id); } catch (error) { cleanupFailures.push(error); From 6e166c983974ceff733bf505f2bd60a7dc21029c Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 21 Aug 2026 22:47:33 +0200 Subject: [PATCH 3/3] fix(port): restore source-compat on SandboxRuntimeCapabilities MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The modes field on SandboxRuntimeCapabilities was mandatory. External TypeScript consumers that construct fixtures with the five pre-modes booleans (asyncExec, reattach, detachedLaunch, warmLease, lifecycle) stopped compiling on upgrade — the shape had a new required field they had no way to know about, and adding modes to every fixture forces the consumer to make claims about a runtime they might only be mocking for one specific concern. Two-type split preserves both invariants: - `SandboxRuntimeCapabilities`: five booleans + `modes?: SandboxCapabilityModes`. The exported base shape a consumer can construct. - `ResolvedSandboxRuntimeCapabilities = SandboxRuntimeCapabilities & { readonly modes: SandboxCapabilityModes }`: what `resolveSandboxRuntimeCapabilities` actually returns. `modes` is required here and always populated by the resolver (defaulting to "unknown", per the discipline the whole modes design encodes). Callers who receive the resolver's output still see modes as required (no optional-chain gymnastics at call sites). Callers who construct fixtures literal-typed as `SandboxRuntimeCapabilities` no longer need to invent modes they haven't observed. Nobody has to make a claim they can't back. Two new port tests pin the contract: one literal-constructs the base type without modes and asserts the value shape; one assigns the resolver output into `ResolvedSandboxRuntimeCapabilities` and reads modes without a cast, so the compiler enforces the strict-return invariant. `index.ts` re-exports the new `ResolvedSandboxRuntimeCapabilities` alongside the existing base type. Addresses the duplicate P2 threads on PR #17 (chatgpt-codex-connector and cubic-dev-ai, both at src/port.ts line 255). Tests: 222 (+2) / 217 pass / 5 skipped / 0 fail. Skips unchanged. Co-Authored-By: Claude Opus 4.7 Session-Id: 34c847c1-1a32-4cba-b341-311f694f1145 --- src/index.ts | 1 + src/port.test.ts | 32 ++++++++++++++++++++++++++++++++ src/port.ts | 31 ++++++++++++++++++++++++++----- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/src/index.ts b/src/index.ts index 9eb6976..8b5a473 100644 --- a/src/index.ts +++ b/src/index.ts @@ -30,6 +30,7 @@ export type { LifetimeMode, OutputStreamMode, SnapshotMode, + ResolvedSandboxRuntimeCapabilities, RunScriptResult, SandboxCapabilityModes, SandboxCountOptions, diff --git a/src/port.test.ts b/src/port.test.ts index 9ec3c50..3408ad7 100644 --- a/src/port.test.ts +++ b/src/port.test.ts @@ -4,7 +4,9 @@ import { describe, it } from "node:test"; import { isPendingEvidence, resolveSandboxRuntimeCapabilities, + type ResolvedSandboxRuntimeCapabilities, type SandboxRuntime, + type SandboxRuntimeCapabilities, } from "./port.js"; /** Minimal runtime: only what the resolver actually inspects. */ @@ -119,4 +121,34 @@ describe("capability modes", () => { resolveSandboxRuntimeCapabilities(instance), ); }); + + it( + "lets a consumer literal-construct SandboxRuntimeCapabilities without modes", + () => { + // Source-compat contract: the exported base shape must still accept the + // five pre-modes fields alone. External TypeScript consumers that built + // fixtures like this before modes existed compile unchanged. + const preModesFixture: SandboxRuntimeCapabilities = { + asyncExec: false, + reattach: false, + detachedLaunch: false, + warmLease: true, + lifecycle: true, + }; + assert.equal(preModesFixture.modes, undefined); + }, + ); + + it( + "resolver returns the stricter ResolvedSandboxRuntimeCapabilities with modes populated", + () => { + // The resolver's return type has modes required. Assign into the strict + // type without a cast: the compiler enforces that modes is present, and + // the runtime confirms it's populated (with unknowns by default). + const resolved: ResolvedSandboxRuntimeCapabilities = + resolveSandboxRuntimeCapabilities(runtime()); + assert.equal(resolved.modes.outputStreams, "unknown"); + assert.equal(resolved.modes.filesystem, "unknown"); + }, + ); }); diff --git a/src/port.ts b/src/port.ts index c943136..9cd0f23 100644 --- a/src/port.ts +++ b/src/port.ts @@ -233,6 +233,11 @@ export type SandboxRuntime = { * narrow `RuntimeCapabilities` in `./types.ts`, which belongs to the live * in-sandbox bootstrap plane and must not be conflated with it. The two are * kept under distinct names on purpose. + * + * `modes` is optional on this shape so a TypeScript consumer can still + * literal-construct a fixture with the five booleans. The resolver returns the + * stricter `ResolvedSandboxRuntimeCapabilities` where `modes` is required and + * always populated (defaulting to `"unknown"` rather than to a claim). */ export type SandboxRuntimeCapabilities = { /** @@ -249,9 +254,25 @@ export type SandboxRuntimeCapabilities = { /** `start`/`stop` actually change sandbox state rather than no-opping. */ readonly lifecycle: boolean; /** - * Structured detail for the capabilities a boolean flattens. Always present - * after resolution, defaulting to `"unknown"` rather than to a claim. + * Structured detail for the capabilities a boolean flattens. Optional on the + * base shape for source-compat with pre-modes fixtures; always populated on + * the resolver's return type (`ResolvedSandboxRuntimeCapabilities`). */ + readonly modes?: SandboxCapabilityModes; +}; + +/** + * The descriptor `resolveSandboxRuntimeCapabilities` returns. `modes` is + * required here — the resolver always populates it, defaulting to `"unknown"` + * so a runtime that declares nothing makes no new claim while still producing + * a fully-shaped resolved descriptor. + * + * Kept distinct from `SandboxRuntimeCapabilities` so external consumers that + * literal-construct fixtures with only the pre-modes fields continue to + * compile; those fixtures satisfy `SandboxRuntimeCapabilities`, and only code + * reading a resolver output relies on `modes` being present. + */ +export type ResolvedSandboxRuntimeCapabilities = SandboxRuntimeCapabilities & { readonly modes: SandboxCapabilityModes; }; @@ -267,7 +288,7 @@ export type DeclaredSandboxRuntimeCapabilities = Pick< const capabilitiesByRuntime = new WeakMap< SandboxRuntime, - SandboxRuntimeCapabilities + ResolvedSandboxRuntimeCapabilities >(); /** @@ -282,7 +303,7 @@ const capabilitiesByRuntime = new WeakMap< */ export function resolveSandboxRuntimeCapabilities( runtime: SandboxRuntime, -): SandboxRuntimeCapabilities { +): ResolvedSandboxRuntimeCapabilities { const cached = capabilitiesByRuntime.get(runtime); if (cached) { return cached; @@ -295,7 +316,7 @@ export function resolveSandboxRuntimeCapabilities( && typeof runtime.getById === "function" && typeof runtime.getScriptStatus === "function" && typeof runtime.getScriptLogs === "function"; - const resolved: SandboxRuntimeCapabilities = { + const resolved: ResolvedSandboxRuntimeCapabilities = { asyncExec, reattach: typeof runtime.getById === "function", detachedLaunch: typeof runtime.launchDetached === "function",