From 795991cd177d280cf6ddbe36aa6d69d2fef76868 Mon Sep 17 00:00:00 2001 From: Ethan Stoner Date: Wed, 26 Aug 2026 17:32:34 -0700 Subject: [PATCH 1/3] fix(sandbox): bound the Daytona calls that requests wait on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sandbox build-status refresh talks to Daytona and had no cap on the round-trip. It runs on GET /api/v1/capabilities, on GET /api/v1/settings/sandbox-providers, and on turn creation when a fresh sandbox is needed, so a provider that accepted the connection and then stalled held those requests until undici's own multi-minute defaults — once per request, with each one keeping a server connection and an event-loop task alive. A Daytona brownout became piled-up concurrent requests, and nothing was logged while it happened. Only the settings write bounded this, wrapping buildImage in withTimeout at three seconds. That budget now lives beside the refresh as SANDBOX_BUILD_REQUEST_TIMEOUT_MS and both paths import it: they issue the same calls, and a duplicated number is one edit away from disagreeing. On expiry the refresh returns the last persisted status instead of throwing. All three callers are asking what the current status is, not asking to reach Daytona: the capability probe already fails closed on an exception and would report the sandbox unconfigured, the settings read already falls back to the record, and turn creation would take an exception it has no better answer for. A stale status serves each of them better than a hang or a 500 while the provider is unwell. A real error — rejected credentials, a failed build — still propagates, so this does not hide anything except slowness. Snapshot registration also carries its own timeout now. withTimeout is a Promise.race: it abandons the result but the request keeps running, so a caller-side cap alone leaves the socket open for the full undici default. AbortSignal.timeout ends the request itself. Verified: the two stall cases hang and fail on a four-second test timeout with the withTimeout calls removed, and pass with them, so the tests fail without the fix. tests/unit/apis and tests/unit/sandbox: 153 passed. tsc clean for trueforge src, its tests/unit project, and trueforge-core. Six failures elsewhere in the package are a Windows host — unix sockets and symlink paths under LocalSandboxProvider — and fail identically on a clean checkout. Not addressed here: the issue also notes that checkSnapshotStatus mutates persisted state and can reactivate a snapshot from GET handlers, so reads drive external writes. That is a design question about where the refresh belongs rather than a timeout, and changing it would move behaviour this change is trying to leave alone. --- .changeset/daytona-request-timeouts.md | 10 ++ .../core/sandbox/provider/DaytonaProvider.ts | 9 ++ .../trueforge/src/apis/sandboxProviders.ts | 6 +- .../trueforge/src/sandbox/providerUtils.ts | 48 +++++++- .../tests/unit/sandbox/providerUtils.test.ts | 111 ++++++++++++++++++ 5 files changed, 174 insertions(+), 10 deletions(-) create mode 100644 .changeset/daytona-request-timeouts.md create mode 100644 packages/trueforge/tests/unit/sandbox/providerUtils.test.ts diff --git a/.changeset/daytona-request-timeouts.md b/.changeset/daytona-request-timeouts.md new file mode 100644 index 000000000..6d4f6368c --- /dev/null +++ b/.changeset/daytona-request-timeouts.md @@ -0,0 +1,10 @@ +--- +"@truefoundry/trueforge": patch +"@truefoundry/trueforge-core": patch +--- + +Bound the Daytona calls reachable from request-handling paths, so a stalled provider fails fast instead of hanging a request. + +The sandbox build-status refresh runs on the capability probe, the sandbox settings read, and turn creation, and none of those capped the round-trip. A Daytona endpoint that accepted the connection and then stalled held each request until undici's multi-minute default, piling up connections during a provider brownout. The refresh now uses the same budget the settings write already applied, and reports the last persisted status when it expires rather than hanging or failing the caller. + +Snapshot registration also carries its own request timeout. Racing a promise against a timer abandons the result but leaves the request running, so the socket needed a deadline of its own. diff --git a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts index 1fba52e16..ddd3b57c6 100644 --- a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts +++ b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts @@ -28,6 +28,14 @@ const BUILD_STATE_INACTIVE = 'inactive'; const BUILD_STATE_ERROR = 'error'; const BUILD_STATE_BUILD_FAILED = 'build_failed'; +/** + * Cap on the snapshot registration round-trip. + * + * A caller racing this against `withTimeout` abandons the promise but leaves the request running, + * so the socket is only released by undici's own ~5-minute default. This aborts the request itself. + */ +const SNAPSHOT_REGISTER_TIMEOUT_MS = 10_000; + const IMAGE_BUILD_NAME_PREFIX = 'trueforge-build-'; /** Same default the Daytona SDK applies when `DaytonaConfig.apiUrl` is omitted. */ const DEFAULT_DAYTONA_API_URL = 'https://app.daytona.io/api'; @@ -312,6 +320,7 @@ export class DaytonaSandboxProvider implements SandboxProvider { 'Content-Type': 'application/json', }, body: JSON.stringify({ name: this.buildRef, imageName: this.imageUri }), + signal: AbortSignal.timeout(SNAPSHOT_REGISTER_TIMEOUT_MS), }); } catch (error) { throw new Error('Daytona snapshot registration request failed.', { cause: error }); diff --git a/packages/trueforge/src/apis/sandboxProviders.ts b/packages/trueforge/src/apis/sandboxProviders.ts index 9df3afa23..93dd0bab8 100644 --- a/packages/trueforge/src/apis/sandboxProviders.ts +++ b/packages/trueforge/src/apis/sandboxProviders.ts @@ -7,6 +7,7 @@ import { getSandboxProviderRoute, putSandboxProviderRoute } from '../routes/sand import { checkSnapshotStatus, isDaytonaAuthError, + SANDBOX_BUILD_REQUEST_TIMEOUT_MS, toDaytonaSandboxProvider, toSandboxStatus, } from '../sandbox/providerUtils'; @@ -14,9 +15,6 @@ import type { SandboxProviderManifest, UpdateSandboxProviderRequest } from '../s import { MissingStoredSecretError, resolveStoredSecretValue, toRedactedSecretValue } from '../utils/secretRedaction'; import { TENANT_ID } from './sessions'; -/** Cap the Daytona register round-trip so a slow/unreachable provider can't hold the request (or DB txn) open. */ -const BUILD_REQUEST_TIMEOUT_MS = 3_000; - export interface SandboxProvidersRouterDeps { sandboxProviderStore: ISandboxProviderStore; withTransaction: WithTransaction; @@ -81,7 +79,7 @@ export function createSandboxProvidersRouter(deps: SandboxProvider ...(locked ? { build_metadata: locked.build_metadata } : {}), }); const built = toSandboxStatus( - await withTimeout(provider.buildImage(), BUILD_REQUEST_TIMEOUT_MS, 'sandbox buildImage'), + await withTimeout(provider.buildImage(), SANDBOX_BUILD_REQUEST_TIMEOUT_MS, 'sandbox buildImage'), ); await deps.sandboxProviderStore.upsertSandboxProvider( { tenant_id: TENANT_ID, manifest: resolved, ...built }, diff --git a/packages/trueforge/src/sandbox/providerUtils.ts b/packages/trueforge/src/sandbox/providerUtils.ts index 9189f7742..b1329780f 100644 --- a/packages/trueforge/src/sandbox/providerUtils.ts +++ b/packages/trueforge/src/sandbox/providerUtils.ts @@ -1,6 +1,13 @@ /** Daytona provider construction + persisted build-status refresh (see checkSnapshotStatus). */ import { Daytona, DaytonaError } from '@daytona/sdk'; -import { DaytonaSandboxProvider, SANDBOX_IMAGE_URI, type SandboxBuild } from '@truefoundry/trueforge-core/core'; +import { + DaytonaSandboxProvider, + PromiseTimeoutError, + SANDBOX_IMAGE_URI, + extractErrorLogFields, + withTimeout, + type SandboxBuild, +} from '@truefoundry/trueforge-core/core'; import type { Logger } from 'winston'; import configuration from '../config'; import type { ISandboxProviderStore, SandboxProviderRecord } from '../db/sandboxProviderStore'; @@ -68,6 +75,14 @@ function sandboxStatusFromRecord(record: SandboxProviderRecord): SandboxStatus { // Daytona deactivates idle snapshots after 14 days; revalidate at 13 to stay a day ahead. const READY_REVALIDATE_INTERVAL_MS = 13 * 24 * 60 * 60 * 1000; +/** + * Cap the Daytona round-trip so a slow or unreachable provider can't hold a request (or DB txn) open. + * + * One budget for both the settings write and the refresh below: they issue the same calls, and a + * refresh sits on the capability probe and on turn creation, where a stall is felt sooner. + */ +export const SANDBOX_BUILD_REQUEST_TIMEOUT_MS = 3_000; + export async function checkSnapshotStatus({ store, tenant_id, @@ -97,11 +112,32 @@ export async function checkSnapshotStatus({ build_metadata: record.build_metadata, }); let build: SandboxBuild; - if (record.status === 'ready') { - // this is because image may have deactivated - build = await provider.buildImage(); - } else { - build = await provider.getImageBuildStatus(); + try { + if (record.status === 'ready') { + // this is because image may have deactivated + build = await withTimeout(provider.buildImage(), SANDBOX_BUILD_REQUEST_TIMEOUT_MS, 'sandbox buildImage'); + } else { + build = await withTimeout( + provider.getImageBuildStatus(), + SANDBOX_BUILD_REQUEST_TIMEOUT_MS, + 'sandbox getImageBuildStatus', + ); + } + } catch (error) { + if (error instanceof PromiseTimeoutError) { + /* + * A refresh that cannot answer in time reports what was last persisted rather than failing + * the caller. This runs on the capability probe, the settings read, and turn creation, none + * of which are asking to talk to Daytona — they are asking for the current status, and a + * stale answer serves them better than a hang or a 500 while the provider is unwell. + */ + logger.warn('Sandbox image status refresh timed out; using last persisted status', { + ...extractErrorLogFields(error), + tenant_id, + }); + return persisted; + } + throw error; } const next = toSandboxStatus(build); const updated = await store.updateSandboxStatus({ tenant_id, ...next }); diff --git a/packages/trueforge/tests/unit/sandbox/providerUtils.test.ts b/packages/trueforge/tests/unit/sandbox/providerUtils.test.ts new file mode 100644 index 000000000..0ea407bfc --- /dev/null +++ b/packages/trueforge/tests/unit/sandbox/providerUtils.test.ts @@ -0,0 +1,111 @@ +// The refresh builds a real provider and then talks to Daytona. Stub the provider class itself so +// these exercise the refresh's timing behaviour without a network, keeping the rest of the module +// (withTimeout, PromiseTimeoutError) real. +jest.mock('@truefoundry/trueforge-core/core', () => { + const actual = jest.requireActual('@truefoundry/trueforge-core/core'); + return { ...actual, DaytonaSandboxProvider: jest.fn() }; +}); + +import { DaytonaSandboxProvider } from '@truefoundry/trueforge-core/core'; +import { createLogger } from 'winston'; +import type { ISandboxProviderStore, SandboxProviderRecord } from '../../../src/db/sandboxProviderStore'; +import { checkSnapshotStatus, SANDBOX_BUILD_REQUEST_TIMEOUT_MS } from '../../../src/sandbox/providerUtils'; + +const mockProvider = DaytonaSandboxProvider as unknown as jest.Mock; +const silentLogger = createLogger({ silent: true }); + +const BUILD_METADATA = { build_ref: 'trueforge-build-029ea5ff', image_uri: 'sandbox:029ea5ff' }; + +function recordWith(overrides: Partial): SandboxProviderRecord { + return { + tenant_id: 'tenant', + manifest: { auth: { api_key: 'dt-key' } }, + status: 'pending', + status_reason: 'Sandbox image build in progress (building).', + build_metadata: BUILD_METADATA, + updated_at: new Date().toISOString(), + ...overrides, + } as SandboxProviderRecord; +} + +function storeReturning(record: SandboxProviderRecord | undefined) { + return { + getSandboxProvider: jest.fn().mockResolvedValue(record), + updateSandboxStatus: jest.fn().mockResolvedValue(undefined), + } as unknown as ISandboxProviderStore & { + getSandboxProvider: jest.Mock; + updateSandboxStatus: jest.Mock; + }; +} + +/** A call that never settles, standing in for a Daytona endpoint that accepts and then stalls. */ +const neverSettles = () => new Promise(() => undefined); + +describe('checkSnapshotStatus when Daytona stalls', () => { + beforeEach(() => { + jest.useFakeTimers(); + mockProvider.mockReset(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('gives up on a stalled status read and reports the persisted status', async () => { + /* + * This refresh sits on the capability probe, the settings read, and turn creation. Unbounded, a + * provider that accepts the connection and never answers held each of those for undici's + * multi-minute default. + */ + const store = storeReturning(recordWith({ status: 'pending' })); + mockProvider.mockImplementation(() => ({ getImageBuildStatus: neverSettles, buildImage: neverSettles })); + + const pending = checkSnapshotStatus({ store, tenant_id: 'tenant', logger: silentLogger }); + await jest.advanceTimersByTimeAsync(SANDBOX_BUILD_REQUEST_TIMEOUT_MS + 1); + + expect(await pending).toEqual({ + status: 'pending', + status_reason: 'Sandbox image build in progress (building).', + build_metadata: BUILD_METADATA, + }); + // Nothing was written, so a stalled provider cannot overwrite a good status with a guess. + expect(store.updateSandboxStatus).not.toHaveBeenCalled(); + }); + + it('gives up on a stalled reactivation of a ready snapshot', async () => { + // A 'ready' record older than the revalidate interval takes the buildImage branch. + const stale = new Date(Date.now() - 14 * 24 * 60 * 60 * 1000).toISOString(); + const store = storeReturning(recordWith({ status: 'ready', status_reason: null, updated_at: stale })); + mockProvider.mockImplementation(() => ({ getImageBuildStatus: neverSettles, buildImage: neverSettles })); + + const pending = checkSnapshotStatus({ store, tenant_id: 'tenant', logger: silentLogger }); + await jest.advanceTimersByTimeAsync(SANDBOX_BUILD_REQUEST_TIMEOUT_MS + 1); + + expect((await pending)?.status).toBe('ready'); + }); + + it('still surfaces a real failure rather than hiding it behind the persisted status', async () => { + const store = storeReturning(recordWith({ status: 'pending' })); + const refused = new Error('Daytona rejected the API key'); + mockProvider.mockImplementation(() => ({ + getImageBuildStatus: () => Promise.reject(refused), + buildImage: neverSettles, + })); + + await expect(checkSnapshotStatus({ store, tenant_id: 'tenant', logger: silentLogger })).rejects.toBe(refused); + }); + + it('reports a status that arrives within the budget', async () => { + const store = storeReturning(recordWith({ status: 'pending' })); + store.updateSandboxStatus.mockResolvedValue(undefined); + mockProvider.mockImplementation(() => ({ + getImageBuildStatus: () => Promise.resolve({ status: 'ready', reason: null, metadata: BUILD_METADATA }), + buildImage: neverSettles, + })); + + const status = await checkSnapshotStatus({ store, tenant_id: 'tenant', logger: silentLogger }); + + expect(status?.status).toBe('ready'); + expect(store.updateSandboxStatus).toHaveBeenCalled(); + }); +}); From 66af42205d89552adbe57a8a5b198d9a976d60cd Mon Sep 17 00:00:00 2001 From: Ethan Stoner Date: Thu, 27 Aug 2026 00:30:31 -0700 Subject: [PATCH 2/3] address review: 1 min budget outside the transaction, throw on expiry The refresh runs outside a transaction, so its budget is a minute rather than the three seconds the settings write uses inside one. The write is untouched and keeps its own constant. On expiry it throws instead of returning the persisted status. Returning it would report 'ready' for a snapshot that may have been deactivated, which is the case the buildImage branch exists to catch. getCapabilities already treats a failure as sandbox-disabled. Registration's request timeout matches the longest caller budget, so the socket outlives no one still waiting on it. Comments trimmed and the test file removed, as asked. --- .changeset/daytona-request-timeouts.md | 6 +- .../core/sandbox/provider/DaytonaProvider.ts | 9 +- .../trueforge/src/apis/sandboxProviders.ts | 6 +- .../trueforge/src/sandbox/providerUtils.ts | 42 ++----- .../tests/unit/sandbox/providerUtils.test.ts | 111 ------------------ 5 files changed, 19 insertions(+), 155 deletions(-) delete mode 100644 packages/trueforge/tests/unit/sandbox/providerUtils.test.ts diff --git a/.changeset/daytona-request-timeouts.md b/.changeset/daytona-request-timeouts.md index 6d4f6368c..9f9cce74c 100644 --- a/.changeset/daytona-request-timeouts.md +++ b/.changeset/daytona-request-timeouts.md @@ -3,8 +3,8 @@ "@truefoundry/trueforge-core": patch --- -Bound the Daytona calls reachable from request-handling paths, so a stalled provider fails fast instead of hanging a request. +Bound the Daytona calls reachable from request-handling paths, so a stalled provider fails instead of hanging a request. -The sandbox build-status refresh runs on the capability probe, the sandbox settings read, and turn creation, and none of those capped the round-trip. A Daytona endpoint that accepted the connection and then stalled held each request until undici's multi-minute default, piling up connections during a provider brownout. The refresh now uses the same budget the settings write already applied, and reports the last persisted status when it expires rather than hanging or failing the caller. +The sandbox build-status refresh runs on the capability probe, the sandbox settings read, and turn creation, and none of those capped the round-trip. A Daytona endpoint that accepted the connection and then stalled held each request until undici's multi-minute default, piling up connections during a provider brownout. The refresh now gives up after a minute; the capability probe already treats a failure as sandbox-disabled. -Snapshot registration also carries its own request timeout. Racing a promise against a timer abandons the result but leaves the request running, so the socket needed a deadline of its own. +Snapshot registration also carries its own request timeout. Racing a promise against a timer frees the caller but leaves the request running, so the socket needed a deadline of its own. diff --git a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts index ddd3b57c6..783e0c6dc 100644 --- a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts +++ b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts @@ -29,12 +29,13 @@ const BUILD_STATE_ERROR = 'error'; const BUILD_STATE_BUILD_FAILED = 'build_failed'; /** - * Cap on the snapshot registration round-trip. + * Cap on the snapshot registration request itself. * - * A caller racing this against `withTimeout` abandons the promise but leaves the request running, - * so the socket is only released by undici's own ~5-minute default. This aborts the request itself. + * `withTimeout` at a caller is a `Promise.race`: it frees the caller but leaves the request running, + * so the connection survives until undici's own default. Matches the longest caller budget, so the + * socket outlives no one still waiting on it. */ -const SNAPSHOT_REGISTER_TIMEOUT_MS = 10_000; +const SNAPSHOT_REGISTER_TIMEOUT_MS = 60_000; const IMAGE_BUILD_NAME_PREFIX = 'trueforge-build-'; /** Same default the Daytona SDK applies when `DaytonaConfig.apiUrl` is omitted. */ diff --git a/packages/trueforge/src/apis/sandboxProviders.ts b/packages/trueforge/src/apis/sandboxProviders.ts index 93dd0bab8..9df3afa23 100644 --- a/packages/trueforge/src/apis/sandboxProviders.ts +++ b/packages/trueforge/src/apis/sandboxProviders.ts @@ -7,7 +7,6 @@ import { getSandboxProviderRoute, putSandboxProviderRoute } from '../routes/sand import { checkSnapshotStatus, isDaytonaAuthError, - SANDBOX_BUILD_REQUEST_TIMEOUT_MS, toDaytonaSandboxProvider, toSandboxStatus, } from '../sandbox/providerUtils'; @@ -15,6 +14,9 @@ import type { SandboxProviderManifest, UpdateSandboxProviderRequest } from '../s import { MissingStoredSecretError, resolveStoredSecretValue, toRedactedSecretValue } from '../utils/secretRedaction'; import { TENANT_ID } from './sessions'; +/** Cap the Daytona register round-trip so a slow/unreachable provider can't hold the request (or DB txn) open. */ +const BUILD_REQUEST_TIMEOUT_MS = 3_000; + export interface SandboxProvidersRouterDeps { sandboxProviderStore: ISandboxProviderStore; withTransaction: WithTransaction; @@ -79,7 +81,7 @@ export function createSandboxProvidersRouter(deps: SandboxProvider ...(locked ? { build_metadata: locked.build_metadata } : {}), }); const built = toSandboxStatus( - await withTimeout(provider.buildImage(), SANDBOX_BUILD_REQUEST_TIMEOUT_MS, 'sandbox buildImage'), + await withTimeout(provider.buildImage(), BUILD_REQUEST_TIMEOUT_MS, 'sandbox buildImage'), ); await deps.sandboxProviderStore.upsertSandboxProvider( { tenant_id: TENANT_ID, manifest: resolved, ...built }, diff --git a/packages/trueforge/src/sandbox/providerUtils.ts b/packages/trueforge/src/sandbox/providerUtils.ts index b1329780f..65974eb36 100644 --- a/packages/trueforge/src/sandbox/providerUtils.ts +++ b/packages/trueforge/src/sandbox/providerUtils.ts @@ -2,9 +2,7 @@ import { Daytona, DaytonaError } from '@daytona/sdk'; import { DaytonaSandboxProvider, - PromiseTimeoutError, SANDBOX_IMAGE_URI, - extractErrorLogFields, withTimeout, type SandboxBuild, } from '@truefoundry/trueforge-core/core'; @@ -75,13 +73,8 @@ function sandboxStatusFromRecord(record: SandboxProviderRecord): SandboxStatus { // Daytona deactivates idle snapshots after 14 days; revalidate at 13 to stay a day ahead. const READY_REVALIDATE_INTERVAL_MS = 13 * 24 * 60 * 60 * 1000; -/** - * Cap the Daytona round-trip so a slow or unreachable provider can't hold a request (or DB txn) open. - * - * One budget for both the settings write and the refresh below: they issue the same calls, and a - * refresh sits on the capability probe and on turn creation, where a stall is felt sooner. - */ -export const SANDBOX_BUILD_REQUEST_TIMEOUT_MS = 3_000; +/** Cap the Daytona round-trip for the refresh, which runs outside a transaction. */ +const STATUS_REFRESH_TIMEOUT_MS = 60_000; export async function checkSnapshotStatus({ store, @@ -112,32 +105,11 @@ export async function checkSnapshotStatus({ build_metadata: record.build_metadata, }); let build: SandboxBuild; - try { - if (record.status === 'ready') { - // this is because image may have deactivated - build = await withTimeout(provider.buildImage(), SANDBOX_BUILD_REQUEST_TIMEOUT_MS, 'sandbox buildImage'); - } else { - build = await withTimeout( - provider.getImageBuildStatus(), - SANDBOX_BUILD_REQUEST_TIMEOUT_MS, - 'sandbox getImageBuildStatus', - ); - } - } catch (error) { - if (error instanceof PromiseTimeoutError) { - /* - * A refresh that cannot answer in time reports what was last persisted rather than failing - * the caller. This runs on the capability probe, the settings read, and turn creation, none - * of which are asking to talk to Daytona — they are asking for the current status, and a - * stale answer serves them better than a hang or a 500 while the provider is unwell. - */ - logger.warn('Sandbox image status refresh timed out; using last persisted status', { - ...extractErrorLogFields(error), - tenant_id, - }); - return persisted; - } - throw error; + if (record.status === 'ready') { + // this is because image may have deactivated + build = await withTimeout(provider.buildImage(), STATUS_REFRESH_TIMEOUT_MS, 'sandbox buildImage'); + } else { + build = await withTimeout(provider.getImageBuildStatus(), STATUS_REFRESH_TIMEOUT_MS, 'sandbox getImageBuildStatus'); } const next = toSandboxStatus(build); const updated = await store.updateSandboxStatus({ tenant_id, ...next }); diff --git a/packages/trueforge/tests/unit/sandbox/providerUtils.test.ts b/packages/trueforge/tests/unit/sandbox/providerUtils.test.ts deleted file mode 100644 index 0ea407bfc..000000000 --- a/packages/trueforge/tests/unit/sandbox/providerUtils.test.ts +++ /dev/null @@ -1,111 +0,0 @@ -// The refresh builds a real provider and then talks to Daytona. Stub the provider class itself so -// these exercise the refresh's timing behaviour without a network, keeping the rest of the module -// (withTimeout, PromiseTimeoutError) real. -jest.mock('@truefoundry/trueforge-core/core', () => { - const actual = jest.requireActual('@truefoundry/trueforge-core/core'); - return { ...actual, DaytonaSandboxProvider: jest.fn() }; -}); - -import { DaytonaSandboxProvider } from '@truefoundry/trueforge-core/core'; -import { createLogger } from 'winston'; -import type { ISandboxProviderStore, SandboxProviderRecord } from '../../../src/db/sandboxProviderStore'; -import { checkSnapshotStatus, SANDBOX_BUILD_REQUEST_TIMEOUT_MS } from '../../../src/sandbox/providerUtils'; - -const mockProvider = DaytonaSandboxProvider as unknown as jest.Mock; -const silentLogger = createLogger({ silent: true }); - -const BUILD_METADATA = { build_ref: 'trueforge-build-029ea5ff', image_uri: 'sandbox:029ea5ff' }; - -function recordWith(overrides: Partial): SandboxProviderRecord { - return { - tenant_id: 'tenant', - manifest: { auth: { api_key: 'dt-key' } }, - status: 'pending', - status_reason: 'Sandbox image build in progress (building).', - build_metadata: BUILD_METADATA, - updated_at: new Date().toISOString(), - ...overrides, - } as SandboxProviderRecord; -} - -function storeReturning(record: SandboxProviderRecord | undefined) { - return { - getSandboxProvider: jest.fn().mockResolvedValue(record), - updateSandboxStatus: jest.fn().mockResolvedValue(undefined), - } as unknown as ISandboxProviderStore & { - getSandboxProvider: jest.Mock; - updateSandboxStatus: jest.Mock; - }; -} - -/** A call that never settles, standing in for a Daytona endpoint that accepts and then stalls. */ -const neverSettles = () => new Promise(() => undefined); - -describe('checkSnapshotStatus when Daytona stalls', () => { - beforeEach(() => { - jest.useFakeTimers(); - mockProvider.mockReset(); - }); - - afterEach(() => { - jest.useRealTimers(); - }); - - it('gives up on a stalled status read and reports the persisted status', async () => { - /* - * This refresh sits on the capability probe, the settings read, and turn creation. Unbounded, a - * provider that accepts the connection and never answers held each of those for undici's - * multi-minute default. - */ - const store = storeReturning(recordWith({ status: 'pending' })); - mockProvider.mockImplementation(() => ({ getImageBuildStatus: neverSettles, buildImage: neverSettles })); - - const pending = checkSnapshotStatus({ store, tenant_id: 'tenant', logger: silentLogger }); - await jest.advanceTimersByTimeAsync(SANDBOX_BUILD_REQUEST_TIMEOUT_MS + 1); - - expect(await pending).toEqual({ - status: 'pending', - status_reason: 'Sandbox image build in progress (building).', - build_metadata: BUILD_METADATA, - }); - // Nothing was written, so a stalled provider cannot overwrite a good status with a guess. - expect(store.updateSandboxStatus).not.toHaveBeenCalled(); - }); - - it('gives up on a stalled reactivation of a ready snapshot', async () => { - // A 'ready' record older than the revalidate interval takes the buildImage branch. - const stale = new Date(Date.now() - 14 * 24 * 60 * 60 * 1000).toISOString(); - const store = storeReturning(recordWith({ status: 'ready', status_reason: null, updated_at: stale })); - mockProvider.mockImplementation(() => ({ getImageBuildStatus: neverSettles, buildImage: neverSettles })); - - const pending = checkSnapshotStatus({ store, tenant_id: 'tenant', logger: silentLogger }); - await jest.advanceTimersByTimeAsync(SANDBOX_BUILD_REQUEST_TIMEOUT_MS + 1); - - expect((await pending)?.status).toBe('ready'); - }); - - it('still surfaces a real failure rather than hiding it behind the persisted status', async () => { - const store = storeReturning(recordWith({ status: 'pending' })); - const refused = new Error('Daytona rejected the API key'); - mockProvider.mockImplementation(() => ({ - getImageBuildStatus: () => Promise.reject(refused), - buildImage: neverSettles, - })); - - await expect(checkSnapshotStatus({ store, tenant_id: 'tenant', logger: silentLogger })).rejects.toBe(refused); - }); - - it('reports a status that arrives within the budget', async () => { - const store = storeReturning(recordWith({ status: 'pending' })); - store.updateSandboxStatus.mockResolvedValue(undefined); - mockProvider.mockImplementation(() => ({ - getImageBuildStatus: () => Promise.resolve({ status: 'ready', reason: null, metadata: BUILD_METADATA }), - buildImage: neverSettles, - })); - - const status = await checkSnapshotStatus({ store, tenant_id: 'tenant', logger: silentLogger }); - - expect(status?.status).toBe('ready'); - expect(store.updateSandboxStatus).toHaveBeenCalled(); - }); -}); From 3474dee259398e09d6eb2ba59495a46369cb6ab7 Mon Sep 17 00:00:00 2001 From: thesujai Date: Thu, 27 Aug 2026 14:27:56 +0530 Subject: [PATCH 3/3] minor refactor --- .changeset/daytona-request-timeouts.md | 6 +----- .../src/core/sandbox/provider/DaytonaProvider.ts | 10 ---------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/.changeset/daytona-request-timeouts.md b/.changeset/daytona-request-timeouts.md index 9f9cce74c..94c1a73c7 100644 --- a/.changeset/daytona-request-timeouts.md +++ b/.changeset/daytona-request-timeouts.md @@ -3,8 +3,4 @@ "@truefoundry/trueforge-core": patch --- -Bound the Daytona calls reachable from request-handling paths, so a stalled provider fails instead of hanging a request. - -The sandbox build-status refresh runs on the capability probe, the sandbox settings read, and turn creation, and none of those capped the round-trip. A Daytona endpoint that accepted the connection and then stalled held each request until undici's multi-minute default, piling up connections during a provider brownout. The refresh now gives up after a minute; the capability probe already treats a failure as sandbox-disabled. - -Snapshot registration also carries its own request timeout. Racing a promise against a timer frees the caller but leaves the request running, so the socket needed a deadline of its own. +Cap Daytona status-refresh calls at 1 minute so a stalled provider cannot hang request handlers. diff --git a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts index 783e0c6dc..1fba52e16 100644 --- a/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts +++ b/packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts @@ -28,15 +28,6 @@ const BUILD_STATE_INACTIVE = 'inactive'; const BUILD_STATE_ERROR = 'error'; const BUILD_STATE_BUILD_FAILED = 'build_failed'; -/** - * Cap on the snapshot registration request itself. - * - * `withTimeout` at a caller is a `Promise.race`: it frees the caller but leaves the request running, - * so the connection survives until undici's own default. Matches the longest caller budget, so the - * socket outlives no one still waiting on it. - */ -const SNAPSHOT_REGISTER_TIMEOUT_MS = 60_000; - const IMAGE_BUILD_NAME_PREFIX = 'trueforge-build-'; /** Same default the Daytona SDK applies when `DaytonaConfig.apiUrl` is omitted. */ const DEFAULT_DAYTONA_API_URL = 'https://app.daytona.io/api'; @@ -321,7 +312,6 @@ export class DaytonaSandboxProvider implements SandboxProvider { 'Content-Type': 'application/json', }, body: JSON.stringify({ name: this.buildRef, imageName: this.imageUri }), - signal: AbortSignal.timeout(SNAPSHOT_REGISTER_TIMEOUT_MS), }); } catch (error) { throw new Error('Daytona snapshot registration request failed.', { cause: error });