diff --git a/docs/de/platform/admin/sandboxes.md b/docs/de/platform/admin/sandboxes.md index 14b2224643..014749600d 100644 --- a/docs/de/platform/admin/sandboxes.md +++ b/docs/de/platform/admin/sandboxes.md @@ -51,7 +51,7 @@ Die Werte aktualisieren sich alle 15 Sekunden. Mit **Aktualisieren** forderst du ## Belegte und inaktive Workspaces unterscheiden -Inhaber und Admins können **Arbeitsbereiche** prüfen. Jede Zeile nennt den zugehörigen Agenten oder Workflow-Lauf, Laufzeitstatus, Belegungsstatus und laufende Aufgaben. Die Zustände beantworten unterschiedliche Fragen: Ein Container kann für die Wiederverwendung weiterlaufen, obwohl er seinen Organisationsplatz bereits freigegeben hat. Der Arbeitsbereich eines Projekt-Agenten bleibt auch im Leerlauf aufgeführt, als **Gestoppt** mit **Kontingent freigegeben**, bis du ihn löschst oder Tale ihn löscht: wenn ihn niemand [während der von deiner Organisation festgelegten Anzahl Tage](#delete-unused-workspaces-automatically) genutzt hat oder wenn [sein Agent, sein Projekt oder das zugehörige Mitglied entfernt wird](#explain-why-a-workspace-disappeared). Wurde der Agent selbst gelöscht, steht in der Zeile **Gelöschter Agent**, bis Tale den Arbeitsbereich gelöscht hat. Der Arbeitsbereich eines Workflow-Laufs wird kurz nach dem Ende des Laufs zurückgefordert. +Inhaber und Admins können **Arbeitsbereiche** prüfen. Jede Zeile nennt den zugehörigen Agenten oder Workflow-Lauf, Laufzeitstatus, Belegungsstatus und laufende Aufgaben. Die Zustände beantworten unterschiedliche Fragen: Ein Container kann für die Wiederverwendung weiterlaufen, obwohl er seinen Organisationsplatz bereits freigegeben hat. Der Arbeitsbereich eines Projekt-Agenten bleibt auch im Leerlauf aufgeführt, als **Gestoppt** mit **Kontingent freigegeben**, bis du ihn löschst oder Tale ihn löscht: wenn ihn niemand [während der von deiner Organisation festgelegten Anzahl Tage](#delete-unused-workspaces-automatically) genutzt hat oder wenn [sein Agent, sein Projekt oder das zugehörige Mitglied entfernt wird](#explain-why-a-workspace-disappeared). Wurde der Agent selbst gelöscht, steht in der Zeile **Gelöschter Agent**, bis Tale den Arbeitsbereich gelöscht hat. Der Arbeitsbereich eines Workflow-Laufs wird kurz nach dem Ende des Laufs zurückgefordert. Verschwindet eine Umgebung auf dem Tale-Server von selbst, etwa nach einem Neustart des Hosts oder weil ihr der Speicher ausgegangen ist, behält ihr Arbeitsbereich seine Dateien und steht auf **Gestoppt**; der nächste Lauf arbeitet darin weiter. Für eine Umgebung auf einem verbundenen [Gerät](/de/platform/admin/sandbox-devices) gilt das nicht. Die Ausgaben enthalten die gemessenen Kosten abgeschlossener Durchläufe. Ein noch laufender Durchlauf wird nach seinem Ende eingerechnet. Vorübergehende Crawler-Umgebungen zählen zur Kapazität, auch ohne eigene dauerhafte Workspace-Zeile. diff --git a/docs/en/platform/admin/sandboxes.md b/docs/en/platform/admin/sandboxes.md index 88b6ac18f8..72a50cb4f1 100644 --- a/docs/en/platform/admin/sandboxes.md +++ b/docs/en/platform/admin/sandboxes.md @@ -51,7 +51,7 @@ Measurements refresh every 15 seconds; **Refresh** requests a new observation. C ## Explain an allocated or idle workspace -Owners and Admins can inspect **Workspaces**. A row identifies its agent or workflow run, runtime state, allocation state and running tasks. These states answer different questions: a container may remain running for reuse after it has released its organization slot. A project agent’s workspace stays listed while the agent is idle, as **Stopped** with **Quota released**, until you destroy it or Tale deletes it: when nobody has used it for [the number of days your organization sets](#delete-unused-workspaces-automatically), or when [its agent, project or member is removed](#explain-why-a-workspace-disappeared). Once the agent itself is deleted, the row reads **Deleted agent** until Tale has deleted the workspace. A workflow run’s workspace is reclaimed shortly after the run ends. +Owners and Admins can inspect **Workspaces**. A row identifies its agent or workflow run, runtime state, allocation state and running tasks. These states answer different questions: a container may remain running for reuse after it has released its organization slot. A project agent’s workspace stays listed while the agent is idle, as **Stopped** with **Quota released**, until you destroy it or Tale deletes it: when nobody has used it for [the number of days your organization sets](#delete-unused-workspaces-automatically), or when [its agent, project or member is removed](#explain-why-a-workspace-disappeared). Once the agent itself is deleted, the row reads **Deleted agent** until Tale has deleted the workspace. A workflow run’s workspace is reclaimed shortly after the run ends. If an environment on the Tale server disappears on its own, for example after a host restart or because it ran out of memory, its workspace keeps its files and reads **Stopped**, and the next run continues in it. This does not apply to an environment on a connected [device](/platform/admin/sandbox-devices). **Spend** adds the metered cost of finished turns. A turn still running is included when it ends. Temporary crawler environments appear in capacity counts even without a standing workspace row. diff --git a/docs/fr/platform/admin/sandboxes.md b/docs/fr/platform/admin/sandboxes.md index 865928ed98..4324fd0d2e 100644 --- a/docs/fr/platform/admin/sandboxes.md +++ b/docs/fr/platform/admin/sandboxes.md @@ -51,7 +51,7 @@ Les mesures s’actualisent toutes les 15 secondes. **Actualiser** demande une n ## Distinguer allocation et environnement inactif -Les Propriétaires et Admins peuvent examiner les espaces de travail. Chaque ligne identifie l’agent ou l’exécution, l’état de l’environnement, celui de l’allocation et les tâches en cours. Ces états répondent à des questions différentes : un conteneur peut rester actif pour être réutilisé après avoir libéré sa place dans l’organisation. L’espace de travail d’un agent de projet reste affiché tant que l’agent est inactif, avec l’état **Arrêtée** et **Quota libéré**, jusqu’à ce que tu le supprimes ou que Tale le supprime : quand personne ne l’a utilisé pendant [le nombre de jours fixé par ton organisation](#delete-unused-workspaces-automatically), ou quand [son agent, son projet ou le membre concerné est retiré](#explain-why-a-workspace-disappeared). Si l’agent lui-même a été supprimé, la ligne indique **Agent supprimé** jusqu’à ce que Tale ait supprimé l’espace. L’espace d’une exécution de workflow est récupéré peu après la fin de l’exécution. +Les Propriétaires et Admins peuvent examiner les espaces de travail. Chaque ligne identifie l’agent ou l’exécution, l’état de l’environnement, celui de l’allocation et les tâches en cours. Ces états répondent à des questions différentes : un conteneur peut rester actif pour être réutilisé après avoir libéré sa place dans l’organisation. L’espace de travail d’un agent de projet reste affiché tant que l’agent est inactif, avec l’état **Arrêtée** et **Quota libéré**, jusqu’à ce que tu le supprimes ou que Tale le supprime : quand personne ne l’a utilisé pendant [le nombre de jours fixé par ton organisation](#delete-unused-workspaces-automatically), ou quand [son agent, son projet ou le membre concerné est retiré](#explain-why-a-workspace-disappeared). Si l’agent lui-même a été supprimé, la ligne indique **Agent supprimé** jusqu’à ce que Tale ait supprimé l’espace. L’espace d’une exécution de workflow est récupéré peu après la fin de l’exécution. Si un environnement sur le serveur Tale disparaît de lui-même, par exemple après un redémarrage de l’hôte ou faute de mémoire, son espace de travail garde ses fichiers et passe à l’état **Arrêtée**, et l’exécution suivante s’y poursuit. Cela ne vaut pas pour un environnement sur un [appareil](/fr/platform/admin/sandbox-devices) connecté. Les dépenses additionnent le coût mesuré des échanges terminés. Un échange en cours est ajouté lorsqu’il se termine. Les environnements temporaires du crawler comptent dans la capacité même sans ligne d’espace de travail permanent. diff --git a/services/platform/backend/core/node_only/sandbox/agent_session.test.ts b/services/platform/backend/core/node_only/sandbox/agent_session.test.ts index a531ed5628..ca6c5943bd 100644 --- a/services/platform/backend/core/node_only/sandbox/agent_session.test.ts +++ b/services/platform/backend/core/node_only/sandbox/agent_session.test.ts @@ -13,7 +13,7 @@ import { const runtime = vi.hoisted(() => ({ sessionCreate: vi.fn(), sessionAcquire: vi.fn(), - sessionDestroyIfIdle: vi.fn(), + sessionStopIfIdle: vi.fn(), })); vi.mock('./helpers/session_client', async (importOriginal) => ({ ...(await importOriginal()), @@ -71,9 +71,9 @@ function fixture( runtime.sessionCreate.mockImplementation(async () => { events.push('create'); }); - runtime.sessionDestroyIfIdle.mockImplementation(async () => { - events.push('destroy'); - return { destroyed: true, busy: false }; + runtime.sessionStopIfIdle.mockImplementation(async () => { + events.push('stop'); + return { stopped: true, busy: false, workspaceKept: true }; }); return { ctx, @@ -237,7 +237,7 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { ); // The standing workspace is the incarnation's state: a failed resume // releases the slot and never destroys it. - expect(runtime.sessionDestroyIfIdle).not.toHaveBeenCalled(); + expect(runtime.sessionStopIfIdle).not.toHaveBeenCalled(); }, ); @@ -284,7 +284,7 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { status: 'active', }); expect(runtime.sessionAcquire).toHaveBeenCalledTimes(adopt ? 1 : 0); - expect(runtime.sessionDestroyIfIdle).not.toHaveBeenCalled(); + expect(runtime.sessionStopIfIdle).not.toHaveBeenCalled(); }, ); @@ -294,7 +294,7 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { await expect(f.ensure()).rejects.toBe(f.mutationError); expect(runtime.sessionCreate).not.toHaveBeenCalled(); - expect(runtime.sessionDestroyIfIdle).not.toHaveBeenCalled(); + expect(runtime.sessionStopIfIdle).not.toHaveBeenCalled(); }); it('marks a failed fresh create as failed instead of leaving its quota occupied', async () => { @@ -310,15 +310,14 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { }); expect(f.events).toEqual([ 'reserveSessionSlotAndInsert', - 'destroy', + 'stop', 'setSessionStatus', ]); }); // A refused create (429: the host is full, or short of memory) made - // nothing — and a destroy of an id with no compute deletes the preserved - // workspace a stopped standing session keeps under it. - it('a create the sandbox host refused destroys nothing, and the row reads failed', async () => { + // nothing to stop. + it('a create the sandbox host refused stops nothing, and the row reads failed', async () => { const f = fixture(scenario, null); const error = new SpawnerBusyError(15_000); runtime.sessionCreate.mockImplementation(async () => { @@ -328,14 +327,14 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { await expect(f.ensure()).rejects.toBe(error); - expect(runtime.sessionDestroyIfIdle).not.toHaveBeenCalled(); + expect(runtime.sessionStopIfIdle).not.toHaveBeenCalled(); expect(f.events).toEqual([ 'reserveSessionSlotAndInsert', 'create', 'setSessionStatus', ]); // Settled as collected: the COLLECT pass would otherwise run the very - // destroy this skipped once the row's grace had passed. + // stop this skipped once the row's grace had passed. expect(f.ctx.runMutation).toHaveBeenLastCalledWith(expect.anything(), { rowId: 'row_1', status: 'failed', @@ -343,9 +342,11 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { }); }); - // The regression (#3494): a create the spawner had started, then failed — - // its container stayed in Docker state `created` with no owner to remove it. - it('destroys what a created-then-failed session left spawner-side before its row reads failed', async () => { + // A create the spawner had started, then failed, left its container in + // Docker state `created` with no owner to remove it. The stop releases that + // compute and keeps the workspace: the id may name one preserved for the + // owner's next turn (its last container lost to a reboot or the OOM killer). + it('stops what a created-then-failed session left spawner-side, keeping its workspace, before its row reads failed [SBX-R17]', async () => { const f = fixture(scenario, null); const error = new Error('runnerd did not become ready'); runtime.sessionCreate.mockImplementation(async () => { @@ -355,14 +356,14 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { await expect(f.ensure()).rejects.toBe(error); - expect(runtime.sessionDestroyIfIdle).toHaveBeenCalledTimes(1); - expect(runtime.sessionDestroyIfIdle).toHaveBeenCalledWith('session_1'); + expect(runtime.sessionStopIfIdle).toHaveBeenCalledTimes(1); + expect(runtime.sessionStopIfIdle).toHaveBeenCalledWith('session_1'); // While the row is still `creating` it holds the owner's slot, so no - // fresh create of the same id can start under the destroy. + // fresh create of the same id can start under the stop. expect(f.events).toEqual([ 'reserveSessionSlotAndInsert', 'create', - 'destroy', + 'stop', 'setSessionStatus', ]); expect(f.ctx.runMutation).toHaveBeenLastCalledWith(expect.anything(), { @@ -371,7 +372,7 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { }); }); - it('destroys an adopted orphan that turned out to be gone', async () => { + it('stops an adopted orphan that turned out to be gone', async () => { const f = fixture(scenario, null); runtime.sessionCreate.mockImplementation(async () => { f.events.push('create'); @@ -381,27 +382,27 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { await expect(f.ensure()).rejects.toBeInstanceOf(SessionNotFoundError); - expect(runtime.sessionDestroyIfIdle).toHaveBeenCalledWith('session_1'); + expect(runtime.sessionStopIfIdle).toHaveBeenCalledWith('session_1'); expect(f.events).toEqual([ 'reserveSessionSlotAndInsert', 'create', - 'destroy', + 'stop', 'setSessionStatus', ]); }); - it('a destroy that fails is logged, never thrown: the row still reads failed and the create error survives', async () => { + it('a stop that fails is logged, never thrown: the row still reads failed and the create error survives', async () => { const f = fixture(scenario, null); const error = new Error('container create failed'); runtime.sessionCreate.mockRejectedValue(error); - const destroyError = new Error('sandbox session destroy failed (502)'); - runtime.sessionDestroyIfIdle.mockRejectedValue(destroyError); + const stopError = new Error('sandbox session stop failed (502)'); + runtime.sessionStopIfIdle.mockRejectedValue(stopError); await expect(f.ensure()).rejects.toBe(error); expect(console.warn).toHaveBeenCalledWith( - expect.stringContaining('destroy after failed create of session_1'), - destroyError, + expect.stringContaining('stop after failed create of session_1'), + stopError, ); expect(f.ctx.runMutation).toHaveBeenLastCalledWith(expect.anything(), { rowId: 'row_1', @@ -416,14 +417,15 @@ describe.each(scenarios)('ensureAgentSession ($owner.type)', (scenario) => { const f = fixture(scenario, null); const error = new Error('runnerd did not become ready'); runtime.sessionCreate.mockRejectedValue(error); - runtime.sessionDestroyIfIdle.mockResolvedValue({ - destroyed: false, + runtime.sessionStopIfIdle.mockResolvedValue({ + stopped: false, busy: true, + workspaceKept: true, }); await expect(f.ensure()).rejects.toBe(error); - expect(runtime.sessionDestroyIfIdle).toHaveBeenCalledWith('session_1'); + expect(runtime.sessionStopIfIdle).toHaveBeenCalledWith('session_1'); expect(console.warn).toHaveBeenCalledWith( expect.stringContaining("runs a sibling turn's exec"), ); diff --git a/services/platform/backend/core/node_only/sandbox/agent_session.ts b/services/platform/backend/core/node_only/sandbox/agent_session.ts index e99b8b95c5..9e3ca4ee8d 100644 --- a/services/platform/backend/core/node_only/sandbox/agent_session.ts +++ b/services/platform/backend/core/node_only/sandbox/agent_session.ts @@ -12,7 +12,7 @@ import { SpawnerBusyError, sessionAcquire, sessionCreate, - sessionDestroyIfIdle, + sessionStopIfIdle, } from './helpers/session_client'; type SessionContext = Pick; @@ -61,10 +61,11 @@ function ownerPolicy( * /agent, including the harness conversation store, survives a stop on both * backends. A new row returns no previous stamp, even when adopting an orphan, * so a caller cannot resume a conversation from an unproven incarnation. - * A new row whose create fails first asks the spawner to destroy whatever it - * holds under the id unless a sibling turn is executing in it, then reads - * `failed`; the sandbox watchdog collects what that best-effort destroy could - * not. + * A new row whose create fails first asks the spawner to stop whatever it + * holds under the id unless a sibling turn is executing in it — compute + * only: the workspace stays, since the id may name one preserved for the + * owner's next turn — then reads `failed`; the sandbox watchdog collects + * what that best-effort stop could not. */ export async function ensureAgentSession( ctx: SessionContext, @@ -152,18 +153,22 @@ export async function ensureAgentSession( } catch (error) { // The spawner may already hold what this create made (one cut short // between Docker's create and start stays `created`), and a `failed` row - // is never reconciled, resumed or listed. Destroy while this row still + // is never reconciled, resumed or listed. Stop it while this row still // holds the owner's slot: once it reads `failed`, a fresh create of the - // same deterministic id may start, and a later destroy would hit that one. - // Only an idle session goes: a sibling turn of the same owner can resume - // this still-`creating` row and create or adopt the session itself, and - // its running exec must never die for this turn's failure. A container - // that never started runs no exec, so it is idle. A spawner that refused - // the create for want of room (429) made nothing to destroy — and a - // destroy of an id with no compute deletes its preserved workspace, which - // a stopped standing session's id still names. Its row is settled as - // collected, too: the watchdog's COLLECT pass would otherwise run that - // very destroy once the row's grace had passed. + // same deterministic id may start, and a later stop would hit that one. + // Only an idle session's compute goes: a sibling turn of the same owner + // can resume this still-`creating` row and create or adopt the session + // itself, and its running exec must never die for this turn's failure. A + // container that never started runs no exec, so it is idle. The workspace + // always stays: this row is new because the last one was settled, not + // because nothing is kept under the id — a container lost to a host + // reboot or the OOM killer leaves its workspace for the next turn, and a + // create that then fails (runnerd slow to start, an unreachable spawner) + // must not turn that loss into the loss of the files. Deleting what + // nothing owns is the workspace cleanup's. A spawner that refused the + // create for want of room (429) made nothing to stop, and its row is + // settled as collected: the watchdog's COLLECT pass would otherwise run + // that very stop once the row's grace had passed. if (error instanceof SpawnerBusyError) { await ctx.runMutation( internal.sandbox.session_mutations.setSessionStatus, @@ -171,17 +176,17 @@ export async function ensureAgentSession( ); throw error; } - await sessionDestroyIfIdle(sessionId) + await sessionStopIfIdle(sessionId) .then(({ busy }) => { if (busy) console.warn( `[sandbox.session] ${sessionId} runs a sibling turn's exec after this failed create; the watchdog collects it once idle`, ); }) - .catch((destroyError: unknown) => { + .catch((stopError: unknown) => { console.warn( - `[sandbox.session] destroy after failed create of ${sessionId} failed (the watchdog collects it):`, - destroyError, + `[sandbox.session] stop after failed create of ${sessionId} failed (the watchdog collects it):`, + stopError, ); }); await ctx.runMutation(internal.sandbox.session_mutations.setSessionStatus, { diff --git a/services/platform/backend/core/node_only/sandbox/helpers/session_client.test.ts b/services/platform/backend/core/node_only/sandbox/helpers/session_client.test.ts index 5c3209493c..92f5e8a91f 100644 --- a/services/platform/backend/core/node_only/sandbox/helpers/session_client.test.ts +++ b/services/platform/backend/core/node_only/sandbox/helpers/session_client.test.ts @@ -13,6 +13,7 @@ import { sandboxDeploymentLimits, sandboxDeviceDisconnect, sandboxDevices, + sandboxWorkspaceInventory, STAGE_BODY_BUDGET_BYTES, SpawnerUnreachableError, sessionAcquire, @@ -22,6 +23,7 @@ import { sessionIsAlive, sessionDestroyWorkspace, sessionReadFile, + sessionStopIfIdle, sessionStageFiles, type SessionStageFile, SpawnerBusyError, @@ -1338,6 +1340,92 @@ describe('sessionDestroyWorkspace', () => { }); }); +describe('sessionStopIfIdle', () => { + const calls: Array<{ url: string; method: string | undefined }> = []; + function answer(body: unknown, status = 200) { + calls.length = 0; + // oxlint-disable-next-line typescript-eslint/no-explicit-any + globalThis.fetch = (async (url: string, init?: RequestInit) => { + calls.push({ url, method: init?.method }); + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); + // oxlint-disable-next-line typescript-eslint/no-explicit-any + }) as any; + } + + test('asks for an idle-only stop that keeps the workspace [SBX-R17]', async () => { + answer({ stopped: true, busy: false, workspaceKept: true }); + expect(await sessionStopIfIdle('pa-1')).toEqual({ + stopped: true, + busy: false, + workspaceKept: true, + }); + expect(calls).toHaveLength(1); + expect(calls[0]?.method).toBe('DELETE'); + const url = new URL(calls[0]?.url ?? ''); + expect(url.pathname).toBe('/v1/sessions/pa-1'); + expect(url.search).toBe('?if_idle=1&keep_workspace=1'); + }); + + test('reads busy, and an older spawner that destroyed instead', async () => { + answer({ destroyed: false, busy: true }); + expect(await sessionStopIfIdle('pa-1')).toEqual({ + stopped: false, + busy: true, + workspaceKept: false, + }); + answer({ destroyed: true, busy: false, deletion: 'done' }); + expect(await sessionStopIfIdle('pa-1')).toEqual({ + stopped: true, + busy: false, + workspaceKept: false, + }); + }); + + test('throws on a failed stop, so the caller never reads it as done', async () => { + answer({ error: 'backend stop failed' }, 502); + await expect(sessionStopIfIdle('pa-1')).rejects.toThrow( + 'sandbox session stop failed (502)', + ); + }); +}); + +describe('sandboxWorkspaceInventory', () => { + // The reconcile reads the inventory under a session's lifecycle lock, so + // the caller's signal must end the read, not only the client's own bound. + test("gives up once the caller's signal aborts", async () => { + const signals: Array = []; + // oxlint-disable-next-line typescript-eslint/no-explicit-any + globalThis.fetch = (async (_url: string, init?: RequestInit) => { + signals.push(init?.signal); + return new Response( + JSON.stringify({ + backend: 'docker', + workspaces: [], + organizations: [], + }), + { status: 200, headers: { 'content-type': 'application/json' } }, + ); + // oxlint-disable-next-line typescript-eslint/no-explicit-any + }) as any; + const caller = new AbortController(); + + await expect( + sandboxWorkspaceInventory({ signal: caller.signal }), + ).resolves.toEqual({ + backend: 'docker', + workspaces: [], + organizations: [], + }); + + expect(signals[0]?.aborted).toBe(false); + caller.abort(); + expect(signals[0]?.aborted).toBe(true); + }); +}); + describe('sessions on connected devices', () => { const calls: Array<{ url: string; init: RequestInit | undefined }> = []; function answer(res: () => Response) { diff --git a/services/platform/backend/core/node_only/sandbox/helpers/session_client.ts b/services/platform/backend/core/node_only/sandbox/helpers/session_client.ts index a809d002c6..ba2b8bfe9b 100644 --- a/services/platform/backend/core/node_only/sandbox/helpers/session_client.ts +++ b/services/platform/backend/core/node_only/sandbox/helpers/session_client.ts @@ -734,6 +734,46 @@ export async function sessionDestroyIfIdle( return { destroyed: parsed.destroyed === true, busy: parsed.busy === true }; } +/** Conditional stop (`?if_idle=1&keep_workspace=1`): the spawner no-ops + * with {busy:true} while the session still has a live exec, exactly as + * {@link sessionDestroyIfIdle} does, and otherwise releases its compute and + * KEEPS its workspace — the cleanup after a failed create of an agent + * session, whose id may name a workspace preserved for its next turn (a + * container lost to a host reboot or the OOM killer leaves exactly that). + * Deleting a workspace nothing owns any more stays the workspace cleanup's + * job. `workspaceKept: false` is the answer of a spawner or device older + * than the flag, which destroyed instead. Same non-2xx THROW contract as + * sessionDestroy. */ +export async function sessionStopIfIdle( + sessionId: string, + options: { signal?: AbortSignal } = {}, +): Promise<{ stopped: boolean; busy: boolean; workspaceKept: boolean }> { + const path = `/v1/sessions/${encodeURIComponent(sessionId)}?if_idle=1&keep_workspace=1`; + const res = await spawnerFetch('DELETE', path, { + signal: AbortSignal.any([ + AbortSignal.timeout(30_000), + ...(options.signal ? [options.signal] : []), + ]), + }); + await throwIfDeviceOffline(res); + if (!res.ok) { + throw new Error(`sandbox session stop failed (${res.status})`); + } + const answer = z + .object({ + stopped: z.boolean().optional(), + destroyed: z.boolean().optional(), + busy: z.boolean().optional(), + workspaceKept: z.boolean().optional(), + }) + .parse(await res.json()); + return { + stopped: answer.stopped === true || answer.destroyed === true, + busy: answer.busy === true, + workspaceKept: answer.workspaceKept === true, + }; +} + /** How far deleting a destroyed workspace's bytes has come, as the spawner * answers a destroy: `done` (Docker: no trash entry of the id is left), * `pending` (still being deleted in the background), `failed` (the last @@ -815,10 +855,16 @@ export type SandboxWorkspaceInventory = z.infer< /** GET /v1/workspaces — every workspace the spawner holds (stopped sessions' * preserved data included) and the organizations holding resources beyond * them. `null` from a spawner that predates the route; THROWS when the - * spawner could not read its inventory. */ -export async function sandboxWorkspaceInventory(): Promise { + * spawner could not read its inventory, or once `signal` aborts (the read + * gives up after 60 s on its own). */ +export async function sandboxWorkspaceInventory( + options: { signal?: AbortSignal } = {}, +): Promise { const res = await spawnerFetch('GET', '/v1/workspaces', { - signal: AbortSignal.timeout(60_000), + signal: AbortSignal.any([ + AbortSignal.timeout(60_000), + ...(options.signal ? [options.signal] : []), + ]), }); if (res.status === 404) return null; if (!res.ok) { diff --git a/services/platform/backend/domains/sandbox/service.test.ts b/services/platform/backend/domains/sandbox/service.test.ts index 4296a25b1e..1f9bdf785d 100644 --- a/services/platform/backend/domains/sandbox/service.test.ts +++ b/services/platform/backend/domains/sandbox/service.test.ts @@ -118,6 +118,20 @@ function fakeSql(row: SessionRow | null, options: { lockFree?: boolean } = {}) { stored.status = 'active'; return [{ id: stored.id }]; } + if (text.startsWith("UPDATE app.sandbox_sessions SET status = 'stopped'")) { + // The heal of an agent session that keeps its workspace: by row id + // and org, only from a compute-holding status, only while unpinned. + if ( + stored === null || + !['creating', 'active', 'degraded'].includes(stored.status) || + stored.pinned || + !values.includes(stored.id) || + !values.includes(stored.organizationId) + ) + return []; + stored.status = 'stopped'; + return [{ id: stored.id }]; + } if (text.startsWith('UPDATE app.sandbox_sessions')) { if ( stored === null || @@ -1014,6 +1028,149 @@ describe('reconcileSession heals unpinned phantoms as before', () => { }); }); +describe('reconcileSession keeps an agent workspace whose sandbox disappeared [SBX-R17]', () => { + const held = (sessionId: string) => ({ + backend: 'docker' as const, + workspaces: [{ sessionId, touchedAtMs: 1, active: false, pinned: false }], + organizations: [], + }); + + it.each(['agent', 'agent-light'])( + 'settles a gone %s session whose workspace the spawner holds as stopped, reclaiming its credentials', + async (profile) => { + const { sql, dataSql, data, stored } = fakeSql({ + ...OWNED_SESSION, + profile, + }); + const inventory = vi.fn(async () => held('session-a')); + const { spawner } = fakeSpawner(false, { inventory }); + + await expect(reconcileSession(sql, ARGS, spawner)).resolves.toBe( + 'healed', + ); + + expect(inventory).toHaveBeenCalledOnce(); + expect(stored?.status).toBe('stopped'); + expect(spawner.create).not.toHaveBeenCalled(); + // Under the organization's admission lock, like every release. + expect(data.map((statement) => statement.text).slice(-2)).toEqual([ + expect.stringContaining('pg_advisory_xact_lock'), + expect.stringContaining("SET status = 'stopped'"), + ]); + expect(revokeSessionGatewayKeys).toHaveBeenCalledExactlyOnceWith( + dataSql, + { ...ARGS, rowId: 'row-a' }, + ); + expect(wakeParkedAgentRuns).toHaveBeenCalledExactlyOnceWith( + dataSql, + 'org-a', + ); + }, + ); + + it('keeps the workspace when the inventory cannot be read', async () => { + for (const inventory of [ + vi.fn(async () => { + throw new Error('sandbox workspace inventory unavailable (503)'); + }), + vi.fn(async () => null), + ]) { + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const { sql, stored } = fakeSql(OWNED_SESSION); + const { spawner } = fakeSpawner(false, { inventory }); + + await expect(reconcileSession(sql, ARGS, spawner)).resolves.toBe( + 'healed', + ); + + expect(stored?.status).toBe('stopped'); + } + }); + + it('settles as destroyed when the spawner holds no workspace under the id', async () => { + const { sql, stored } = fakeSql(OWNED_SESSION); + const { spawner } = fakeSpawner(false, { + inventory: vi.fn(async () => held('another-session')), + }); + + await expect(reconcileSession(sql, ARGS, spawner)).resolves.toBe('healed'); + + expect(stored?.status).toBe('destroyed'); + }); + + it('settles a gone render session as destroyed without asking for the inventory', async () => { + const { sql, stored } = fakeSql({ + ...OWNED_SESSION, + profile: 'default', + ownerType: 'render', + }); + const inventory = vi.fn(async () => held('session-a')); + const { spawner } = fakeSpawner(false, { inventory }); + + await expect(reconcileSession(sql, ARGS, spawner)).resolves.toBe('healed'); + + expect(stored?.status).toBe('destroyed'); + expect(inventory).not.toHaveBeenCalled(); + }); + + // The inventory is read under the session's lifecycle lock: the read is + // bounded on its own, and stops with the pass that asked for it. + it('bounds the inventory read, which a pass out of time ends without settling the row', async () => { + const pass = new AbortController(); + const { sql, stored } = fakeSql(OWNED_SESSION); + const inventory = vi.fn(async (options?: { signal?: AbortSignal }) => { + expect(options?.signal?.aborted).toBe(false); + pass.abort(); + options?.signal?.throwIfAborted(); + return held('session-a'); + }); + const { spawner } = fakeSpawner(false, { inventory }); + + await expect( + reconcileSession(sql, ARGS, spawner, { signal: pass.signal }), + ).rejects.toThrow(); + + expect(inventory).toHaveBeenCalledOnce(); + expect(stored?.status).toBe('active'); + expect(revokeSessionGatewayKeys).not.toHaveBeenCalled(); + expect(wakeParkedAgentRuns).not.toHaveBeenCalled(); + }); + + it('reads an inventory that times out on its own as unknown, keeping the workspace', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => undefined); + const pass = new AbortController(); + const { sql, stored } = fakeSql(OWNED_SESSION); + const inventory = vi.fn(async (options?: { signal?: AbortSignal }) => { + expect(options?.signal).toBeInstanceOf(AbortSignal); + throw new DOMException('The operation timed out.', 'TimeoutError'); + }); + const { spawner } = fakeSpawner(false, { inventory }); + + await expect( + reconcileSession(sql, ARGS, spawner, { signal: pass.signal }), + ).resolves.toBe('healed'); + + expect(stored?.status).toBe('stopped'); + }); + + it('leaves a row pinned meanwhile to its next visit, settling and revoking nothing', async () => { + const { sql, stored } = fakeSql(OWNED_SESSION); + const { spawner } = fakeSpawner(false, { + inventory: vi.fn(async () => { + // A pin taken meanwhile keeps the row from being hibernated. + if (stored !== null) stored.pinned = true; + return held('session-a'); + }), + }); + + await expect(reconcileSession(sql, ARGS, spawner)).resolves.toBe('skipped'); + + expect(stored?.status).toBe('active'); + expect(revokeSessionGatewayKeys).not.toHaveBeenCalled(); + expect(wakeParkedAgentRuns).not.toHaveBeenCalled(); + }); +}); + describe('every lifecycle transition takes the session lock first', () => { // `order` interleaves the three connections and the spawner: nothing but // the reconcile's unlocked first look may run before the advisory lock, diff --git a/services/platform/backend/domains/sandbox/service.ts b/services/platform/backend/domains/sandbox/service.ts index fed4a10df2..28b3e20e97 100644 --- a/services/platform/backend/domains/sandbox/service.ts +++ b/services/platform/backend/domains/sandbox/service.ts @@ -2,11 +2,13 @@ import postgres, { type Sql, type TransactionSql } from 'postgres'; import { SessionDuplicateError, + sandboxWorkspaceInventory, sessionCreate, sessionDestroy, sessionIsAlive, sessionObserve, sessionSetPinned, + type SandboxWorkspaceInventory, type SessionCreateBody, } from '../../core/node_only/sandbox/helpers/session_client.ts'; import { addJobInTx } from '../../jobs/enqueue.ts'; @@ -15,6 +17,7 @@ import { getSessionBySessionId, markRecreatedSessionActive, markSessionDestroyed, + markSessionStopped, setSessionPinned, type SessionRow, } from './sessions.ts'; @@ -308,6 +311,13 @@ export interface ReconcileSpawner { /** POST /v1/sessions — under an id whose workspace the spawner preserved, * the create re-attaches that workspace (a resume, not a fresh one). */ create: (body: SessionCreateBody) => Promise; + /** GET /v1/workspaces — every workspace the spawner holds, stopped + * sessions' preserved data included; `null` from a spawner without the + * route. Where absent, an agent session whose compute is gone heals as + * destroyed, as a render's always does. */ + inventory?: (options?: { + signal?: AbortSignal; + }) => Promise; } const DEFAULT_RECONCILE_SPAWNER: ReconcileSpawner = { @@ -315,6 +325,7 @@ const DEFAULT_RECONCILE_SPAWNER: ReconcileSpawner = { observe: (sessionId, signal) => sessionObserve(sessionId, signal), setPinned: sessionSetPinned, create: sessionCreate, + inventory: sandboxWorkspaceInventory, }; /** Queues the recreate of one pinned session gone spawner-side. */ @@ -368,7 +379,8 @@ export type ReconcileOutcome = /** Alive spawner-side and unpinned: nothing to do. */ | 'live' /** Gone spawner-side and unpinned (or a pinned render): the row settled - * as destroyed. */ + * — as stopped for an agent session whose workspace the spawner still + * holds, as destroyed otherwise. */ | 'healed' /** Alive and pinned: the pin re-asserted spawner-side. Also a gone pinned * session whose recreate the spawner answered as a duplicate — back @@ -400,7 +412,17 @@ export type ReconcileOutcome = * spawner answers 404. Retained unpinned stopped/expired rows may receive * metadata-only repair of a proven stale runtime pin. * - Unpinned compute-holding rows: a container gone spawner-side settles - * the row as destroyed (phantom heal); `healed` counts only such a row. + * the row (phantom heal); `healed` counts only such a row. An agent + * session (`agent`, `agent-light`: a project agent's or an automation + * run's) whose workspace the spawner still holds — a container lost to a + * host reboot, a daemon restart, the OOM killer or the spawner's own TTL — + * settles as `stopped`: hibernated, its slot freed and its credentials + * reclaimed, and resumed in place by the next turn, which keeps the + * incarnation and so the harness conversation. Settled as destroyed, the + * next turn would open a fresh incarnation beside the kept files. An + * inventory that cannot be read keeps the workspace too; one that lists no + * workspace under the id, and a render session (disposable), settle as + * destroyed. * - Pinned: the row is the durable truth. Pin-change jobs deliver the latest * intent; reconciliation restores it after lost compute (a new create * always starts unpinned). A live session has the pin @@ -548,6 +570,14 @@ async function reconcileLocked( if (alive) return 'live'; const body = row.pinned ? recreateBody(row) : null; if (body === null) { + if (!row.pinned && (await keepsWorkspace(spawner, row, mode.signal))) { + return (await markSessionStopped(sessionSql, { + ...args, + rowId: row.id, + })) + ? 'healed' + : 'skipped'; + } return (await markSessionDestroyed(sessionSql, args)) ? 'healed' : 'skipped'; @@ -580,6 +610,49 @@ async function reconcileLocked( return created ? 'recreated' : 'repinned'; } +/** How long the reconcile waits for the spawner's workspace inventory. It + * is read under the session's lifecycle lock, so it gets the bound of the + * reconcile's other spawner reads rather than the cleanup's minute; an + * inventory slower than that is one that cannot be read. */ +const RECONCILE_INVENTORY_TIMEOUT_MS = 15_000; + +/** Does the spawner still hold the workspace of an agent session whose + * compute is gone? A render session's workspace is disposable, so never. An + * inventory that cannot be read answers yes: settling the row as stopped + * frees its slot all the same, and a resume that finds no conversation + * falls back to a fresh one, while settling it as destroyed would leave the + * files to a fresh incarnation. The read stops with the caller's `signal`, + * and then THROWS: a pass that ran out of time settles nothing on a guess, + * and leaves the row to its next visit. */ +async function keepsWorkspace( + spawner: ReconcileSpawner, + row: SessionRow, + signal: AbortSignal | undefined, +): Promise { + if (row.profile !== 'agent' && row.profile !== 'agent-light') return false; + if (spawner.inventory === undefined) return false; + let inventory: SandboxWorkspaceInventory | null; + try { + inventory = await spawner.inventory({ + signal: AbortSignal.any([ + AbortSignal.timeout(RECONCILE_INVENTORY_TIMEOUT_MS), + ...(signal ? [signal] : []), + ]), + }); + } catch (error) { + signal?.throwIfAborted(); + console.warn( + `[sandbox] workspace inventory unavailable while healing ${row.sessionId}; its workspace is kept:`, + error, + ); + return true; + } + if (inventory === null) return true; + return inventory.workspaces.some( + (workspace) => workspace.sessionId === row.sessionId, + ); +} + async function requirePin( spawner: ReconcileSpawner, sessionId: string, diff --git a/services/platform/backend/domains/sandbox/sessions.ts b/services/platform/backend/domains/sandbox/sessions.ts index 4eb8d34a20..08a6f6c8ae 100644 --- a/services/platform/backend/domains/sandbox/sessions.ts +++ b/services/platform/backend/domains/sandbox/sessions.ts @@ -483,6 +483,50 @@ export async function resumeSessionSlot( }); } +/** + * Hibernate ONE incarnation whose compute is gone while the spawner keeps its + * workspace — the reconcile's heal of an agent session after a host reboot, + * a daemon restart or the OOM killer took its container. The row reads + * `stopped` (its slot freed, its `createdAt` kept), so the next turn resumes + * it in place on the preserved files and conversation, exactly as after an + * idle release. By row id, only from a compute-holding status and only while + * unpinned, under the organization's admission lock like every release (a + * resume taken first keeps the row). The credentials go as at every + * teardown edge — the gateway keys settled and deleted, the session tokens + * revoked: the compute that held them is gone, and the next turn mints its + * own. A freed slot is a release edge, so parked runs are woken. + */ +export async function markSessionStopped( + sql: Sql, + args: { organizationId: string; sessionId: string; rowId: string }, +): Promise { + const stopped = await sql.begin(async (tx) => { + await lockOrgAdmission(tx, args.organizationId); + const rows = await tx<{ id: string }[]>` + UPDATE app.sandbox_sessions SET status = 'stopped' + WHERE id = ${args.rowId} AND org_id = ${args.organizationId} + AND session_id = ${args.sessionId} + AND status IN ('creating', 'active', 'degraded') + AND pinned = false + RETURNING id + `; + return rows.length > 0; + }); + if (!stopped) return false; + await revokeSessionGatewayKeys(sql, args).catch((error: unknown) => { + console.error( + `[sandbox] gateway key reclaim for stopped ${args.sessionId} failed:`, + error, + ); + }); + await wakeParkedAgentRuns(sql, args.organizationId).catch( + (error: unknown) => { + console.warn('[sandbox] capacity wake failed:', error); + }, + ); + return true; +} + /** Terminal: revoke the gateway keys + mark destroyed + revoke tokens. The * single bottom of EVERY destroy — the admin Destroy, the watchdog's * phantom heal and ended-run reclaim, the session teardown — so credential diff --git a/services/platform/backend/domains/sandbox/spec.md b/services/platform/backend/domains/sandbox/spec.md index 178f2811da..9017b5da15 100644 --- a/services/platform/backend/domains/sandbox/spec.md +++ b/services/platform/backend/domains/sandbox/spec.md @@ -4,9 +4,10 @@ A sandbox is the isolated environment an agent works in, and its workspace is the files it keeps between turns. These rules cover who can see and manage workspaces, what an agent in a -sandbox can reach, how many sandboxes run at once, when a workspace is deleted, and how a -turn's spend is booked. Connected devices and agent secrets have their own specs. Egress, the -health checks, image generation and how waiting work resumes are not covered; see Not yet. +sandbox can reach, how many sandboxes run at once, what is left of a workspace whose sandbox +disappears, when a workspace is deleted, and how a turn's spend is booked. Connected devices +and agent secrets have their own specs. Egress, the health checks, image generation and how +waiting work resumes are not covered; see Not yet. ## Who can do what @@ -151,6 +152,23 @@ every attempt has failed, its row says so. - **Example**: Ada destroys a pinned workspace while the sandbox service is down → the workspace stays in the list, now unpinned, and the Destroy is tried again. +## When a sandbox disappears + +A sandbox can end without a Destroy, for example when its host restarts or it runs out of +memory. A pinned workspace gets a new sandbox (`SBX-R10`); this covers the others. + +### SBX-R17 · A workspace outlives the sandbox it ran in + +When the sandbox of a project agent or an automation run on the Tale server disappears +without a Destroy, for example because the host restarted or the sandbox ran out of memory, its +workspace is kept and reads as stopped. The next turn goes on in its files. A start that fails +after that removes only the sandbox it began, never the files. A crawler's temporary sandbox is +closed instead, and so is a workspace the sandbox service no longer holds, or one on a +connected device, which it does not list. + +- **Example**: Ada's agent is working on a task when the host restarts → the workspace reads + **Stopped**, and the agent's next turn finds its files and goes on with its conversation. + ## Automatic deletion Tale deletes a workspace on its own in two cases: nobody has used it for a while, or what it @@ -231,7 +249,7 @@ request is refused the same way (`budget_exceeded`) and holds nothing either. the place in line a waiting start gets, giving a slot back when a turn ends, and waking the waiting runs (`sessions.ts`, `idle-release.ts`, `core/node_only/sandbox/capacity_refusal.ts`). - **Health checks and repair**: ending a session after its lifetime while sparing a turn that - is still working, closing a workspace whose sandbox disappeared, collecting failed starts, + is still working, closing a crawler's sandbox that disappeared, collecting failed starts, reclaiming the sandboxes of ended runs and crawls, and picking a turn up again after a restart (`watchdogs.ts`, `recovery.ts`, `service.ts`, `wait-retention.ts`, `retirement-schedule.ts`). diff --git a/services/platform/backend/domains/sandbox/wait-retention.test.ts b/services/platform/backend/domains/sandbox/wait-retention.test.ts new file mode 100644 index 0000000000..2156321278 --- /dev/null +++ b/services/platform/backend/domains/sandbox/wait-retention.test.ts @@ -0,0 +1,86 @@ +// @vitest-environment node + +/** + * The sweep of what waiting for sandbox room leaves behind, against scripted + * SQL: the statement shapes and the batch loop. The rows each statement + * deletes on a real database are the integration lane's + * (`checkWatchdogs` in `backend/integration-check.ts`). + */ + +import type { Sql } from 'postgres'; +import { describe, expect, it } from 'vitest'; + +import { sweepRoomWaitLeftovers } from './wait-retention.ts'; + +interface Statement { + text: string; + values: unknown[]; +} + +/** Scripted `sql`: each op-row and session-row DELETE pops its next answer; + * every statement is recorded. */ +function fakeSql(script: { ops?: number[]; sessions?: number[] }): { + sql: Sql; + statements: Statement[]; +} { + const statements: Statement[] = []; + const rows = (count: number | undefined) => + Array.from({ length: count ?? 0 }, (_, index) => ({ id: `row-${index}` })); + const fn = (strings: TemplateStringsArray, ...values: unknown[]) => { + const text = strings.join('?'); + statements.push({ text, values }); + if (text.includes('DELETE FROM app.sandbox_session_ops')) { + return Promise.resolve(rows(script.ops?.shift())); + } + if (text.includes('DELETE FROM app.sandbox_sessions')) { + return Promise.resolve(rows(script.sessions?.shift())); + } + return Promise.resolve([]); + }; + // oxlint-disable-next-line typescript/no-unsafe-type-assertion -- scripted postgres tag + return { sql: fn as unknown as Sql, statements }; +} + +const sessionDeletes = (statements: Statement[]) => + statements.filter((statement) => + statement.text.includes('DELETE FROM app.sandbox_sessions'), + ); + +describe('sweepRoomWaitLeftovers', () => { + it('deletes a collected failed row a day after its collection', async () => { + const now = 10_000_000_000; + const { sql, statements } = fakeSql({}); + + await sweepRoomWaitLeftovers(sql, { now }); + + const [statement] = sessionDeletes(statements); + expect(statement?.text).toContain("s.status = 'failed'"); + expect(statement?.text).toContain('s.destroyed_at_ms IS NOT NULL'); + expect(statement?.values).toContain(now - 24 * 60 * 60 * 1000); + }); + + // Collecting a failed create of an agent session keeps its workspace, and + // the cleanup finds a project agent's workspace through its rows: the + // newest row naming the id is what the unused rule and the agent and + // member removals reach it by. + it("keeps a project agent's newest row of its session id, deleting one only behind a newer row", async () => { + const { sql, statements } = fakeSql({}); + + await sweepRoomWaitLeftovers(sql, { now: 10_000_000_000 }); + + const text = sessionDeletes(statements)[0]?.text.replace(/\s+/g, ' '); + expect(text).toContain( + "AND ( s.owner_type <> 'project_agent' OR EXISTS ( SELECT 1 FROM app.sandbox_sessions newer WHERE newer.session_id = s.session_id AND (newer.created_at_ms, newer.id) > (s.created_at_ms, s.id) ) )", + ); + }); + + it('deletes batch after batch until one comes back short, and counts the rows', async () => { + const { sql, statements } = fakeSql({ ops: [2, 1], sessions: [2, 2, 0] }); + + await expect( + sweepRoomWaitLeftovers(sql, { now: 10_000_000_000, batch: 2 }), + ).resolves.toEqual({ ops: 3, sessions: 4 }); + + expect(sessionDeletes(statements)).toHaveLength(3); + }); +}); diff --git a/services/platform/backend/domains/sandbox/wait-retention.ts b/services/platform/backend/domains/sandbox/wait-retention.ts index 9ecf671278..d5a67a046b 100644 --- a/services/platform/backend/domains/sandbox/wait-retention.ts +++ b/services/platform/backend/domains/sandbox/wait-retention.ts @@ -18,12 +18,22 @@ import type { Sql } from 'postgres'; * and every other reader (the latest incarnation of an id, a recent op's * attribution) looks at newer rows or at rows still holding compute. A * start at the front of the sandbox host's line asks every few seconds, - * so a long wait leaves hundreds of them an hour. + * so a long wait leaves hundreds of them an hour. The newest row of a + * project agent's session id stays, though: collecting a failed create of + * an agent session stops its compute and keeps its workspace, and the + * workspace cleanup finds a project agent's workspace through its rows, + * to delete it once unused or with its agent or member. One that no row + * names is kept for as long as its agent exists (a restored database may + * have lost the row its next run resumes), so deleting the last row would + * keep the workspace for good. The row goes once a newer one names the + * id. An automation run's rows go regardless: the cleanup deletes a run's + * workspace that no row names. */ /** How long a refused start's op row stays after it ended. */ const AWAITING_ROOM_OP_RETENTION_MS = 60 * 60 * 1000; -/** How long a failed session row stays after it was collected. */ +/** How long a failed session row stays after it was collected — a project + * agent's newest row of its id stays until a newer one follows. */ const COLLECTED_SESSION_RETENTION_MS = 24 * 60 * 60 * 1000; /** Rows deleted per statement, and statements per table per sweep — a @@ -79,11 +89,19 @@ export async function sweepRoomWaitLeftovers( const deleted = await sql<{ id: string }[]>` DELETE FROM app.sandbox_sessions WHERE id IN ( - SELECT id FROM app.sandbox_sessions - WHERE status = 'failed' - AND destroyed_at_ms IS NOT NULL - AND destroyed_at_ms < ${sessionCutoff} - ORDER BY destroyed_at_ms + SELECT s.id FROM app.sandbox_sessions s + WHERE s.status = 'failed' + AND s.destroyed_at_ms IS NOT NULL + AND s.destroyed_at_ms < ${sessionCutoff} + AND ( + s.owner_type <> 'project_agent' + OR EXISTS ( + SELECT 1 FROM app.sandbox_sessions newer + WHERE newer.session_id = s.session_id + AND (newer.created_at_ms, newer.id) > (s.created_at_ms, s.id) + ) + ) + ORDER BY s.destroyed_at_ms LIMIT ${batch} ) RETURNING id diff --git a/services/platform/backend/domains/sandbox/watchdogs.test.ts b/services/platform/backend/domains/sandbox/watchdogs.test.ts index 2bbe1442a1..1d954fb3a0 100644 --- a/services/platform/backend/domains/sandbox/watchdogs.test.ts +++ b/services/platform/backend/domains/sandbox/watchdogs.test.ts @@ -11,9 +11,10 @@ * once the job's signal aborts (stamping only the rows it visited), the * ended-run reclaim settles a row * only when the spawner confirmed the session is gone or idle (busy and - * errors leave it for the next tick), and the failed-create collect destroys - * a session only when no newer or live incarnation carries its id, then - * stamps that one row, and the render release destroys the idle session of + * errors leave it for the next tick), and the failed-create collect removes + * a session only when no newer or live incarnation carries its id — an agent + * session's compute alone, keeping its workspace — then stamps that one row, + * and the render release destroys the idle session of * a render row older than a scan link can keep one. Every tick also deletes the settled model-endpoint * request rows a week old, between closing the lost requests and the * settlement sweep, and a failure there stops neither. The real-Postgres @@ -48,11 +49,13 @@ import { } from './watchdogs.ts'; vi.mock('../../core/node_only/sandbox/helpers/session_client.ts', () => ({ + sandboxWorkspaceInventory: vi.fn(), sessionCreate: vi.fn(), sessionIsAlive: vi.fn(), sessionObserve: vi.fn(), sessionDestroyIfIdle: vi.fn(), sessionSetPinned: vi.fn(), + sessionStopIfIdle: vi.fn(), })); vi.mock('../tasks/agent-runs.ts', () => ({ wakeParkedAgentRuns: vi.fn(() => Promise.resolve()), @@ -76,6 +79,7 @@ interface Candidate { id: string; sessionId: string; orgId: string; + ownerType?: string; } /** @@ -148,8 +152,9 @@ function candidate(id: string): Candidate { return { id, sessionId: `ses-${id}`, orgId: 'org_1' }; } -/** A scripted spawner: every session alive, every pin taken, every create - * and idle destroy served — a test overrides the verbs it scripts. */ +/** A scripted spawner: every session alive, every pin taken, every create, + * idle destroy and idle stop served — a test overrides the verbs it + * scripts. */ function scriptedSpawner( overrides: Partial = {}, ): WatchdogSpawner { @@ -160,6 +165,7 @@ function scriptedSpawner( destroyIfIdle: vi.fn(() => Promise.resolve({ destroyed: true, busy: false }), ), + stopIfIdle: vi.fn(() => Promise.resolve({ stopped: true, busy: false })), ...overrides, }; } @@ -374,6 +380,74 @@ describe('runSandboxWatchdog — fair reconcile', () => { }); }); +describe('runSandboxWatchdog — the workspace inventory in the reconcile', () => { + // A host reboot leaves every session's compute gone at once, and each + // agent session's heal asks whether its workspace is still held. + it('reads the inventory at most once per pass, however many rows heal [SBX-R17]', async () => { + const { sql } = fakeSql({ + reconcile: [[candidate('gone-a'), candidate('gone-b'), candidate('c')]], + }); + const inventory = vi.fn(() => + Promise.resolve({ + backend: 'docker' as const, + workspaces: [], + organizations: [], + }), + ); + vi.mocked(reconcileSession).mockImplementation( + async (_sql, args, spawner) => { + if (args.sessionId === 'ses-c') return 'live'; + await spawner?.inventory?.(); + return 'healed'; + }, + ); + + const result = await runSandboxWatchdog(sql, { + spawner: scriptedSpawner({ inventory }), + }); + + expect(result.healed).toBe(2); + expect(inventory).toHaveBeenCalledOnce(); + }); + + it("bounds the shared read by the pass's signal", async () => { + const { sql } = fakeSql({ reconcile: [[candidate('gone-a')]] }); + const inventory = vi.fn((_options?: { signal?: AbortSignal }) => + Promise.resolve(null), + ); + const passSignal = new AbortController().signal; + vi.mocked(reconcileSession).mockImplementation( + async (_sql, _args, spawner) => { + await spawner?.inventory?.({ signal: passSignal }); + return 'healed'; + }, + ); + + await runSandboxWatchdog(sql, { + spawner: scriptedSpawner({ inventory }), + }); + + expect(inventory).toHaveBeenCalledExactlyOnceWith({ signal: passSignal }); + }); + + it('reads it again on the next pass', async () => { + const inventory = vi.fn(() => Promise.resolve(null)); + vi.mocked(reconcileSession).mockImplementation( + async (_sql, _args, spawner) => { + await spawner?.inventory?.(); + return 'healed'; + }, + ); + for (let pass = 0; pass < 2; pass += 1) { + const { sql } = fakeSql({ reconcile: [[candidate(`gone-${pass}`)]] }); + await runSandboxWatchdog(sql, { + spawner: scriptedSpawner({ inventory }), + }); + } + expect(inventory).toHaveBeenCalledTimes(2); + }); +}); + describe('runSandboxWatchdog — pinned sessions in the reconcile', () => { it('counts a pinned session whose recreate it queued apart from a healed phantom', async () => { const { sql } = fakeSql({ @@ -670,7 +744,7 @@ describe('runSandboxWatchdog — collect of failed creates', () => { expect(destroyIfIdle).toHaveBeenCalledTimes(2); expect(collectStampsOf(statements)).toHaveLength(0); expect(warn).toHaveBeenCalledWith( - expect.stringContaining('failed-session destroy failed'), + expect.stringContaining('failed-session collect failed'), expect.any(Error), ); expect(stampsOf(statements)[0]?.values).toContainEqual([ @@ -679,6 +753,59 @@ describe('runSandboxWatchdog — collect of failed creates', () => { ]); }); + // An agent session's id may name a workspace preserved for the owner's + // next turn — its last container lost to a host reboot or the OOM killer + // — so its leftover loses its compute alone. A render's is disposable. + it('stops an agent session keeping its workspace, and destroys a render session whole [SBX-R17]', async () => { + const project = { + ...candidate('failed-project'), + ownerType: 'project_agent', + }; + const run = { ...candidate('failed-run'), ownerType: 'workflow_run' }; + const render = { ...candidate('failed-render'), ownerType: 'render' }; + const { sql, statements } = fakeSql({ + collect: [[project, run, render]], + }); + const spawner = scriptedSpawner(); + + const result = await runSandboxWatchdog(sql, { spawner }); + + expect(result.collected).toBe(3); + expect(vi.mocked(spawner.stopIfIdle).mock.calls.map(([id]) => id)).toEqual([ + project.sessionId, + run.sessionId, + ]); + expect( + vi.mocked(spawner.destroyIfIdle).mock.calls.map(([id]) => id), + ).toEqual([render.sessionId]); + expect(collectStampsOf(statements).map((s) => s.values[1])).toEqual([ + project.id, + run.id, + render.id, + ]); + const select = statements.find((s) => + s.text.includes("WHERE status = 'failed' AND destroyed_at_ms IS NULL"), + ); + expect(select?.text).toContain('owner_type AS "ownerType"'); + }); + + it('leaves an agent session a turn is executing in for a later tick', async () => { + const busy = { + ...candidate('failed-busy-agent'), + ownerType: 'project_agent', + }; + const { sql, statements } = fakeSql({ collect: [[busy]] }); + const spawner = scriptedSpawner({ + stopIfIdle: vi.fn(() => Promise.resolve({ stopped: false, busy: true })), + }); + + const result = await runSandboxWatchdog(sql, { spawner }); + + expect(result.collected).toBe(0); + expect(spawner.destroyIfIdle).not.toHaveBeenCalled(); + expect(collectStampsOf(statements)).toHaveLength(0); + }); + it('targets only unstamped failed rows past the grace, in the fair order', async () => { const { sql, statements } = fakeSql({}); const before = Date.now(); diff --git a/services/platform/backend/domains/sandbox/watchdogs.ts b/services/platform/backend/domains/sandbox/watchdogs.ts index 402629711e..84357868ac 100644 --- a/services/platform/backend/domains/sandbox/watchdogs.ts +++ b/services/platform/backend/domains/sandbox/watchdogs.ts @@ -1,11 +1,14 @@ import type { Sql, TransactionSql } from 'postgres'; import { + sandboxWorkspaceInventory, sessionCreate, sessionDestroyIfIdle, sessionIsAlive, sessionObserve, sessionSetPinned, + sessionStopIfIdle, + type SandboxWorkspaceInventory, } from '../../core/node_only/sandbox/helpers/session_client.ts'; import { SANDBOX_SESSION_LIVE_STATUSES } from '../../core/sandbox/session_constants.ts'; import { closeStaleModelApiOps } from '../model_api/metering.ts'; @@ -33,6 +36,12 @@ export interface WatchdogSpawner extends ReconcileSpawner { sessionId: string, options?: { signal?: AbortSignal }, ) => Promise<{ destroyed: boolean; busy: boolean }>; + /** DELETE /v1/sessions/:id?if_idle=1&keep_workspace=1 — compute only: + * the workspace stays for the owner's next turn. */ + stopIfIdle: ( + sessionId: string, + options?: { signal?: AbortSignal }, + ) => Promise<{ stopped: boolean; busy: boolean }>; } const DEFAULT_SPAWNER: WatchdogSpawner = { @@ -40,7 +49,9 @@ const DEFAULT_SPAWNER: WatchdogSpawner = { observe: (sessionId, signal) => sessionObserve(sessionId, signal), setPinned: sessionSetPinned, create: sessionCreate, + inventory: sandboxWorkspaceInventory, destroyIfIdle: sessionDestroyIfIdle, + stopIfIdle: sessionStopIfIdle, }; /** @@ -54,10 +65,10 @@ export const SANDBOX_RUN_SESSION_RECLAIM_GRACE_MS = 10 * 60_000; /** * How long after a fresh create failed its row becomes collectable. The - * failing turn already asked the spawner to destroy what the create left, so - * this pass is the backstop for a destroy that could not run or failed. Two + * failing turn already asked the spawner to remove what the create left, so + * this pass is the backstop for a removal that could not run or failed. Two * sweep ticks, so a turn that took the row over before it read `failed` has - * started its exec by then — and the `if_idle` destroy leaves a busy session + * started its exec by then — and the `if_idle` removal leaves a busy session * alone. */ export const SANDBOX_FAILED_SESSION_COLLECT_GRACE_MS = 10 * 60_000; @@ -135,8 +146,10 @@ export interface SandboxWatchdogResult { * while the container still works. An op silent past the window spares * nothing, so a dead one cannot pin its session. * - RECONCILE: a bounded batch of compute-holding rows is checked against - * the spawner; a container gone spawner-side settles the row as destroyed - * (phantom heal) — unless the row is pinned: a pinned agent workspace has + * the spawner; a container gone spawner-side settles the row (phantom + * heal) — as stopped for an agent session whose workspace the spawner + * still holds, so the next turn resumes it in place, as destroyed + * otherwise — unless the row is pinned: a pinned agent workspace has * its recreate in place queued (`sandbox.recreate_pinned` — same id, so * the spawner re-attaches its preserved workspace; never inline, since a * create can take minutes). Live sessions have pin drift repaired in either @@ -164,8 +177,11 @@ export interface SandboxWatchdogResult { * failing turn or crawler render batch destroys them best-effort before * the flip (a render batch refused as a duplicate destroys nothing: the * id may be another run's live session); this pass collects what that - * destroy could not, past a grace, behind the same `if_idle` guard as - * RECLAIM, and stamps `destroyed_at_ms` on the row. + * could not, past a grace, behind the same `if_idle` guard as RECLAIM, + * and stamps `destroyed_at_ms` on the row. An agent session's leftover + * loses its compute only: its id may name a workspace preserved for the + * owner's next turn, and deleting what nothing owns is the workspace + * cleanup's. A render's is destroyed whole. * - RELEASE: the crawler's render sessions a stopped process left behind. * A scan link destroys its render session when the batch ends; a link * cut off mid-batch (a restart, a deploy, a crash) leaves the row @@ -320,8 +336,9 @@ export async function runSandboxWatchdog( // What waiting for sandbox room leaves behind: the op rows of refused // starts an hour after they ended (each session's newest kept, the run - // view reads it) and failed session rows a day after they were collected - // (domains/sandbox/wait-retention.ts). + // view reads it) and failed session rows a day after they were collected, + // except a project agent's newest row of its id, which names the + // workspace its collect kept (domains/sandbox/wait-retention.ts). try { const pruned = await sweepRoomWaitLeftovers(sql, { now }); if (pruned.ops + pruned.sessions > 0) { @@ -447,6 +464,7 @@ async function reconcilePass( let healed = 0; let recreating = 0; const visited: Candidate[] = []; + const passSpawner = sharingInventory(spawner); for (const candidate of candidates) { if (args.signal?.aborted === true) break; visited.push(candidate); @@ -454,7 +472,7 @@ async function reconcilePass( const outcome = await reconcileSession( sql, { organizationId: candidate.orgId, sessionId: candidate.sessionId }, - spawner, + passSpawner, { ...(args.scheduleRecreate !== undefined ? { schedule: args.scheduleRecreate } @@ -477,6 +495,19 @@ async function reconcilePass( return { healed, recreating }; } +/** The spawner, reading its workspace inventory at most once for the pass + * (when the first heal asks): a host reboot leaves every session's compute + * gone at once, and each agent session's heal asks whether its workspace is + * still held. A failed read is shared too; each heal reads it as unknown. + * The first heal's options bound the read: every heal of the pass passes + * the same pass signal. */ +function sharingInventory(spawner: WatchdogSpawner): WatchdogSpawner { + const read = spawner.inventory; + if (read === undefined) return spawner; + let shared: Promise | undefined; + return { ...spawner, inventory: (options) => (shared ??= read(options)) }; +} + /** * The Sandboxes page's mount-time probe (the 0.4 `reconcileOrgSessions`): * the SAME fair lifecycle and pin-drift pass the sweep runs, scoped to one @@ -627,9 +658,10 @@ async function releaseAbandonedRenderSessions( * Session ids are deterministic, so the next turn inserts a fresh row under * the failed row's id: a row whose id a newer or live incarnation carries is * settled WITHOUT a spawner call, since whatever the spawner holds under the - * id is that incarnation's. Otherwise the spawner destroys the session only - * when idle (`if_idle`), exactly like the reclaim: busy and errors leave the - * row for a later tick. + * id is that incarnation's. Otherwise the spawner removes the session only + * when idle (`if_idle`), exactly like the reclaim — an agent session's + * compute alone, keeping its workspace, a render's whole: busy and errors + * leave the row for a later tick. */ async function collectFailedSessions( sql: Sql, @@ -637,8 +669,9 @@ async function collectFailedSessions( args: { batch: number; graceMs: number; now: number; signal?: AbortSignal }, ): Promise { const horizon = args.now - args.graceMs; - const candidates = await sql` - SELECT id, session_id AS "sessionId", org_id AS "orgId" + const candidates = await sql<(Candidate & { ownerType: string })[]>` + SELECT id, session_id AS "sessionId", org_id AS "orgId", + owner_type AS "ownerType" FROM app.sandbox_sessions WHERE status = 'failed' AND destroyed_at_ms IS NULL AND coalesce(last_activity_at_ms, created_at_ms) < ${horizon} @@ -653,14 +686,18 @@ async function collectFailedSessions( // Asked per row, right before the spawner call, so a successor inserted // after the batch was selected still holds the destroy off. if (!(await isSupersededIncarnation(sql, candidate))) { - let outcome: { destroyed: boolean; busy: boolean }; + let outcome: { busy: boolean }; try { - outcome = await spawner.destroyIfIdle(candidate.sessionId, { - signal: args.signal, - }); + outcome = AGENT_OWNER_TYPES.has(candidate.ownerType) + ? await spawner.stopIfIdle(candidate.sessionId, { + signal: args.signal, + }) + : await spawner.destroyIfIdle(candidate.sessionId, { + signal: args.signal, + }); } catch (error) { console.warn( - `[watchdog] failed-session destroy failed for ${candidate.sessionId}:`, + `[watchdog] failed-session collect failed for ${candidate.sessionId}:`, error, ); continue; @@ -673,6 +710,13 @@ async function collectFailedSessions( return collected; } +/** The owners whose sessions run an agent in a workspace kept between its + * turns or steps: a project agent's, an automation run's. */ +const AGENT_OWNER_TYPES: ReadonlySet = new Set([ + 'project_agent', + 'workflow_run', +]); + /** Does another incarnation own the spawner session a failed row names — a * newer row of any status, or a live row of any age? Deployment-wide, not * per organization: the spawner's session namespace is. */ diff --git a/services/platform/backend/domains/websites/render-failed-create.integration.ts b/services/platform/backend/domains/websites/render-failed-create.integration.ts index 4c312512d2..8c3284be9c 100644 --- a/services/platform/backend/domains/websites/render-failed-create.integration.ts +++ b/services/platform/backend/domains/websites/render-failed-create.integration.ts @@ -154,6 +154,10 @@ export async function checkRenderFailedCreate( : { destroyed: false, busy: true }, ); }, + // Agent sessions' leftovers lose their compute alone; this lane has + // none, and other lanes' rows answer busy. + stopIfIdle: (): Promise<{ stopped: boolean; busy: boolean }> => + Promise.resolve({ stopped: false, busy: true }), }; let collected = await readRows(); let passes = 0; diff --git a/services/platform/backend/integration-check.ts b/services/platform/backend/integration-check.ts index 5d490420d7..5993f53918 100644 --- a/services/platform/backend/integration-check.ts +++ b/services/platform/backend/integration-check.ts @@ -57911,6 +57911,8 @@ async function checkWatchdogs( : { destroyed: false, busy: true }, ); }, + stopIfIdle: (): Promise<{ stopped: boolean; busy: boolean }> => + Promise.resolve({ stopped: false, busy: true }), }; const tick1 = await sandboxWatchdogs.runSandboxWatchdog(sql, { reconcileBatch: 2, @@ -58080,6 +58082,8 @@ async function checkWatchdogs( : { destroyed: true, busy: false }, ); }, + stopIfIdle: (): Promise<{ stopped: boolean; busy: boolean }> => + Promise.resolve({ stopped: false, busy: true }), }, }); const releaseRows = await sql< @@ -58172,6 +58176,8 @@ async function checkWatchdogs( }, destroyIfIdle: (): Promise<{ destroyed: boolean; busy: boolean }> => Promise.resolve({ destroyed: false, busy: false }), + stopIfIdle: (): Promise<{ stopped: boolean; busy: boolean }> => + Promise.resolve({ stopped: false, busy: false }), }; const orgLaneRows = [ 'wd-org-phantom', @@ -58252,12 +58258,13 @@ async function checkWatchdogs( `; // Lane 3c: the failed-create collect (#3494). A failed row whose spawner - // session is still live is destroyed and stamped by primary key, keeping - // `failed`; a failed row whose deterministic id a newer, hibernated - // incarnation carries is stamped WITHOUT a spawner call, and that - // incarnation's row and token stay untouched; a busy session and a failure - // inside the grace wait. The scripted spawner answers busy for every - // session outside this lane, so rows other lanes left are not disturbed. + // session is still live is removed (an agent session's compute alone, its + // workspace kept) and stamped by primary key, keeping `failed`; a failed + // row whose deterministic id a newer, hibernated incarnation carries is + // stamped WITHOUT a spawner call, and that incarnation's row and token + // stay untouched; a busy session and a failure inside the grace wait. The + // scripted spawner answers busy for every session outside this lane, so + // rows other lanes left are not disturbed. const collectAt = now - 2 * 3_600_000; await sql` INSERT INTO app.sandbox_sessions ( @@ -58291,6 +58298,17 @@ async function checkWatchdogs( ) `; const collectAsked: string[] = []; + // These lanes' failed rows are automation runs' agent sessions, whose + // leftovers lose their compute alone (`stopIfIdle`); a render's would be + // destroyed whole. Both answer the same script. + const collectAnswer = ( + sessionId: string, + ): { removed: boolean; busy: boolean } => { + collectAsked.push(sessionId); + return sessionId === 'wd-collect-live' + ? { removed: true, busy: false } + : { removed: false, busy: true }; + }; const collectSpawner = { isAlive: (): Promise => Promise.resolve(true), setPinned: (): Promise => Promise.resolve(true), @@ -58298,12 +58316,14 @@ async function checkWatchdogs( destroyIfIdle: ( sessionId: string, ): Promise<{ destroyed: boolean; busy: boolean }> => { - collectAsked.push(sessionId); - return Promise.resolve( - sessionId === 'wd-collect-live' - ? { destroyed: true, busy: false } - : { destroyed: false, busy: true }, - ); + const { removed, busy } = collectAnswer(sessionId); + return Promise.resolve({ destroyed: removed, busy }); + }, + stopIfIdle: ( + sessionId: string, + ): Promise<{ stopped: boolean; busy: boolean }> => { + const { removed, busy } = collectAnswer(sessionId); + return Promise.resolve({ stopped: removed, busy }); }, }; const readCollectRows = () => sql< @@ -58439,6 +58459,12 @@ async function checkWatchdogs( refusedAsked.push(sessionId); return Promise.resolve({ destroyed: false, busy: true }); }, + stopIfIdle: ( + sessionId: string, + ): Promise<{ stopped: boolean; busy: boolean }> => { + refusedAsked.push(sessionId); + return Promise.resolve({ stopped: false, busy: true }); + }, }, }); const refusedAfter = await sql< @@ -58461,7 +58487,11 @@ async function checkWatchdogs( // What waiting for room leaves behind goes: the op rows of refused starts // an hour after they ended — the session's newest kept, the run view - // reads it — and failed session rows a day after they were collected. + // reads it — and failed session rows a day after they were collected, + // except the newest row of a project agent's id: a collected failed + // create of an agent session keeps its workspace, and that row is what + // the unused, member and agent cleanup find it by. A first create that + // failed leaves such a row alone; an automation run's goes all the same. const waitSession = `wf-wd-wait-${randomUUID()}`; const hourAgo = now - 2 * 60 * 60 * 1000; for (const [execId, startedAt] of [ @@ -58490,16 +58520,27 @@ async function checkWatchdogs( ) `; const day = 24 * 60 * 60 * 1000; - const collectedRows = await sql<{ id: string; old: boolean }[]>` + const collectedRows = await sql<{ id: string; label: string }[]>` INSERT INTO app.sandbox_sessions ( org_id, session_id, status, owner_type, owner_id, created_by, created_at_ms, expires_at_ms, destroyed_at_ms ) VALUES + (${orgId}, 'pa-wd-collected-old', 'failed', 'project_agent', 'agent-wd', + 'itest', ${now - day - 180_000}, ${now}, ${now - day - 150_000}), (${orgId}, 'pa-wd-collected-old', 'failed', 'project_agent', 'agent-wd', 'itest', ${now - day - 120_000}, ${now}, ${now - day - 60_000}), + (${orgId}, 'pa-wd-collected-lone', 'failed', 'project_agent', + 'agent-wd', 'itest', ${now - day - 120_000}, ${now}, + ${now - day - 60_000}), + (${orgId}, 'wf-wd-collected-lone', 'failed', 'workflow_run', 'run-wd', + 'itest', ${now - day - 120_000}, ${now}, ${now - day - 60_000}), (${orgId}, 'pa-wd-collected-new', 'failed', 'project_agent', 'agent-wd', 'itest', ${now - 120_000}, ${now}, ${now - 60_000}) - RETURNING id, destroyed_at_ms < ${now - day} AS old + RETURNING id, + session_id || CASE + WHEN session_id = 'pa-wd-collected-old' + AND created_at_ms = ${now - day - 180_000} + THEN ':older' ELSE '' END AS label `; const { sweepRoomWaitLeftovers } = await import('./domains/sandbox/wait-retention.ts'); @@ -58510,17 +58551,25 @@ async function checkWatchdogs( WHERE session_id = ${waitSession} ORDER BY started_at_ms ` ).map((row) => row.execId); - const collectedLeft = await sql<{ id: string }[]>` - SELECT id FROM app.sandbox_sessions - WHERE id = ANY(${collectedRows.map((row) => row.id)}) - `; - const keptRecent = collectedRows.find((row) => !row.old)?.id; - record( - 'what waiting for room leaves behind is deleted past its retention, the newest op and a keyed one kept', - waitOpsLeft.join(',') === 'wait-keyed,wait-3' && - collectedLeft.length === 1 && - collectedLeft[0]?.id === keptRecent, - `ops=${waitOpsLeft.join(',')} (want wait-keyed,wait-3) sessions=${collectedLeft.length}/1 recent kept=${String(collectedLeft[0]?.id === keptRecent)}`, + const collectedLeft = new Set( + ( + await sql<{ id: string }[]>` + SELECT id FROM app.sandbox_sessions + WHERE id = ANY(${collectedRows.map((row) => row.id)}) + ` + ).map((row) => row.id), + ); + const collectedKept = collectedRows + .filter((row) => collectedLeft.has(row.id)) + .map((row) => row.label) + .sort() + .join(','); + const wantKept = + 'pa-wd-collected-lone,pa-wd-collected-new,pa-wd-collected-old'; + record( + "what waiting for room leaves behind is deleted past its retention, the newest op, a keyed one and a project agent's newest row kept", + waitOpsLeft.join(',') === 'wait-keyed,wait-3' && collectedKept === wantKept, + `ops=${waitOpsLeft.join(',')} (want wait-keyed,wait-3) sessions kept=${collectedKept} (want ${wantKept})`, ); // Lane 4: a stale chat generation (hard-killed turn) clears; the thread diff --git a/services/platform/tests/manual/reference/automation/tasks.md b/services/platform/tests/manual/reference/automation/tasks.md index 1880275909..3ecba59b7a 100644 --- a/services/platform/tests/manual/reference/automation/tasks.md +++ b/services/platform/tests/manual/reference/automation/tasks.md @@ -26,6 +26,10 @@ Rows for [`tasks`](../../suites/tasks.md) boxes that moved out of the suite. Rea | [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | Drain checkpoints preserve partial parser and background-task state; oversized JSONL records fail at an 8 MiB UTF-8 limit, and recovery only clears a replay gap after its checkpoint covers the missing range; new lightweight workspaces preserve their profile on resume; explicit Claude effort changes the prompt and per-exec setting | ✅ automated | `backend/core/node_only/sandbox/agent_session.test.ts`, `backend/core/automations/agent_host.progress.test.ts`, `backend/core/node_only/sandbox/session_exec.test.ts`, `backend/core/chat/external_turn_shared.test.ts`, `lib/harnesses/jsonl.test.ts`, `lib/harnesses/exec-builder.test.ts`; live model quality and host storage quotas remain deployment checks | | [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | Slow progress storage retains one in-flight write and one merged pending snapshot, preserving intervening tool events and exact final reports. Full exec replay preserves background tasks and OpenCode/Pi usage beyond the bounded display history; replay loss is explicit. Warm preparation verifies source identities and actual file hashes, transfers only misses, prunes managed roots after successful staging, and permits a mount to switch between inline content and a folder without discarding unchanged folder files. Sandbox acquisitions admit idle-to-active growth once, Pending Kubernetes Pods consume capacity until fenced recovery, and failed pin or unpin acknowledgements remain retryable. Recovery rotates durable visit clocks past unreachable runs with bounded cancellable probes, while direct chat uses independent worker slots. Staging cancellation reaches managed-root inspection, replacement and legacy cleanup; staging and Docker creation obey whole-operation budgets; failed creates preserve workspace directories and PVCs, and cleanup removes only the Docker container or Kubernetes object owned by that attempt. Delayed progress never overwrites a newer heartbeat; FIFO replacements cannot block file verification or streamed reads. | ✅ unit + HTTP + real Postgres | `backend/core/automations/agent_host.progress.test.ts`, `backend/core/chat/external_turn_shared.test.ts`, `lib/harnesses/projection.test.ts`, `backend/core/node_only/sandbox/helpers/session_client.staging.test.ts`, `backend/core/automations/agent_host.files_staging.test.ts`, `backend/core/automations/agent_host.files_staging.transitions.test.ts`, `backend/domains/sandbox/service.test.ts`, `backend/domains/sandbox/recovery.test.ts`, `backend/jobs/runner.test.ts`, `backend/domains/tasks/agent-turn-shim.integration.ts`, `backend/integration-check.ts`, `services/sandbox/src/session/session-routes.test.ts`, `services/sandbox/src/backend/kubernetes/k8s-session-backend.test.ts`, `services/sandbox/src/backend/docker/docker-session-backend.test.ts`, `services/sandbox-runtime/daemon/src/exec-manager.test.ts`, `services/sandbox-runtime/daemon/src/file-ops.test.ts`; production throughput and host sizing still require representative deployment measurements | | [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | Reconnects seek to the requested journal suffix while preserving exact output and replay markers; bounded vector writes tolerate partial writes and keep disposed files charged until closure. Stalled writes, reads and checkpoints fail within their I/O deadline while pending kernel operations retain descriptor and physical-budget ownership until settlement. Fragmented SSE frames preserve CRLF-delimited output and the existing frame limit without rescanning retained prefixes. Concurrent sandbox probes share only in-flight work, and a delayed liveness result cannot evict a replacement or release a stop-owned slot. | ✅ unit + HTTP + filesystem | `services/sandbox-runtime/daemon/src/exec-replay.test.ts`, `services/sandbox-runtime/daemon/src/exec-replay.io.test.ts`, `services/sandbox-runtime/daemon/src/exec-replay.performance.test.ts`, `services/sandbox-runtime/daemon/src/exec-manager.replay.test.ts`, `services/sandbox-runtime/daemon/src/exec-manager.test.ts`, `backend/core/node_only/sandbox/helpers/session_client.test.ts`, `services/sandbox/src/session/session-routes.test.ts`; `scripts/performance/journal.ts` measures host journal writes and late reconnects, not complete agent-run latency | +| [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | A sandbox create that loses the session's name to a live session the spawner's registry does not hold (a running container of its own instance, a Pod neither terminating nor ended) answers `409 duplicate`, so the platform adopts that session through acquire instead of cleaning up after a failed create; a name held by anything else (a container still `created` or being removed, another instance's container, a terminating or ended Pod) stays `502 create_failed`, and nothing under the name is removed | ✅ unit + HTTP | `services/sandbox/src/backend/docker/docker-session-create-cleanup.test.ts` (the `docker run` name conflict per container state and instance), `services/sandbox/src/backend/kubernetes/k8s-session-backend.test.ts` (Pod and Secret 409 per Pod phase), `services/sandbox/src/session/session-routes.test.ts` (409 against 502 on create); the platform's adoption on 409 is `backend/core/node_only/sandbox/agent_session.test.ts` | +| [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | `SBX-R17`: a project agent's or automation run's sandbox on the server that disappears without a Destroy (a host reboot, a daemon restart, the OOM killer) leaves its workspace. The watchdog's reconcile settles the unpinned agent row as `stopped` while the spawner's inventory lists the workspace or cannot be read (credentials reclaimed, slot freed, parked runs woken), so the next turn resumes it in place with its conversation; a render session, a workspace the spawner no longer holds, or one on a connected device (the inventory lists the server's only), settles as destroyed; the inventory is read at most once per pass. A create that fails afterwards, and the COLLECT pass for such a failed row, stop the session's compute only (`?if_idle=1&keep_workspace=1`, busy refused) and never delete the workspace; a render's leftover is still destroyed whole; the device hub keeps the placement of a workspace its device kept. A collected failed row of a project agent stays past the day's retention while it is the newest row naming its id, so the unused rule and the agent and member removals still find the workspace it kept; it goes once a newer row names the id | 🔶 unit + HTTP | `backend/domains/sandbox/service.test.ts` (heal to stopped or destroyed per profile and inventory), `backend/domains/sandbox/watchdogs.test.ts` (one inventory read per pass; COLLECT stop against destroy by owner), `backend/core/node_only/sandbox/agent_session.test.ts`, `backend/core/node_only/sandbox/helpers/session_client.test.ts`, `services/sandbox/src/session/session-routes.test.ts` (idle, busy, absent and failed stops), `services/sandbox/src/server.test.ts` (the flag reaches the route), `services/sandbox/src/devices/hub.test.ts` (the placement stays), `backend/domains/sandbox/wait-retention.test.ts` (the retention statement's shape); `backend/integration-check.ts` keeps the COLLECT lane and the retention of a project agent's newest row on real Postgres, while the heal's `stopped` write is proven against scripted SQL only and a live host reboot stays a deployment check | +| [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | runnerd survives an unhandled promise rejection (logged), where Node would end it and with it the session container and every exec in it; an uncaught exception stops it through the SIGTERM path (live execs told to end, the forced two-second deadline) and it exits 70, a second exception during that stop starting nothing new | ✅ unit + Node process | `services/sandbox-runtime/daemon/src/process-guard.test.ts` (the handlers on a stand-in emitter; the bundled guard under Node, surviving a stray rejection and exiting 70 through the stop on an exception); a real runnerd crash inside a container stays a deployment check | +| [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | An exec's command, and everything it starts, runs with an OOM score adjustment of at least 900 while the subreaper shim and runnerd keep the container's (500 on Docker), so a session at its memory limit loses a build or a test run to the OOM killer before runnerd and with it the whole container; a higher score is never lowered, and a kernel without the file or refusing the write runs the command unchanged and reports nothing | 🔶 unit (Linux) | `services/sandbox-runtime/daemon/src/exec-shim.test.ts` (on Linux, the shim built from its source: the command's `/proc/self/oom_score_adj` against the shim's); a real cgroup OOM kill inside a session stays a deployment check | | [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | Corrupt SSE/NDJSON frames fail before advancing replay cursors and release stream readers without waiting for cancellation acknowledgements; staging has one batch deadline; inner Docker readiness is bounded and capability loss refuses new work while preserving busy or pinned sessions. Devices enforce host admission, recheck candidate connection identity and compatibility before admission and after durable publication, retain ambiguous remote placements, and reject corrupt placement snapshots; a proven pre-send refusal releases only a fresh placement for fallback, while an established retry remains sticky. Disk admission considers workspace and verified Docker data-root filesystems, preferring explicit mounts and falling back to verified automatic discovery. Kubernetes daemon liveness stays independent of inner Docker readiness. | ✅ unit + HTTP | `backend/core/node_only/sandbox/helpers/session_client.test.ts`, `services/sandbox/src/session/runnerd-client.test.ts`, `session-routes.test.ts`, `services/sandbox/src/devices/hub.test.ts`, `placements.test.ts`, `services/sandbox/src/host-disk.test.ts`, `services/sandbox/src/docker-data-disk.test.ts`, `services/sandbox/src/backend/kubernetes/k8s-session-pod-spec.test.ts`, `services/sandbox-runtime/daemon/src/file-ops.test.ts`, `inner-docker-health.test.ts`, `inner-docker-network.test.ts`, `main.test.ts`; sustained production load and external volume quota enforcement remain deployment checks | | [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | Fragmented harness output keeps exact bounded UTF-16 tails, JSON byte limits and immutable timeline snapshots without repeatedly serializing growing text. Completed execs retain journal metadata without duplicate output strings, and old/full journal replay stays exact. Sandbox recovery pairs endpoints with the listed creation stamp and refreshes replacement metadata; stale readiness, admission and cleanup completions cannot launch work against or clear a successor's state. Fenced linger stops preserve replacement compute. | ✅ unit + HTTP + filesystem + Linux runtime | `lib/harnesses/bounded-text-tail.test.ts`, `lib/harnesses/projection.test.ts`, `services/sandbox-runtime/daemon/src/exec-manager.memory.test.ts`, `services/sandbox/src/session/session-routes.test.ts`, `services/sandbox/src/backend/docker/docker-session-backend.test.ts`, `services/sandbox/src/backend/kubernetes/k8s-session-backend.test.ts`; `scripts/performance/projection.ts` measures fixed-input display projection, not complete agent-run latency; live Kubernetes replacement recovery remains a deployment check | | [tasks](../../suites/tasks.md) / [automations](../../suites/automations.md) | Parser deduplication ledgers fail explicitly at bounded entry/character budgets; workflow terminal answers without final text preserve the complete bounded fallback beyond the display tail; task runs still require an explicit final report. Malformed base64 fails before output delivery or cursor advancement. Session provisioning reuses only its own verified provider IDs for minting. Exec and attach share eight consumer slots, staging uses one batch deadline and preserves executable permissions, and cached-source probes reject pipes and replaced files. Docker-free agents default to 4 GiB even in a Docker-enabled deployment; explicit memory overrides apply to both modes. | ✅ unit + HTTP + Linux daemon | `lib/harnesses/jsonl.test.ts`, `lib/harnesses/projection.test.ts`, `backend/core/chat/external_turn_shared.test.ts`, `backend/core/automations/agent_host.empty_answer.test.ts`, `backend/core/tasks/agent_run_host.empty_answer.test.ts`, `backend/core/node_only/sandbox/helpers/session_client.test.ts`, `backend/core/node_only/sandbox/gateway_provisioning.test.ts`, `backend/core/node_only/sandbox/llm_gateway_admin.test.ts`, `services/sandbox-runtime/daemon/src/main.test.ts`, `services/sandbox-runtime/daemon/src/file-ops.test.ts`, `services/sandbox/src/config.test.ts`, `services/sandbox/src/session/docker-session-args.test.ts`, `services/sandbox/src/backend/kubernetes/k8s-session-pod-spec.test.ts`; live provider throughput and production memory peaks remain deployment checks | diff --git a/services/sandbox-runtime/daemon/exec-shim/tale-exec-shim.c b/services/sandbox-runtime/daemon/exec-shim/tale-exec-shim.c index e276404c68..6628ed5344 100644 --- a/services/sandbox-runtime/daemon/exec-shim/tale-exec-shim.c +++ b/services/sandbox-runtime/daemon/exec-shim/tale-exec-shim.c @@ -26,12 +26,21 @@ * command started. It exits once no descendant is left, with the command's * status (128 + N for a command ended by signal N). Without the subreaper (a * kernel that refuses it) it still runs the command and reports it. + * + * The command runs with an OOM score adjustment of at least 900, above the + * session container's (and so runnerd's): when the container reaches its + * memory limit, the kernel's OOM killer picks among the highest scores, and + * runnerd, whose end ends the container and every exec in it, must not be + * the one it picks. Raising a score needs no privilege; where the kernel has + * no such file or refuses the write, the command runs with the score it + * inherited, and nothing is reported. */ #define _GNU_SOURCE #include #include #include #include +#include #include #include #include @@ -39,6 +48,7 @@ #include #define STATUS_FD 3 +#define COMMAND_OOM_SCORE_ADJ 900 /* Write one status line; lines are far below PIPE_BUF, so each write is * atomic and the command's own spawn-error line never interleaves. */ @@ -63,6 +73,28 @@ static void report_errno(const char *event, int error) { report(line); } +/* Raise this process's OOM score adjustment to at least `target`, never + * lower it (an operator's higher score stands). Best effort and silent: the + * exec runs whatever the kernel answers. */ +static void raise_oom_score_adj(int target) { + const char *path = "/proc/self/oom_score_adj"; + char current[16]; + int fd = open(path, O_RDONLY | O_CLOEXEC); + if (fd < 0) return; + ssize_t got = read(fd, current, sizeof current - 1); + close(fd); + if (got <= 0) return; + current[got] = '\0'; + if (atoi(current) >= target) return; + char value[16]; + int length = snprintf(value, sizeof value, "%d\n", target); + fd = open(path, O_WRONLY | O_CLOEXEC); + if (fd < 0) return; + ssize_t written = write(fd, value, (size_t)length); + (void)written; + close(fd); +} + static void ignore_ending_signals(void) { signal(SIGTERM, SIG_IGN); signal(SIGINT, SIG_IGN); @@ -109,6 +141,9 @@ int main(int argc, char **argv) { setpgid(0, 0); default_signals(); sigprocmask(SIG_SETMASK, &previous, NULL); + /* The command and everything it starts inherit the higher score; the + * shim keeps runnerd's. */ + raise_oom_score_adj(COMMAND_OOM_SCORE_ADJ); /* The status pipe closes on a successful exec; it stays open only to * say why the exec failed. */ fcntl(STATUS_FD, F_SETFD, FD_CLOEXEC); diff --git a/services/sandbox-runtime/daemon/src/exec-shim.test.ts b/services/sandbox-runtime/daemon/src/exec-shim.test.ts index 23739d3002..f922fdbfc6 100644 --- a/services/sandbox-runtime/daemon/src/exec-shim.test.ts +++ b/services/sandbox-runtime/daemon/src/exec-shim.test.ts @@ -1,16 +1,18 @@ // Execs under the subreaper shim (exec-shim/tale-exec-shim.c), built from its // source for this run. Everything an exec starts stays the shim's // descendant, so a process that leaves the exec's group, moves to a session -// of its own and drops the exec's tag is still ended with the exec. Linux -// only, and only where a C compiler is at hand; a compiler that fails to -// build the shim fails the run. What runnerd makes of the shim's status -// pipe is checked everywhere, against stand-in shims. +// of its own and drops the exec's tag is still ended with the exec, and runs +// with an OOM score above runnerd's. Linux only, and only where a C compiler +// is at hand; a compiler that fails to build the shim fails the run. What +// runnerd makes of the shim's status pipe is checked everywhere, against +// stand-in shims. import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; import { spawnSync } from 'node:child_process'; import { mkdirSync, mkdtempSync, + readFileSync, realpathSync, rmSync, writeFileSync, @@ -165,6 +167,30 @@ describe.skipIf(SHIM === null)('ExecManager under the subreaper shim', () => { }); }); + // A cgroup OOM kill picks among the highest scores: the command and what + // it starts must rank above runnerd, whose end ends the whole session. + test('the command runs with an OOM score of at least 900, and the shim keeps runnerd’s', async () => { + using mgr = shimmed(); + const { events, emit } = collect(); + const own = Number(readFileSync('/proc/self/oom_score_adj', 'utf8')); + await mgr.run( + { + ...base, + execId: 'sh-oom', + shell: 'cat /proc/self/oom_score_adj /proc/$PPID/oom_score_adj', + cwd: ROOT, + }, + emit, + ); + const [command, shim] = decode(events, 'stdout').trim().split('\n'); + expect(Number(command)).toBe(Math.max(900, own)); + expect(Number(shim)).toBe(own); + expect(events[events.length - 1]).toMatchObject({ + t: 'exit', + exitCode: 0, + }); + }); + test('a command ended by a signal exits with 128 + its number', async () => { using mgr = shimmed(); const { events, emit } = collect(); diff --git a/services/sandbox-runtime/daemon/src/main.ts b/services/sandbox-runtime/daemon/src/main.ts index 6b0c485057..c07478f397 100644 --- a/services/sandbox-runtime/daemon/src/main.ts +++ b/services/sandbox-runtime/daemon/src/main.ts @@ -40,6 +40,7 @@ import { InnerDockerHealth, LAZY_DOCKER_HEALTH_SOCKET, } from './inner-docker-health.ts'; +import { installProcessGuards } from './process-guard.ts'; import { RUNNERD_CHECKPOINT_MAX_BYTES, parseRunnerdSequence, @@ -779,19 +780,24 @@ if ( // transcript and a wrapper to remove what it staged before the teardown. // Either exit ends the daemon even past a /proc read that never returns // (daemon-exit.ts). + const shutdown = (code: number): void => { + // Journal/file I/O can block libuv too; the hard deadline cannot rely + // on the process-table read counter to decide whether exit is safe. + setTimeout(() => exitDaemon(code, { force: true }), 2_000); + void execManager + .terminateAll() + .catch((error: unknown) => { + console.warn('[runnerd] passing the stop on failed:', error); + }) + .finally(() => server.close(() => exitDaemon(code))); + }; for (const sig of ['SIGTERM', 'SIGINT'] as const) { - process.on(sig, () => { - // Journal/file I/O can block libuv too; the hard deadline cannot rely - // on the process-table read counter to decide whether exit is safe. - setTimeout(() => exitDaemon(0, { force: true }), 2_000); - void execManager - .terminateAll() - .catch((error: unknown) => { - console.warn('[runnerd] passing the stop on failed:', error); - }) - .finally(() => server.close(() => exitDaemon(0))); - }); + process.on(sig, () => shutdown(0)); } + // An unhandled rejection is logged and survived; an uncaught exception + // stops the daemon the same way, with an exit code of its own + // (process-guard.ts). + installProcessGuards({ shutdown }); // Every harness finds the image's built-in skills among its own from the // session's first moment, not only after its first exec. diff --git a/services/sandbox-runtime/daemon/src/process-guard.test.ts b/services/sandbox-runtime/daemon/src/process-guard.test.ts new file mode 100644 index 0000000000..8b277ab535 --- /dev/null +++ b/services/sandbox-runtime/daemon/src/process-guard.test.ts @@ -0,0 +1,114 @@ +// runnerd is the session container's only process under its init: an error +// nothing handled must not end every exec in the session. A rejection is +// survived; an uncaught exception stops the daemon through its graceful stop +// and exits 70. The second test runs the bundled guard under Node, as the +// image does, whose default ends a process on an unhandled rejection. + +import { afterAll, describe, expect, test } from 'bun:test'; +import { spawn, spawnSync } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; + +import { + installProcessGuards, + UNCAUGHT_EXCEPTION_EXIT_CODE, +} from './process-guard.ts'; + +const roots: string[] = []; +afterAll(() => { + for (const root of roots) rmSync(root, { recursive: true, force: true }); +}); + +const hasNode = spawnSync('node', ['--version']).status === 0; + +/** Run `body` with console.error captured. */ +function quietly(body: () => void): string[] { + const logged: string[] = []; + const error = console.error; + console.error = (...args: unknown[]) => { + logged.push(args.map(String).join(' ')); + }; + try { + body(); + } finally { + console.error = error; + } + return logged; +} + +describe('installProcessGuards', () => { + test('logs an unhandled rejection and keeps the daemon running', () => { + const events = new EventEmitter(); + const stops: number[] = []; + installProcessGuards({ events, shutdown: (code) => stops.push(code) }); + const logged = quietly(() => { + events.emit('unhandledRejection', new Error('stray rejection')); + }); + expect(stops).toEqual([]); + expect(logged.join('\n')).toContain('unhandled rejection (surviving)'); + expect(logged.join('\n')).toContain('stray rejection'); + }); + + test('stops the daemon once on an uncaught exception, with its own exit code', () => { + const events = new EventEmitter(); + const stops: number[] = []; + installProcessGuards({ events, shutdown: (code) => stops.push(code) }); + const logged = quietly(() => { + events.emit('uncaughtException', new Error('boom'), 'uncaughtException'); + events.emit('uncaughtException', new Error('again'), 'uncaughtException'); + }); + expect(UNCAUGHT_EXCEPTION_EXIT_CODE).toBe(70); + expect(stops).toEqual([70]); + // Both are said; only the first starts the stop. + expect( + logged.filter((line) => line.includes('uncaught exception')), + ).toHaveLength(2); + }); + + test.skipIf(!hasNode)( + 'under Node, a stray rejection is survived and an uncaught exception exits 70 through the stop', + async () => { + const root = mkdtempSync(`${tmpdir()}/runnerd-guard-`); + roots.push(root); + writeFileSync( + `${root}/entry.ts`, + [ + `import { installProcessGuards } from '${import.meta.dir}/process-guard.ts';`, + `installProcessGuards({ shutdown: (code) => {`, + ` console.log('stopping ' + code);`, + ` setTimeout(() => process.exit(code), 10);`, + `} });`, + `void Promise.reject(new Error('stray rejection'));`, + `setTimeout(() => console.log('survived'), 50);`, + `setTimeout(() => { throw new Error('boom'); }, 100);`, + `setTimeout(() => console.log('still running'), 5_000);`, + ].join('\n'), + ); + const built = await Bun.build({ + entrypoints: [`${root}/entry.ts`], + outdir: `${root}/dist`, + target: 'node', + }); + expect(built.success).toBe(true); + const child = spawn('node', [`${root}/dist/entry.js`], { + stdio: ['ignore', 'pipe', 'ignore'], + }); + let stdout = ''; + child.stdout.on('data', (chunk: Buffer) => { + stdout += chunk.toString('utf8'); + }); + const ended = await new Promise((resolve) => { + const timer = setTimeout(() => resolve('still running'), 5_000); + child.on('exit', (code, signal) => { + clearTimeout(timer); + resolve(signal ?? `code ${code}`); + }); + }); + if (ended === 'still running') child.kill('SIGKILL'); + expect(ended).toBe(`code ${UNCAUGHT_EXCEPTION_EXIT_CODE}`); + expect(stdout).toBe('survived\nstopping 70\n'); + }, + 15_000, + ); +}); diff --git a/services/sandbox-runtime/daemon/src/process-guard.ts b/services/sandbox-runtime/daemon/src/process-guard.ts new file mode 100644 index 0000000000..9a05a189cf --- /dev/null +++ b/services/sandbox-runtime/daemon/src/process-guard.ts @@ -0,0 +1,51 @@ +// What runnerd does with an error nothing handled. runnerd is the only child +// of the session container's init, so the daemon ending ends the container +// and every exec running in it. Node ends a process on an unhandled promise +// rejection by default: one stray rejection in a request path would take the +// whole session with it. A rejection is logged and the daemon carries on, as +// the spawner does. An uncaught exception leaves the daemon in an unknown +// state, so it stops the way a SIGTERM stops it (live execs are told to end, +// bounded by the forced deadline) and exits with a code of its own. + +/** The exit code after an uncaught exception: EX_SOFTWARE from + * sysexits(3), apart from a graceful stop's 0, Node's own 1 and 7, and the + * 128 + N of a signal. */ +export const UNCAUGHT_EXCEPTION_EXIT_CODE = 70; + +/** The two process events the guards listen to. */ +interface ProcessEvents { + on(event: 'unhandledRejection', listener: (reason: unknown) => void): unknown; + on( + event: 'uncaughtException', + listener: (error: Error, origin: string) => void, + ): unknown; +} + +export interface ProcessGuardDeps { + /** Stop the daemon: pass the stop on to the live execs, close the + * server and exit with `code`, bounded by the forced deadline. */ + shutdown: (code: number) => void; + /** Where the handlers go — the process itself unless a test hands in + * another emitter. */ + events?: ProcessEvents; +} + +/** Install runnerd's handlers for errors nothing else handled. */ +export function installProcessGuards(deps: ProcessGuardDeps): void { + const events = deps.events ?? process; + events.on('unhandledRejection', (reason) => { + console.error('[runnerd] unhandled rejection (surviving):', reason); + }); + let stopping = false; + events.on('uncaughtException', (error, origin) => { + console.error( + `[runnerd] uncaught exception (${origin}); stopping live execs and exiting ${UNCAUGHT_EXCEPTION_EXIT_CODE}:`, + error, + ); + // A second one while the stop runs changes nothing: the stop already + // under way exits, and its forced deadline still holds. + if (stopping) return; + stopping = true; + deps.shutdown(UNCAUGHT_EXCEPTION_EXIT_CODE); + }); +} diff --git a/services/sandbox/docs/sessions.md b/services/sandbox/docs/sessions.md index f927a06b14..28b256ecc0 100644 --- a/services/sandbox/docs/sessions.md +++ b/services/sandbox/docs/sessions.md @@ -94,6 +94,26 @@ exec's group and replaced its environment. runnerd's startup line names the shim it uses (`execShim=`). On SIGTERM, runnerd passes the signal on to every live exec, and to what exited execs left waiting, before it exits. +**An exec's command ranks above runnerd for the OOM killer.** runnerd keeps +the score its container starts with (`--oom-score-adj=500` on Docker); before +the shim runs the command it raises the command's adjustment to 900, never +lowering a higher one, and everything the command starts inherits it, while +the shim itself keeps runnerd's. When a session reaches its memory limit, the +kernel's OOM killer, which picks the highest score, ends a build or a test run +before runnerd, whose end would be the container's and every exec's in it. +Raising a score needs no privilege, so this holds with every capability +dropped; where the kernel has no such file or refuses the write, the command +runs with the score it inherited and nothing is reported. + +runnerd is the only child of the container's init, so its end is the +container's, and every exec in it ends too. An error nothing handled does not +take the session with it: an unhandled promise rejection (which Node 24 turns +into an exit) is logged and survived, as in the spawner; an uncaught exception, +after which the daemon's state is unknown, stops runnerd the way SIGTERM does +(live execs told to end, the two-second forced deadline still holding) and it +exits 70 (`EX_SOFTWARE`), apart from a stop's 0 and a signal's 128 + N, or by +SIGKILL when that deadline passes first. + Reading another process's environment waits on that process's memory lock, which a process stuck under memory pressure can hold for minutes. While every exec a round covers still has its shim, the round reads `/proc//stat` @@ -194,6 +214,16 @@ budget after cancellation or failure: only acknowledged Pod and Secret UIDs can be removed, observed Pod deletion also fences its resource version, and workspace PVCs and ambiguous API outcomes remain for retry or recovery. +A create that loses the session's deterministic name to a LIVE session this +spawner's registry does not hold — a running container of this spawner's +instance, or a Pod that is neither terminating nor ended — answers +`409 duplicate`, as a create of a registered session does: the platform then +adopts it through acquire, and the registry-miss resolve below registers it. +Answered as `502 create_failed`, the platform would clean up after a failed +create and remove that session's compute. A name held by anything else (a +container still `created` or being removed, a terminating Pod, one that +cannot be read) stays a `502`, and nothing under the name is touched. + A session absent from this spawner's registry is resolved from the backend. If that inventory or endpoint lookup fails, or an existing nonterminal runtime is still starting, session routes return `503 session_unavailable` with @@ -540,6 +570,18 @@ workspace. Pinned ("always-on") and live-exec sessions are exempt from the reaper entirely, except that compute which has already ended is removed (the pin's own reconcile recreates a pinned session). +Losing compute is not losing the workspace. When the platform's reconcile finds +the compute of an unpinned agent session gone without a Destroy — a host +reboot, a daemon restart, an OOM-killed runnerd, the spawner's own TTL stop — +it settles the row as `stopped` while the spawner's inventory +(`GET /v1/workspaces`) lists the workspace, or cannot be read: the next turn +resumes it in place, same incarnation and harness conversation included. A +render session, or an agent session whose workspace is gone, settles as +destroyed, and so does a session on a connected device: the inventory lists +this host's workspaces only. A create that fails after such a loss removes +only compute (`?keep_workspace=1`, below), never the workspace it would have +re-attached. + A pin change succeeds only after runnerd and the backend's durable record acknowledge it. Failure returns 503 and keeps the last acknowledged `pinned` value visible with `pinSynchronized: false`, so platform reconciliation retries @@ -673,6 +715,16 @@ The spawner's part: defers the destroy. A failed create removes only its own container and preserves every workspace and organization marker, including a newly created directory. A later explicit destroy performs the workspace cleanup. +- `DELETE /v1/sessions/:id?if_idle=1&keep_workspace=1` — compute only: it + refuses (`{busy:true}`) as `if_idle` does, and otherwise stops the session + (`backend.stopSession`) and keeps its workspace, answering + `{stopped, busy: false, workspaceKept: true}`. The platform sends it after a + failed create of an agent session (`agent_session.ts`) and from the + watchdog's collect of such a failed row: the id may name a workspace kept + for its next turn, and deleting what nothing owns is this cleanup's. The + device hub leaves the placement of a session whose device kept the workspace + as it was. A spawner or device older than the flag destroys instead, and its + answer carries no `workspaceKept`. - `DELETE /v1/organizations/:id` — for an organization the platform deleted: destroys every session the backend still holds for it (containers/Pods with their workspaces) and every stopped workspace attributed to it, then its diff --git a/services/sandbox/src/backend/docker/docker-session-backend.ts b/services/sandbox/src/backend/docker/docker-session-backend.ts index b3b6273c9f..280f1defdf 100644 --- a/services/sandbox/src/backend/docker/docker-session-backend.ts +++ b/services/sandbox/src/backend/docker/docker-session-backend.ts @@ -83,6 +83,7 @@ import { } from '../../volume.ts'; import { ORG_ID_ALPHABET_RE } from '../../wire.ts'; import { + SessionExistsError, SessionIncarnationChangedError, type BackendSession, type BackendWorkspace, @@ -495,11 +496,19 @@ export class DockerSessionBackend implements SessionBackend { // A name conflict that survived the reconcile above (the container is // running/created — a likely concurrent winner — or a retry that re-lost // the race) is NOT ours to tear down: surface it without the destructive - // cleanup below. adoptExisting + the route's 409-reuse path recover a - // running peer on a later turn. + // cleanup below. A RUNNING container of this spawner's instance is a + // live session the registry does not hold: answered as a duplicate, so + // the caller adopts it through acquire. Read as a failed create, the + // caller's cleanup would remove what runs under the id. Any other + // holder (still `created`, being removed, unreadable) stays a failure. const nameConflict = isDockerNameConflict(stderr); if (!nameConflict) { await this.cleanupCreateAttempt(spec.sessionId, createAttemptId); + } else if (await this.runsLiveSession(containerName)) { + throw new SessionExistsError( + spec.sessionId, + `container ${containerName} is running`, + ); } throw new Error( `docker run (session) failed: ${stderr || run.stdout.trim()}`, @@ -708,6 +717,27 @@ export class DockerSessionBackend implements SessionBackend { return inspect.stdout.trim(); } + /** Does the named container run, as a session of this spawner's instance? + * An unreadable state is "no": only a definite running container is + * answered as a live session. */ + private async runsLiveSession(containerName: string): Promise { + const inspect = await runDocker( + [ + 'inspect', + '--format', + `{{.State.Status}}\t{{with index .Config.Labels "${SESSION_INSTANCE_LABEL}"}}{{.}}{{end}}`, + containerName, + ], + { timeoutMs: 5_000 }, + ); + if (inspect.exitCode !== 0) return false; + const [status, instance] = inspect.stdout.replace(/\r?\n$/, '').split('\t'); + return ( + status?.trim() === 'running' && + belongsToInstance(instance?.trim(), this.cfg.instance) + ); + } + /** Read identity and status together: a replacement after this observation * must never inherit an orphan's terminal-state removal verdict. */ private async containerState( diff --git a/services/sandbox/src/backend/docker/docker-session-create-cleanup.test.ts b/services/sandbox/src/backend/docker/docker-session-create-cleanup.test.ts index 59241f1c40..a284a85227 100644 --- a/services/sandbox/src/backend/docker/docker-session-create-cleanup.test.ts +++ b/services/sandbox/src/backend/docker/docker-session-create-cleanup.test.ts @@ -9,8 +9,12 @@ async function failedCreate(options: { removalFails?: boolean; discardFails?: boolean; destroyAfterFailure?: boolean; + /** `docker run` loses the name to a container in this state, labelled + * with this instance (the default instance when absent). */ + conflict?: { status: string; instance?: string }; }): Promise<{ error: string | null; + errorName: string | null; destroyError: string | null; warnings: string[]; workspace: boolean; @@ -40,11 +44,15 @@ mock.module(spawnPath, () => ({...realSpawn, runDocker: async (args) => { if (args[0] === 'run') { createAttempt = args.find(value => value.startsWith('tale.create-attempt='))?.split('=')[1] ?? ''; + if (options.conflict) return {...success,exitCode:125,stderr:'docker: Error response from daemon: Conflict. The container name "/tale-sbx-ses-failed" is already in use by container "'+containerId+'".'}; return {...success,exitCode:1,stderr:'runtime startup failed'}; } if (args[0] === 'inspect') { if (containerGone) return {...success,exitCode:1,stderr:'No such container'}; const format = args[2]; + if (options.conflict && format.includes('State.Status')) { + return {...success,stdout:format.includes('.Id') ? containerId+'\t'+options.conflict.status : options.conflict.status+'\t'+(options.conflict.instance ?? '')}; + } if (format.includes('tale.create-attempt')) return {...success,stdout:containerId+'\\t0\\t'+createAttempt}; if (format.includes('tale.docker')) return {...success,stdout:containerId+'\\tfalse\\t'}; return {...success,stdout:format.includes('Mounts') ? '' : 'true'}; @@ -87,17 +95,18 @@ const cfg = { session:{...TEST_SESSION_CONFIG,agentProfile:{...TEST_SESSION_CONFIG.agentProfile,uid:process.getuid() || 10001,gid:process.getgid() || 10001}}, }; let error = null; +let errorName = null; let destroyError = null; const backend = new DockerSessionBackend(cfg,trash); try { await backend.createSession({sessionId:'failed',organizationId:'org-failed',profile:'agent',env:{},createdAtMs:0,ttlMs:1000,idleTimeoutMs:1000}); -} catch (e) { error = e.message; } +} catch (e) { error = e.message; errorName = e.name; } if (options.destroyAfterFailure) { try { await backend.destroySession('failed'); } catch (e) { destroyError = e.message; } } const exists = async (path) => stat(path).then(() => true, () => false); -const result = {error,destroyError,warnings,workspace:await exists(workspace),owner:await exists(owner),trashEntries:(await readdir(trash.dir).catch(() => [])).length,discarded,containerGone,removalTargets}; +const result = {error,errorName,destroyError,warnings,workspace:await exists(workspace),owner:await exists(owner),trashEntries:(await readdir(trash.dir).catch(() => [])).length,discarded,containerGone,removalTargets}; gate.resolve(); await trash.empty(); await rm(root,{recursive:true,force:true}); @@ -168,4 +177,31 @@ describe('Docker session failed-create cleanup', () => { expect(result.discarded).toBe(1); expect(result.trashEntries).toBe(1); }); + + // A running container of this spawner's instance under the name is a live + // session the registry lost: answered as a duplicate, so the platform + // adopts it instead of cleaning up after a failed create. + test('a name taken by a running session is a live duplicate, and nothing is removed', async () => { + const result = await failedCreate({ conflict: { status: 'running' } }); + expect(result.errorName).toBe('SessionExistsError'); + expect(result.error).toContain('already exists'); + expect(result.removalTargets).toEqual([]); + expect(result.containerGone).toBe(false); + expect(result.workspace).toBe(true); + expect(result.owner).toBe(true); + }); + + test('a name taken by a container that is not running stays a failed create, and nothing is removed', async () => { + for (const conflict of [ + { status: 'created' }, + { status: 'removing' }, + { status: 'running', instance: 'device' }, + ]) { + const result = await failedCreate({ conflict }); + expect(result.errorName).toBe('Error'); + expect(result.error).toContain('is already in use'); + expect(result.removalTargets).toEqual([]); + expect(result.workspace).toBe(true); + } + }); }); diff --git a/services/sandbox/src/backend/kubernetes/k8s-session-backend.test.ts b/services/sandbox/src/backend/kubernetes/k8s-session-backend.test.ts index 02a2584ce6..207e5f00ba 100644 --- a/services/sandbox/src/backend/kubernetes/k8s-session-backend.test.ts +++ b/services/sandbox/src/backend/kubernetes/k8s-session-backend.test.ts @@ -14,7 +14,11 @@ import { operationSignal } from '../../operation-budget.ts'; import { SessionRoutes } from '../../session/session-routes.ts'; import { TEST_SESSION_CONFIG } from '../../session/session-test-config.ts'; import type { SpawnerConfig } from '../../types.ts'; -import { SessionIncarnationChangedError, type SessionSpec } from '../types.ts'; +import { + SessionExistsError, + SessionIncarnationChangedError, + type SessionSpec, +} from '../types.ts'; import type { K8sClient } from './k8s-client.ts'; import { KubernetesSessionBackend } from './k8s-session-backend.ts'; import { @@ -908,6 +912,64 @@ describe('KubernetesSessionBackend.createSession — a 409 name conflict is not expect(calls.pvcDeleted).toBe(false); expect(calls.secretDeleted).toBe(1); }); + + // A live Pod under the name is a session the route answers as a duplicate, + // so the platform adopts it rather than cleaning up after a failed create. + test.each(['Running', 'Pending'])( + 'Pod 409 against a live %s Pod: a live duplicate, nothing of it removed', + async (phase) => { + const { client, calls } = stub( + () => Promise.resolve({ metadata: { uid: 'own-secret' } }), + conflict, + { + pod: { + metadata: { annotations: { 'tale.dev/created-at': '1' } }, + status: { phase }, + }, + }, + ); + const err = await rejection( + new KubernetesSessionBackend(cfg, client).createSession(spec), + ); + expect(err).toBeInstanceOf(SessionExistsError); + expect(err?.message).toMatch(/session sess_c4 already exists/); + expect(calls.podDeleted).toBe(false); + expect(calls.pvcDeleted).toBe(false); + expect(calls.secretDeleted).toBe(1); + }, + ); + + test('Secret 409 beside a live Pod: a live duplicate, nothing deleted', async () => { + const { client, calls } = stub(conflict, undefined, { + secret: { uid: 'peer-uid', createdAt: new Date(Date.now() - 600_000) }, + pod: { metadata: {}, status: { phase: 'Running' } }, + }); + const err = await rejection( + new KubernetesSessionBackend(cfg, client).createSession(spec), + ); + expect(err).toBeInstanceOf(SessionExistsError); + expect(calls.secretDeleted).toBe(0); + expect(calls.podDeleted).toBe(false); + }); + + test('Pod 409 against a terminating or ended Pod stays a failed create', async () => { + for (const pod of [ + { metadata: { deletionTimestamp: new Date() }, status: {} }, + { metadata: {}, status: { phase: 'Succeeded' } }, + { metadata: {}, status: { phase: 'Failed' } }, + ]) { + const { client } = stub( + () => Promise.resolve({ metadata: { uid: 'own-secret' } }), + conflict, + { pod }, + ); + const err = await rejection( + new KubernetesSessionBackend(cfg, client).createSession(spec), + ); + expect(err).not.toBeInstanceOf(SessionExistsError); + expect(err?.message).toMatch(/session sess_c4 already exists/); + } + }); }); describe('KubernetesSessionBackend.createSession — a render session has no volume', () => { diff --git a/services/sandbox/src/backend/kubernetes/k8s-session-backend.ts b/services/sandbox/src/backend/kubernetes/k8s-session-backend.ts index 3497e9ccce..470b77c9b4 100644 --- a/services/sandbox/src/backend/kubernetes/k8s-session-backend.ts +++ b/services/sandbox/src/backend/kubernetes/k8s-session-backend.ts @@ -25,6 +25,7 @@ import { isAgentSessionProfile } from '../../session/session-profile.ts'; import type { SpawnerConfig } from '../../types.ts'; import { ID_ALPHABET_RE } from '../../wire.ts'; import { + SessionExistsError, SessionIncarnationChangedError, type BackendSession, type BackendWorkspace, @@ -56,9 +57,21 @@ const ORGANIZATION_ID_ANNOTATION = 'tale.dev/organization-id'; /** Pod annotation carrying the durable "always-on" pin (see setPinned). */ const PINNED_ANNOTATION = 'tale.dev/pinned'; -/** A create that lost the deterministic-name race. The route answers 502 and - * the platform retries; by then adoption has made the live session routable. */ -function conflictError(sessionId: string, cause: unknown): Error { +/** A create that lost the deterministic-name race. A live Pod under the name + * is a session the route answers as a duplicate, so the platform adopts it. + * Anything else (a Pod terminating or ended, a peer's Secret whose Pod is not + * there yet, a Pod that cannot be read) answers 502 and the platform retries; + * by then adoption or the orphan reaps have settled the name. */ +function conflictError( + sessionId: string, + cause: unknown, + livePod: boolean, +): Error { + if (livePod) { + return new SessionExistsError(sessionId, 'a live Pod holds its name', { + cause, + }); + } return new Error( `session ${sessionId} already exists (concurrent create or unadopted live Pod)`, { cause }, @@ -201,7 +214,13 @@ export class KubernetesSessionBackend implements SessionBackend { // may own). Surface the conflict without any cleanup — parity with the // Docker backend's name-conflict rule; adoptExisting / the route's // registry-miss re-resolve pick the live session up on a later turn. - if (httpStatusCode(err) === 409) throw conflictError(spec.sessionId, err); + if (httpStatusCode(err) === 409) { + throw conflictError( + spec.sessionId, + err, + await this.livePodHolds(spec.sessionId), + ); + } // An ambiguous response does not establish ownership. Preserve every // object without an acknowledged UID, and every workspace PVC. throw err; @@ -237,7 +256,11 @@ export class KubernetesSessionBackend implements SessionBackend { // only the Secret THIS call created is ours, and leaving it behind // would 409 every future create of this session forever. ownership.podConflict = true; - throw conflictError(spec.sessionId, err); + throw conflictError( + spec.sessionId, + err, + await this.livePodHolds(spec.sessionId), + ); } } else { throw err; @@ -279,6 +302,29 @@ export class KubernetesSessionBackend implements SessionBackend { } } + /** Does a live Pod hold the session's name — one neither being deleted nor + * ended (Succeeded/Failed)? A Pending Pod counts: a peer replica is still + * starting it. A read that fails is "no". */ + private async livePodHolds(sessionId: string): Promise { + try { + const pod = await this.readPod(sessionId); + const phase = pod.status?.phase; + return ( + pod.metadata?.deletionTimestamp == null && + phase !== 'Succeeded' && + phase !== 'Failed' + ); + } catch (error) { + if (httpStatusCode(error) !== 404) { + console.warn( + `[sandbox.session] cannot tell whether a live pod holds ${sessionId}'s name:`, + error, + ); + } + return false; + } + } + /** The session's Secret, read through `list`, which the Role grants (not * `get`): undefined when there is none, null when it cannot be read. */ private async readSessionSecret( diff --git a/services/sandbox/src/backend/types.ts b/services/sandbox/src/backend/types.ts index af079eb427..fb9c03bd17 100644 --- a/services/sandbox/src/backend/types.ts +++ b/services/sandbox/src/backend/types.ts @@ -30,6 +30,20 @@ export class SessionIncarnationChangedError extends Error { } } +/** A create found a LIVE session under the id's deterministic name — a + * running container, or a Pod that is neither terminating nor ended — that + * the route's registry does not hold: a peer replica's create, or compute + * this spawner lost track of (a restart before adoption). Nothing was + * touched. The route answers it as a duplicate, so the caller adopts the + * session through acquire instead of treating the create as failed and + * tearing down what runs under the id. */ +export class SessionExistsError extends Error { + constructor(sessionId: string, detail: string, options?: ErrorOptions) { + super(`session ${sessionId} already exists (${detail})`, options); + this.name = 'SessionExistsError'; + } +} + export interface SweepOptions { /** Reap runtimes whose start time is older than this epoch-ms threshold. */ staleBeforeMs: number; diff --git a/services/sandbox/src/devices/hub.test.ts b/services/sandbox/src/devices/hub.test.ts index 08084ae694..aebddadb58 100644 --- a/services/sandbox/src/devices/hub.test.ts +++ b/services/sandbox/src/devices/hub.test.ts @@ -772,6 +772,50 @@ describe('DeviceHub placement', () => { ]); }); + // The cleanup after a failed create stops a session and keeps its + // workspace on the device: its id must still reach that device. + test('a stop that kept the workspace leaves the placement as it was', async () => { + const { hub } = await makeHub(); + const device = connectDevice(hub, 'dev-1', (stream) => + stream.respond( + { status: stream.head.method === 'DELETE' ? 200 : 201, headers: [] }, + new Response( + JSON.stringify( + stream.head.method === 'DELETE' + ? { stopped: true, busy: false, workspaceKept: true } + : {}, + ), + ).body, + ), + ); + await device.hello(); + const create = createRequest('pa-kept', 'device'); + await (await hub.maybeForward(create.req, create.url, create.body))?.text(); + const stop = callRequest( + 'DELETE', + '/v1/sessions/pa-kept?if_idle=1&keep_workspace=1', + ); + const answer = await hub.maybeForward(stop.req, stop.url, ''); + expect(await answer?.json()).toEqual({ + stopped: true, + busy: false, + workspaceKept: true, + }); + expect( + device.served.some( + (call) => + call.method === 'DELETE' && + call.path === '/v1/sessions/pa-kept?if_idle=1&keep_workspace=1', + ), + ).toBe(true); + expect(hub.capacityOverlay(ORG).placements).toEqual([ + { sessionId: 'pa-kept', deviceId: 'dev-1' }, + ]); + // The next call for the id still goes to the device. + const again = callRequest('GET', '/v1/sessions/pa-kept'); + expect(await hub.maybeForward(again.req, again.url, '')).not.toBeNull(); + }); + test('a destroyed session keeps its route to the device until the device confirms the bytes are gone', async () => { const { hub } = await makeHub(); // What the device answers each destroy: an older device's answer with no diff --git a/services/sandbox/src/devices/hub.ts b/services/sandbox/src/devices/hub.ts index cac23b4626..2037b9c125 100644 --- a/services/sandbox/src/devices/hub.ts +++ b/services/sandbox/src/devices/hub.ts @@ -669,12 +669,14 @@ export class DeviceHub { const text = await res.text(); let busy = false; let deleted = false; + let kept = false; try { const parsed: unknown = JSON.parse(text); if (parsed !== null && typeof parsed === 'object') { busy = Reflect.get(parsed, 'busy') === true; const deletion: unknown = Reflect.get(parsed, 'deletion'); deleted = deletion === 'done' || deletion === 'handed_off'; + kept = Reflect.get(parsed, 'workspaceKept') === true; } } catch (err) { console.warn('[sandbox.devices] unreadable destroy answer:', err); @@ -682,9 +684,11 @@ export class DeviceHub { } // `?if_idle=1` on a busy session destroyed nothing, and a create for the // same id forwarded while the destroy ran made the session anew there: - // the placement stays as it is. Otherwise no session lives on the device - // any more, but its workspace's bytes may: only an explicit completion - // lets go of the route. A device still deleting (`pending`, `failed`) — + // the placement stays as it is. So does a stop that kept the workspace + // (`?keep_workspace=1`): the device still holds it for the id's next + // session. Otherwise no session lives on the device any more, but its + // workspace's bytes may: only an explicit completion lets go of the + // route. A device still deleting (`pending`, `failed`) — // or one older than the `deletion` contract, whose answer says nothing // about the bytes — keeps it, marked deleting, so the destroy that asks // again reaches the device holding them. @@ -695,7 +699,7 @@ export class DeviceHub { const recreated = this.creates.get(sessionId) !== createsBefore || this.placements.get(sessionId) !== placementBefore; - if (!busy && !recreated) { + if (!busy && !kept && !recreated) { if (deleted) { await this.placements.delete(sessionId); this.creates.delete(sessionId); diff --git a/services/sandbox/src/server.test.ts b/services/sandbox/src/server.test.ts index 8d3fc15233..c69454679e 100644 --- a/services/sandbox/src/server.test.ts +++ b/services/sandbox/src/server.test.ts @@ -245,6 +245,62 @@ describe('session HTTP routes', () => { } }); + test('a destroy passes keep_workspace=1 on, and only that', async () => { + const destroy = spyOn( + SessionRoutes.prototype, + 'handleDestroy', + ).mockImplementation(async () => + Response.json({ stopped: true, busy: false, workspaceKept: true }), + ); + try { + for (const query of ['?if_idle=1&keep_workspace=1', '?if_idle=1']) { + const path = `/v1/sessions/sess1${query}`; + const timestamp = String(Date.now()); + const nonce = crypto.randomUUID(); + const response = await router( + new Request(`http://sandbox${path}`, { + method: 'DELETE', + headers: { + [SIGNATURE_HEADER]: sign( + 'DELETE', + path, + timestamp, + '', + 'route-test-secret', + nonce, + ), + [TIMESTAMP_HEADER]: timestamp, + [NONCE_HEADER]: nonce, + }, + }), + ); + expect(response.status).toBe(200); + } + expect(destroy.mock.calls).toEqual([ + [ + 'sess1', + { + ifIdle: true, + ifStopped: false, + awaitDeletion: false, + keepWorkspace: true, + }, + ], + [ + 'sess1', + { + ifIdle: true, + ifStopped: false, + awaitDeletion: false, + keepWorkspace: false, + }, + ], + ]); + } finally { + destroy.mockRestore(); + } + }); + test('checkpoint GET and PUT pass the authenticated router with a bounded body', async () => { const checkpoint = spyOn( SessionRoutes.prototype, diff --git a/services/sandbox/src/server.ts b/services/sandbox/src/server.ts index 6cbadd6638..fe1374e7ee 100644 --- a/services/sandbox/src/server.ts +++ b/services/sandbox/src/server.ts @@ -430,11 +430,14 @@ async function handleSessionRoutes( // — the workspace cleanup's: no-op while ANY compute runs under the id. // `?await_deletion=1` — the cleanup's too: wait a bounded time for the // workspace's bytes, and answer how far their deletion came. + // `?keep_workspace=1` — remove the compute alone and keep the + // workspace (the cleanup after a failed create of an agent session). // The query string is HMAC-covered (authorize signs pathname + search). return getSessionRoutes().handleDestroy(id, { ifIdle: url.searchParams.get('if_idle') === '1', ifStopped: url.searchParams.get('if_stopped') === '1', awaitDeletion: url.searchParams.get('await_deletion') === '1', + keepWorkspace: url.searchParams.get('keep_workspace') === '1', }); } } diff --git a/services/sandbox/src/session/session-routes.test.ts b/services/sandbox/src/session/session-routes.test.ts index d05e89b525..cff63da58f 100644 --- a/services/sandbox/src/session/session-routes.test.ts +++ b/services/sandbox/src/session/session-routes.test.ts @@ -19,6 +19,7 @@ import { getEventListeners } from 'node:events'; import { ActivityGate } from '../../../sandbox-runtime/daemon/src/activity-gate.ts'; import { + SessionExistsError, SessionIncarnationChangedError, type BackendSession, type SessionBackend, @@ -532,6 +533,7 @@ let backendCheckThrows = false; // Sessions whose backend destroy fails (a wedged dockerd) — destroySession // throws for these so the route's honesty path (no laundered success) is tested. const backendDestroyThrows = new Set(); +const backendStopThrows = new Set(); // Sessions whose create is a RESUME (the workspace pre-existed backend-side). const resumedSessions = new Set(); // The backend's durable pin record (what a restart would re-adopt). @@ -568,6 +570,9 @@ const fakeBackend: SessionBackend = { return had; }, async stopSession(sessionId: string) { + if (backendStopThrows.has(sessionId)) { + throw new Error('backend stop failed (wedged dockerd)'); + } // Stop releases compute but PRESERVES the workspace — never marks destroyed. const had = created.has(sessionId); stopped.add(sessionId); @@ -626,6 +631,7 @@ async function readSse(res: Response): Promise<{ events: SseEvent[] }> { // into another test's assertions). fakeServer/fakeBaseUrl stay (beforeAll-owned). beforeEach(() => { created.clear(); + backendStopThrows.clear(); destroyed.clear(); stopped.clear(); stdinWrites.length = 0; @@ -1664,6 +1670,39 @@ describe('SessionRoutes (fake runnerd)', () => { expect(again.status).toBe(409); }); + // The platform adopts a 409 and cleans up after a 502: a live session the + // registry lost must read as the former, or its compute is removed. + test('a create that finds a live unregistered session → 409 duplicate; a failed create → 502', async () => { + const routes = new SessionRoutes(cfg, { + ...fakeBackend, + async createSession(spec) { + if (spec.sessionId === 'live-unregistered') { + throw new SessionExistsError( + spec.sessionId, + 'container tale-sbx-ses-live-unregistered is running', + ); + } + throw new Error( + 'docker run (session) failed: Conflict. The container name is already in use', + ); + }, + }); + const live = await routes.handleCreate( + JSON.stringify({ sessionId: 'live-unregistered', organizationId: 'o' }), + ); + expect(live.status).toBe(409); + expect(await live.json()).toMatchObject({ error: 'duplicate' }); + const failed = await routes.handleCreate( + JSON.stringify({ sessionId: 'not-live', organizationId: 'o' }), + ); + expect(failed.status).toBe(502); + expect(await failed.json()).toMatchObject({ error: 'create_failed' }); + for (const id of ['live-unregistered', 'not-live']) { + expect(destroyed.has(id)).toBe(false); + expect(stopped.has(id)).toBe(false); + } + }); + test('env / files / content / attach round-trip through runnerd', async () => { const routes = new SessionRoutes(cfg, fakeBackend); await routes.handleCreate( @@ -2392,6 +2431,82 @@ describe('SessionRoutes (fake runnerd)', () => { }); }); + // The platform's cleanup after a failed create of an agent session: the + // id may name a workspace preserved for the owner's next turn, so only the + // compute goes. + describe('conditional stop that keeps the workspace (ifIdle + keepWorkspace)', () => { + test('idle: the compute is stopped, the workspace kept, and the answer says so', async () => { + const routes = new SessionRoutes(cfg, fakeBackend); + await routes.handleCreate( + JSON.stringify({ sessionId: 'keep1', organizationId: 'org_keep' }), + ); + fakeHealth.liveExecs = 0; + const res = await routes.handleDestroy('keep1', { + ifIdle: true, + keepWorkspace: true, + }); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ + stopped: true, + busy: false, + workspaceKept: true, + }); + expect(stopped.has('keep1')).toBe(true); + expect(destroyed.has('keep1')).toBe(false); + expect((await routes.handleGet('keep1')).status).toBe(404); + }); + + test('busy: nothing is stopped', async () => { + const routes = new SessionRoutes(cfg, fakeBackend); + await routes.handleCreate( + JSON.stringify({ sessionId: 'keep2', organizationId: 'org_keep' }), + ); + fakeHealth.liveExecs = 1; + const res = await routes.handleDestroy('keep2', { + ifIdle: true, + keepWorkspace: true, + }); + expect(await res.json()).toEqual({ destroyed: false, busy: true }); + expect(stopped.has('keep2')).toBe(false); + expect(destroyed.has('keep2')).toBe(false); + }); + + test('no compute under the id: nothing stopped, and the workspace is never destroyed', async () => { + const routes = new SessionRoutes(cfg, fakeBackend); + backendGone.add('dead-keep3'); + const res = await routes.handleDestroy('dead-keep3', { + ifIdle: true, + keepWorkspace: true, + }); + expect(await res.json()).toEqual({ + stopped: false, + busy: false, + workspaceKept: true, + }); + expect(destroyed.has('dead-keep3')).toBe(false); + }); + + test('a failed backend stop answers 502 and keeps the registry entry', async () => { + const routes = new SessionRoutes(cfg, fakeBackend); + await routes.handleCreate( + JSON.stringify({ sessionId: 'keep4', organizationId: 'org_keep' }), + ); + fakeHealth.liveExecs = 0; + backendStopThrows.add('keep4'); + const res = await routes.handleDestroy('keep4', { + ifIdle: true, + keepWorkspace: true, + }); + expect(res.status).toBe(502); + expect(await res.json()).toMatchObject({ + destroyed: false, + error: 'backend stop failed', + }); + expect((await routes.handleGet('keep4')).status).toBe(200); + expect(destroyed.has('keep4')).toBe(false); + }); + }); + describe('zombie-session eviction', () => { test('a delayed dead probe cannot evict a replacement or join its liveness check', async () => { const started = Promise.withResolvers(); diff --git a/services/sandbox/src/session/session-routes.ts b/services/sandbox/src/session/session-routes.ts index 344b9ff566..8cad0c8444 100644 --- a/services/sandbox/src/session/session-routes.ts +++ b/services/sandbox/src/session/session-routes.ts @@ -4,6 +4,7 @@ // lifecycle and runnerd addressing; runnerd owns the actual exec. import { + SessionExistsError, SessionIncarnationChangedError, type BackendSession, type CreateSessionResult, @@ -1897,6 +1898,15 @@ export class SessionRoutes { signal: operationSignal(), }); } catch (err) { + // A live session the registry does not hold already runs under the + // id: a duplicate, which the caller adopts through acquire. As a + // failed create, its cleanup would remove that session's compute. + if (err instanceof SessionExistsError) { + return jsonResponse( + { error: 'duplicate', message: err.message }, + 409, + ); + } return jsonResponse( { error: 'create_failed', @@ -2423,6 +2433,7 @@ export class SessionRoutes { ifIdle?: boolean; ifStopped?: boolean; awaitDeletion?: boolean; + keepWorkspace?: boolean; } = {}, ): Promise { // A destroyed session asks for no room any more. @@ -2442,6 +2453,12 @@ export class SessionRoutes { // resumed, before its first exec — or a create in flight means someone // came back to it, and the cleanup must leave it alone. // + // `?keep_workspace=1` removes the compute alone (`backend.stopSession`) + // and keeps the workspace: the platform's cleanup after a failed create + // of an agent session, whose id may name a workspace preserved for its + // next turn. Deleting a workspace nothing owns is the workspace + // cleanup's, through a plain destroy. + // // Either condition also refuses while a create of the id is in flight: // the create is laying out the very workspace this would delete. The // destroy is announced BEFORE that check, so a create admitted while the @@ -2463,7 +2480,10 @@ export class SessionRoutes { this.creating.has(sessionId); if (busy) return jsonResponse({ destroyed: false, busy: true }, 200); } - const outcome = await this.destroyNow(sessionId); + const outcome = await this.destroyNow( + sessionId, + opts.keepWorkspace === true, + ); if (outcome instanceof Response) return outcome; destroyed = outcome.destroyed; } finally { @@ -2472,6 +2492,14 @@ export class SessionRoutes { this.destroySettled.delete(sessionId); } } + // Nothing was deleted: the answer says the workspace stays, so a device + // hub keeps routing the id to the machine holding it. + if (opts.keepWorkspace === true) { + return jsonResponse( + { stopped: destroyed, busy: false, workspaceKept: true }, + 200, + ); + } // Out of use is not deleted: `deletion` says how far the workspace's // bytes came, on every answer that is not busy — an erasure or a // retirement settles only on an explicit `done` (or Kubernetes' @@ -2512,9 +2540,11 @@ export class SessionRoutes { } /** The destroy itself: whether it reached anything under the id, or the - * 502 a failed backend destroy answers. */ + * 502 a failed backend destroy answers. `keepWorkspace` stops the session + * instead — its compute removed, its workspace kept. */ private async destroyNow( sessionId: string, + keepWorkspace = false, ): Promise { // Delete from the registry BEFORE awaiting the backend so a concurrent // destroy of the same id sees an empty cache and can't double-call @@ -2527,7 +2557,9 @@ export class SessionRoutes { // the incarnation and follow the deterministic id onto a later resume. this.forgetReclaimMarks(sessionId); try { - const backendExisted = await this.backend.destroySession(sessionId); + const backendExisted = keepWorkspace + ? await this.backend.stopSession(sessionId) + : await this.backend.destroySession(sessionId); this.unregistered.delete(sessionId); return { destroyed: had || backendExisted }; } catch (err) { @@ -2537,11 +2569,12 @@ export class SessionRoutes { // user's data lives on — the "success toast, workspace survives" defect. // Restore the registry entry so the session isn't lost, and surface the // failure so the caller retries. - console.error('[sandbox.session] destroy backend failed:', err); - reportSandboxError(err, 'session-destroy'); + const verb = keepWorkspace ? 'stop' : 'destroy'; + console.error(`[sandbox.session] ${verb} backend failed:`, err); + reportSandboxError(err, `session-${verb}`); if (entry !== undefined) this.registry.set(entry); return jsonResponse( - { destroyed: false, busy: false, error: 'backend destroy failed' }, + { destroyed: false, busy: false, error: `backend ${verb} failed` }, 502, ); }