diff --git a/services/platform/tests/manual/reference/automation.md b/services/platform/tests/manual/reference/automation.md index 067335cbff..dcb793b1ed 100644 --- a/services/platform/tests/manual/reference/automation.md +++ b/services/platform/tests/manual/reference/automation.md @@ -465,6 +465,7 @@ Legend: โœ… fully automated ยท ๐Ÿ”ถ partially automated ยท โ›” manual-only (no s | [settings](../suites/settings.md) | The first-come line for host room: a create refused for room waits by session id in the order of its first refusal; freed room goes to the oldest waiters still asking (a slot each, and memory for their working sets, before a newer create โ€” none for a waiter the host could never fit); each 429 carries `queue: { position, waiting }` and a `retry-after` of 5 s plus 5 s a place, up to 60 s; asking again keeps a waiter's place, one that stops asking gives it up after twice its hint and 15 s, a destroy takes it out, and a restart starts with no line | โœ… automated | `services/sandbox/src/session/session-routes.test.ts` (`the first-come line for host room`, and the capacity and memory admission tests) | | [settings](../suites/settings.md) | Idle sandbox reclamation under deployment capacity pressure | โœ… automated | `services/sandbox/src/session/session-routes.test.ts` (released compute reclaimed before admission, persistent workspace retained, busy/pinned/unknown/legacy runtime protection, delayed release after reacquire, concurrent creates and stop failure); `services/sandbox-runtime/daemon/src/activity-gate.test.ts` (atomic idle claim, active-operation protection and frozen execution gate); `services/sandbox/src/backend/docker/docker-session-backend.test.ts`, `backend/kubernetes/k8s-session-backend.test.ts` (incarnation-fenced compute removal and retained workspace); `backend/domains/sandbox/idle-release.test.ts` (ticket capture, transactional queueing and committed-state checks), `idle-release.integration.ts` (28 real-Postgres checks: organization/allocation/owner guards, admission and release ordering, atomic cancellation and release-job rollback, post-op retry on an already-stopped workflow); `backend/core/node_only/sandbox/agent_session.test.ts` (`ensureAgentSession` refuses a false allocation resume before acquiring runtime compute) | | [settings](../suites/settings.md) | The session sweep frees what nothing uses: a released session stops after the short idle window (Docker-in-sandbox agent sessions keep the full one), a running session whose daemon has not answered five sweeps is stopped (a sweep that cannot probe it starts the count over), ended containers and Pods are removed by their incarnation with the workspace kept, beside the API and several at a time, a create of the id waits for that removal (bounded) and a session registered under the id is neither held up by it nor counted as freed by it, a pass still running is joined, a burst of creates at capacity reclaims one released session each, idle-longest first, a create refused at capacity probes at most eight daemons, sessions last seen released first, and none that answered busy (to a reclaim probe or the sweep) or was acquired within 15 s unless released since, and the refusals in the 3 s after a walk that found nothing are answered at once, the build-cache reconcile after adoption covers agent sessions without holding adoption up, a slow build-cache job never holds the next pass's sweep up (one job at a time, organizations adopted meanwhile reconciled right after it), and a caller hanging up on an attach costs no backend check | โœ… automated | `services/sandbox/src/session/session-routes.test.ts` (`sweep and adoption hygiene`, `creates refused at capacity do not probe every busy session`), `services/sandbox/src/backend/docker/docker-session-backend.test.ts` (a failed inner image volume removal is reported) | +| [settings](../suites/settings.md) | A turn spends no backend check on a session its runnerd vouches for: a Docker session's container carries its creation stamp to runnerd, which names it in `/healthz` and in every activity answer and refuses with 409 `incarnation_mismatch`, before anything changes, an activity request (release ticket, acquire, release, reclaim, pin) meant for another incarnation; once runnerd has named the registered incarnation (the create's readiness answer included), acquire, release ticket and release skip the backend's existence check (`docker inspect`), a session read and the sweep of a pinned session ask runnerd's `/healthz` instead, and a replacement answering under the name is evicted without one (workspace kept, durable pin untouched); the backend is still asked when runnerd fails or names no incarnation (an older runtime image, or a replacement launched without a stamp, checked after the fact), and always on Kubernetes, where a terminating Pod still answers | โœ… automated | `services/sandbox/src/session/session-routes.test.ts` (`runnerd answers naming the session incarnation`), `services/sandbox/src/session/runnerd-client.test.ts` (`session incarnation`), `services/sandbox/src/session/docker-session-args.test.ts`, `services/sandbox/src/backend/docker/docker-session-create-calls.test.ts` (`the create reports the incarnation its readiness answer named`), `services/sandbox-runtime/daemon/src/incarnation.test.ts`, `services/sandbox-runtime/daemon/src/main.test.ts` (`activity answers name the incarnation`) | | [settings](../suites/settings.md) | Failed sandbox starts leave nothing behind: a fresh agent session whose create fails asks the spawner to destroy what it left before its row reads `failed` (best-effort, logged, the create's error kept; a failed resume never destroys the standing workspace), and the sandbox watchdog collects every `failed` row that destroy missed once past its grace โ€” destroyed only when idle, never while a newer or live incarnation carries the reused session id, stamped `destroyed_at_ms` by row id with the status kept `failed` | โœ… backend + real Postgres | `backend/core/node_only/sandbox/agent_session.test.ts`, `backend/domains/sandbox/watchdogs.test.ts`, `backend/integration-check.ts` (`checkWatchdogs`); an empty `docker ps -a --filter name=tale-sbx --filter status=created` on a real host after a failed start remains a deployment check | | [settings](../suites/settings.md) / [governance](../suites/governance.md) | Sandbox workspace cleanup (the logic half of `SET-F54` and `SET-F69`โ€“`SET-F72`, `SET-B28`, `GOV-F49`): one retire path re-decides under the organization's admission lock, ends the live incarnation (`expired`, unpinned on both sides, gateway keys revoked) before the spawner deletes and settles only the rows it claimed after โ€” a fresh incarnation started meanwhile stays live; a workspace resumed after it was judged stays; the verdict โ€” a deleted agent's workspace, or one of runs a departed person started, goes even pinned or warm (never a member's or a schedule's), one in use, pinned or with a queued run stays, a live agent's goes once unused past the `sandbox_workspaces` window and never before a full window since that rule took effect (the upgrade, turning it on, a shorter window) or since its unpin, an ended run's after the watchdog's reclaim window, a render's or legacy owner's as an orphan; an organization or custodian hold keeps what it covers; a member's removal (every door, through the membership cascade) and a user's erasure (strict, whatever runs) delete their workspaces; an agent's or project's deletion queues its agents' workspaces in the deleting transaction; an organization's deletion reads its workspaces, gateway keys and devices before the cascade and revokes keys first, releases devices only once every slice's workspaces are gone (a healed project-agent workspace included), then tears the organization down on the spawner; the hourly sweep's inventory deletes stopped workspaces no live row owns, and of those no row names only this deployment's (a render's, ours, a deleted organization's โ€” never another deployment's), with deletions and attempts per pass bounded; a deletion settles (rows destroyed, audited, counted by an erasure) only on an explicit completion โ€” `deletion: done`, or Kubernetes' `handed_off`, recorded in the audit metadata โ€” and one still being deleted, failing to delete, or answered without a deletion state by a spawner older than the contract (19776cf18's) is deferred by the sweep, throws for the owner's and the organization's jobs and fails the erasure's pass (receipt partial, Retry settles it once explicit), and a failing one is audited as a failure; the Sandboxes list dates each unused workspace's deletion. Spawner side: every destroy answers `deletion` โ€” read from the Docker trash (`pending` until the bytes go, `failed` after a failed attempt, never `done` across a restart, a fresh workspace under the id never counted), `handed_off` from Kubernetes โ€” `await_deletion` retrying and waiting a bounded time, and the hub keeping a destroyed session's placement, marked deleting and out of the capacity view, until its device confirms (an older device's answer without a state included), asking the device again itself at most every 5 minutes, least recently asked first, so a placement nobody destroys or creates again ends once its bytes are gone (destroy finalization compares both the captured create generation and immutable placement after pending creates settle, so an ask dispatched before or during a durable create write cannot delete or mark the new route deleting), and a fresh create placed with the normal fallbacks but trying that device first; the workspace inventory in both layouts and on Kubernetes (PVCs joined with Pods, organization annotation), `if_stopped` refusing any compute or a create in flight, destroys of one id serialized and a create waiting for one, a stopped workspace naming its organization (`.owners/` marker), a deleted organization's leftover sessions and stopped workspaces, build helpers (by id, ownership-checked, egress detached before the network goes) and package caches, a create's wait for a destroy bounded | โœ… backend + real Postgres | `backend/domains/sandbox/workspace-cleanup.test.ts`, `backend/domains/sandbox/workspace-cleanup.integration.ts` (`backend:integration`, lane `checkWorkspaceCleanup`), `backend/domains/sandbox/routes.test.ts`, `backend/auth/membership.cascade.test.ts`, `backend/auth/auth.member-removal.test.ts`, `backend/domains/scim/service.test.ts`, `backend/domains/governance/routes.test.ts` (the rule recorded with the policy's save), `backend/domains/projects/service.agents.test.ts`, `backend/domains/projects/service.delete.test.ts`, `backend/domains/organizations/service.test.ts`, `services/sandbox/src/session/session-routes.test.ts`, `services/sandbox/src/session/workspace-inventory.test.ts`, `services/sandbox/src/backend/docker/docker-workspaces.test.ts`, `services/sandbox/src/backend/docker/docker-session-backend.test.ts`, `services/sandbox/src/session/workspace-trash.test.ts`, `services/sandbox/src/devices/hub.test.ts`, `services/sandbox/src/devices/placements.test.ts`, `backend/core/node_only/sandbox/helpers/session_client.test.ts`, `services/sandbox/src/backend/docker/docker-session-build-cache.test.ts`, `services/sandbox/src/backend/kubernetes/k8s-session-backend.test.ts`, `backend/core/sandbox/session_naming.test.ts`, `app/features/settings/sandboxes/workspace-cleanup-editor.test.tsx` (defaults without a policy file, the saved payload, the disabled days field, the schema's exact bounds, one localized failure toast), `app/features/settings/sandboxes/sandboxes-settings.test.tsx` (the dated row, the editor above Workspaces, no editor or policy read for Developers), `app/lib/backend/settings-sandbox.test.ts` (a `sandbox_workspaces` save refreshes the workspace list); a real spawner deleting a directory or PVC, a device coming back online, and the page in a browser stay manual | | [settings](../suites/settings.md) | The sandbox TTL expiry never cuts a live turn: a session past its window keeps its status, slot and credentials while one of its `running` ops shows a sign of life inside the recovery staleness window (the re-attach sweeps' rule, `sessionOpLastSignOfLifeMs` against `RECOVERY_STALE_MS`); an op silent past that window or already finished spares nothing, and the first tick after the turn ends expires the session | โœ… backend + real Postgres | `backend/domains/sandbox/watchdogs.test.ts`, `backend/integration-check.ts` (`checkWatchdogs`) | diff --git a/services/sandbox-runtime/README.md b/services/sandbox-runtime/README.md index a1dfa936fc..482f31e040 100644 --- a/services/sandbox-runtime/README.md +++ b/services/sandbox-runtime/README.md @@ -424,3 +424,9 @@ checks daemon liveness; `/readyz` also checks requested Docker capability. `/healthz` reuses that Docker snapshot and adds bounded, coalesced egress diagnostics when configured. Docker-disabled sessions remain ready; health failures do not hide activity or prevent file access and exec cancellation. +A container launched with `TALE_RUNNERD_INCARNATION` (the spawner's creation +stamp) names it as `incarnation` in `/healthz` and in every activity answer, +and refuses with `409 incarnation_mismatch` an activity request whose +`x-tale-runnerd-incarnation` header names another stamp, before anything +changes. A malformed stamp is never named; without one, answers name none and +requests are not checked. diff --git a/services/sandbox-runtime/daemon/src/incarnation.test.ts b/services/sandbox-runtime/daemon/src/incarnation.test.ts new file mode 100644 index 0000000000..2a29f4340f --- /dev/null +++ b/services/sandbox-runtime/daemon/src/incarnation.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, spyOn, test } from 'bun:test'; + +import { namesOtherIncarnation, readIncarnation } from './incarnation.ts'; + +describe('readIncarnation', () => { + test('a launch without a stamp names no incarnation', () => { + expect(readIncarnation(undefined)).toBeUndefined(); + expect(readIncarnation('')).toBeUndefined(); + }); + + test('a creation stamp is named as given', () => { + expect(readIncarnation('1760000000000')).toBe('1760000000000'); + expect(readIncarnation('0')).toBe('0'); + }); + + test.each(['1760000000000.5', '-1', ' 1760000000000', 'abc', '1'.repeat(17)])( + 'a malformed stamp %p is never named', + (raw) => { + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect(readIncarnation(raw)).toBeUndefined(); + expect(warn).toHaveBeenCalledTimes(1); + } finally { + warn.mockRestore(); + } + }, + ); +}); + +describe('namesOtherIncarnation', () => { + test('only a request naming another stamp to a daemon that has one is refused', () => { + expect(namesOtherIncarnation('1760000000000', '1760000000001')).toBe(true); + expect(namesOtherIncarnation('1760000000000', ['1', '1760000000000'])).toBe( + true, + ); + expect(namesOtherIncarnation('1760000000000', '1760000000000')).toBe(false); + expect(namesOtherIncarnation('1760000000000', undefined)).toBe(false); + expect(namesOtherIncarnation('1760000000000', '')).toBe(false); + expect(namesOtherIncarnation(undefined, '1760000000001')).toBe(false); + }); +}); diff --git a/services/sandbox-runtime/daemon/src/incarnation.ts b/services/sandbox-runtime/daemon/src/incarnation.ts new file mode 100644 index 0000000000..68ceaa4f53 --- /dev/null +++ b/services/sandbox-runtime/daemon/src/incarnation.ts @@ -0,0 +1,36 @@ +// The session incarnation this daemon serves: the creation stamp the spawner +// records on the backend object (Docker's `tale.created` label) and passes in +// at launch. Answers name it, so the spawner proves which incarnation answered +// without asking its backend; an activity request meant for another one is +// refused before it changes anything. + +import { RUNNERD_INCARNATION_ENV } from './protocol.ts'; + +/** A creation stamp is a millisecond epoch, written as a decimal integer. */ +const STAMP_RE = /^[0-9]{1,16}$/; + +/** The stamp from the launch environment, or undefined when there is none. A + * malformed value proves nothing about the incarnation, so it is never named. */ +export function readIncarnation(raw: string | undefined): string | undefined { + if (raw === undefined || raw === '') return undefined; + if (STAMP_RE.test(raw)) return raw; + console.warn( + `[runnerd] ${RUNNERD_INCARNATION_ENV} is not a creation stamp; answers name no incarnation`, + ); + return undefined; +} + +/** Does a request name an incarnation other than the one served? A request + * naming none, or a daemon launched without a stamp, cannot tell. */ +export function namesOtherIncarnation( + served: string | undefined, + named: string | string[] | undefined, +): boolean { + const value = Array.isArray(named) ? named[0] : named; + return ( + served !== undefined && + value !== undefined && + value !== '' && + value !== served + ); +} diff --git a/services/sandbox-runtime/daemon/src/main.test.ts b/services/sandbox-runtime/daemon/src/main.test.ts index 44421e7ec7..bc4e2313c0 100644 --- a/services/sandbox-runtime/daemon/src/main.test.ts +++ b/services/sandbox-runtime/daemon/src/main.test.ts @@ -22,9 +22,11 @@ import { InnerDockerHealth } from './inner-docker-health.ts'; const workspace = realpathSync(mkdtempSync(`${tmpdir()}/runnerd-http-`)); const token = 'runnerd-http-test-token'; +const incarnation = '1760000000000'; const previous = { workspace: process.env.TALE_WORKSPACE_ROOT, token: process.env.TALE_RUNNERD_TOKEN, + incarnation: process.env.TALE_RUNNERD_INCARNATION, browser: process.env.TALE_BROWSER_CDP, }; let server: Server; @@ -33,6 +35,7 @@ let baseUrl: string; beforeAll(async () => { process.env.TALE_WORKSPACE_ROOT = workspace; process.env.TALE_RUNNERD_TOKEN = token; + process.env.TALE_RUNNERD_INCARNATION = incarnation; // An old deployment's leftover env cannot revive the retired stack. process.env.TALE_BROWSER_CDP = '1'; ({ server } = await import('./main.ts')); @@ -51,6 +54,7 @@ afterAll(async () => { for (const [name, value] of [ ['TALE_WORKSPACE_ROOT', previous.workspace], ['TALE_RUNNERD_TOKEN', previous.token], + ['TALE_RUNNERD_INCARNATION', previous.incarnation], ['TALE_BROWSER_CDP', previous.browser], ] as const) { if (value === undefined) delete process.env[name]; @@ -206,6 +210,7 @@ describe('runnerd HTTP service', () => { expect(await response.json()).toEqual({ ok: true, bootedAtMs: expect.any(Number), + incarnation, lastActivityAtMs: expect.any(Number), liveExecs: 0, activity: { @@ -353,10 +358,10 @@ describe('runnerd HTTP service', () => { expect(activeOperations).toBe(1); expect( (await activityPost('/release', await releaseTicket())).value, - ).toEqual({ released: false }); + ).toEqual({ released: false, incarnation }); expect( (await activityPost('/reclaim', { claimId: 'upload' })).value, - ).toEqual({ claimed: false }); + ).toEqual({ claimed: false, incarnation }); } finally { upload.end('}'); } @@ -383,10 +388,10 @@ describe('runnerd HTTP service', () => { await fetched.promise; expect( (await activityPost('/release', await releaseTicket())).value, - ).toEqual({ released: false }); + ).toEqual({ released: false, incarnation }); expect( (await activityPost('/reclaim', { claimId: 'staging' })).value, - ).toEqual({ claimed: false }); + ).toEqual({ claimed: false, incarnation }); complete.resolve(); expect((await staging).value).toEqual({ staged: [{ path: 'pressure.txt', bytes: 9 }], @@ -792,6 +797,57 @@ describe('runnerd HTTP service', () => { }, ); + test('activity answers name the incarnation, and a request meant for another is refused before it changes anything', async () => { + const before = await releaseTicket(); + expect(before).toEqual({ generation: expect.any(String), incarnation }); + const other = { ...headers, 'x-tale-runnerd-incarnation': '1' }; + for (const [method, path, body] of [ + ['GET', '/release', undefined], + ['POST', '/acquire', undefined], + ['POST', '/release', { generation: before.generation }], + ['POST', '/pin', { pinned: true }], + [ + 'POST', + '/reclaim', + { claimId: 'replacement', generation: before.generation }, + ], + ] as const) { + const refused = await fetch(`${baseUrl}${path}`, { + method, + headers: other, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), + }); + expect(refused.status).toBe(409); + expect(await refused.json()).toEqual({ + error: 'incarnation_mismatch', + incarnation, + }); + } + const health = record( + await (await fetch(`${baseUrl}/healthz`, { headers })).json(), + ); + expect(health.incarnation).toBe(incarnation); + expect(record(health.activity)).toMatchObject({ + generation: before.generation, + pinned: false, + reclaiming: false, + }); + const own = await fetch(`${baseUrl}/acquire`, { + method: 'POST', + headers: { ...headers, 'x-tale-runnerd-incarnation': incarnation }, + }); + expect(own.status).toBe(200); + const acquired = record(await own.json()); + expect(acquired).toEqual({ generation: expect.any(String), incarnation }); + expect(acquired.generation).not.toBe(before.generation); + // Whether it releases depends on work earlier tests left in flight; the + // answer names the incarnation either way. + expect( + (await activityPost('/release', { generation: acquired.generation })) + .value, + ).toEqual({ released: expect.any(Boolean), incarnation }); + }); + // KEEP LAST: the claim below freezes the one daemon this file shares โ€” // by design a successful claim never expires โ€” so every request a later // test would make answers 503 `reclaiming`. @@ -800,6 +856,7 @@ describe('runnerd HTTP service', () => { expect((await activityPost('/acquire')).status).toBe(200); expect((await activityPost('/release', stale)).value).toEqual({ released: false, + incarnation, }); const current = await releaseTicket(); for (const idleBeforeMs of [null, '0', -1, 1.5]) { @@ -819,20 +876,21 @@ describe('runnerd HTTP service', () => { idleBeforeMs: 0, }) ).value, - ).toEqual({ claimed: false }); + ).toEqual({ claimed: false, incarnation }); expect((await activityPost('/release', current)).value).toEqual({ released: true, + incarnation, }); expect((await activityPost('/pin', { pinned: true })).status).toBe(200); expect( (await activityPost('/reclaim', { claimId: 'pressure' })).value, - ).toEqual({ claimed: false }); + ).toEqual({ claimed: false, incarnation }); expect((await activityPost('/pin', { pinned: false })).status).toBe(200); expect( (await activityPost('/reclaim', { claimId: 'pressure' })).value, - ).toEqual({ claimed: true }); + ).toEqual({ claimed: true, incarnation }); expect((await activityPost('/reclaim', { claimId: 'peer' })).value).toEqual( - { claimed: true }, + { claimed: true, incarnation }, ); for (const [path, body] of [ ['/acquire', {}], diff --git a/services/sandbox-runtime/daemon/src/main.ts b/services/sandbox-runtime/daemon/src/main.ts index 6b0c485057..2d31e99e00 100644 --- a/services/sandbox-runtime/daemon/src/main.ts +++ b/services/sandbox-runtime/daemon/src/main.ts @@ -36,6 +36,7 @@ import { type StageItem, } from './file-ops.ts'; import { readJsonBody } from './http-body.ts'; +import { namesOtherIncarnation, readIncarnation } from './incarnation.ts'; import { InnerDockerHealth, LAZY_DOCKER_HEALTH_SOCKET, @@ -44,6 +45,8 @@ import { RUNNERD_CHECKPOINT_MAX_BYTES, parseRunnerdSequence, RUNNERD_CONSUMER_BUFFER_MAX_BYTES, + RUNNERD_INCARNATION_ENV, + RUNNERD_INCARNATION_HEADER, RUNNERD_MAX_LIVE_EXECS, RUNNERD_PORT, RUNNERD_TOKEN_HEADER, @@ -58,6 +61,12 @@ const MAX_STAGING_OPERATIONS = 2; let stagingOperations = 0; const TOKEN = process.env.TALE_RUNNERD_TOKEN ?? ''; +// Named in /healthz and every activity answer, so the spawner can trust an +// answer as coming from the incarnation it registered without asking the +// backend. Empty for a container launched without a stamp. +const INCARNATION = readIncarnation(process.env[RUNNERD_INCARNATION_ENV]); +const incarnation = + INCARNATION === undefined ? {} : { incarnation: INCARNATION }; const innerDocker = new InnerDockerHealth(process.env.TALE_DIND === '1', { socketPath: LAZY_DOCKER_HEALTH_SOCKET, supervisor: true, @@ -120,6 +129,21 @@ function sendJson(res: ServerResponse, status: number, body: unknown): void { res.end(payload); } +/** An activity request meant for another incarnation of this session reached + * this one (a replacement under the same name): refuse it before it changes + * anything, naming the incarnation served here. */ +function refusedForOtherIncarnation( + req: IncomingMessage, + res: ServerResponse, +): boolean { + if ( + !namesOtherIncarnation(INCARNATION, req.headers[RUNNERD_INCARNATION_HEADER]) + ) + return false; + sendJson(res, 409, { error: 'incarnation_mismatch', ...incarnation }); + return true; +} + function isObject(v: unknown): v is Record { return v !== null && typeof v === 'object' && !Array.isArray(v); } @@ -392,6 +416,7 @@ async function router( const body: Record = { ok: true, bootedAtMs, + ...incarnation, lastActivityAtMs, liveExecs: execManager.liveCount(), activity: activity.snapshot(), @@ -402,10 +427,15 @@ async function router( return; } if (req.method === 'GET' && path === '/release') { - sendJson(res, 200, { generation: activity.snapshot().generation }); + if (refusedForOtherIncarnation(req, res)) return; + sendJson(res, 200, { + generation: activity.snapshot().generation, + ...incarnation, + }); return; } if (req.method === 'POST' && path === '/acquire') { + if (refusedForOtherIncarnation(req, res)) return; if ((await innerDocker.snapshot()).dockerReady === false) { sendJson(res, 503, { error: 'docker_unavailable' }); return; @@ -414,7 +444,9 @@ async function router( sendJson( res, generation === null ? 503 : 200, - generation === null ? { error: 'reclaiming' } : { generation }, + generation === null + ? { error: 'reclaiming' } + : { generation, ...incarnation }, ); return; } @@ -427,6 +459,7 @@ async function router( sendJson(res, body.status, { error: body.error }); return; } + if (refusedForOtherIncarnation(req, res)) return; if (!isObject(body.value)) { sendJson(res, 400, { error: 'bad_request' }); return; @@ -440,7 +473,7 @@ async function router( sendJson( res, applied ? 200 : 503, - applied ? { ok: true } : { error: 'reclaiming' }, + applied ? { ok: true, ...incarnation } : { error: 'reclaiming' }, ); return; } @@ -473,13 +506,14 @@ async function router( res, 200, path === '/release' - ? { released: activity.release(token) } + ? { released: activity.release(token), ...incarnation } : { claimed: activity.claim( token, String(body.value.generation), typeof idleBeforeMs === 'number' ? idleBeforeMs : undefined, ), + ...incarnation, }, ); return; diff --git a/services/sandbox-runtime/daemon/src/protocol.ts b/services/sandbox-runtime/daemon/src/protocol.ts index 4019d7f97b..082635dca1 100644 --- a/services/sandbox-runtime/daemon/src/protocol.ts +++ b/services/sandbox-runtime/daemon/src/protocol.ts @@ -8,6 +8,16 @@ export const RUNNERD_PORT = 8200; export const RUNNERD_TOKEN_HEADER = 'x-tale-runnerd-token'; export const RUNNERD_TOKEN_CONTEXT = 'runnerd-v1:'; +/** Environment variable carrying the session's creation stamp (the value of + * the backend object's `tale.created` label) into the container. runnerd names + * it in /healthz and in its activity answers, so the spawner can tell the + * incarnation it registered from a replacement under the same name without + * asking the backend. A container launched without it names none. */ +export const RUNNERD_INCARNATION_ENV = 'TALE_RUNNERD_INCARNATION'; +/** Request header naming the incarnation an activity request is meant for. + * runnerd refuses the request with 409 `incarnation_mismatch` (naming its own) + * when it serves another, before anything changes. */ +export const RUNNERD_INCARNATION_HEADER = 'x-tale-runnerd-incarnation'; export const RUNNERD_MAX_LIVE_EXECS = 4; /** Per-consumer in-flight write ceiling. A slow/stalled (but still attached) @@ -54,6 +64,9 @@ export interface RunnerdHealth { /** Sustained probe failure or observed terminal Docker state; permits fenced idle recovery. */ dockerRecoveryRequired?: boolean; bootedAtMs: number; + /** The creation stamp the container was launched with (see + * RUNNERD_INCARNATION_ENV); absent when it was launched without one. */ + incarnation?: string; lastActivityAtMs: number; liveExecs: number; /** Optional dependency diagnostics; do not affect daemon liveness. */ diff --git a/services/sandbox-runtime/entrypoint.sh b/services/sandbox-runtime/entrypoint.sh index e63e30741f..2a12707f03 100644 --- a/services/sandbox-runtime/entrypoint.sh +++ b/services/sandbox-runtime/entrypoint.sh @@ -22,6 +22,7 @@ # HTTPS_PROXY / HTTP_PROXY -> http://sandbox-egress:3128 # TALE_DIND / TALE_TRANSPARENT_EGRESS -> feature signals # TALE_RUNNERD_TOKEN / TALE_SESSION_ENV -> runnerd auth + seed env +# TALE_RUNNERD_INCARNATION -> creation stamp runnerd names in its answers # # Conventions: # - The session workspace is /agent (host bind / PVC). HOME and the diff --git a/services/sandbox/docs/sessions.md b/services/sandbox/docs/sessions.md index f927a06b14..eaf3a6d926 100644 --- a/services/sandbox/docs/sessions.md +++ b/services/sandbox/docs/sessions.md @@ -178,6 +178,26 @@ nowhere. `SANDBOX_TOKEN` is required (the spawner refuses to boot without it โ€” `loadConfig` fails closed), so every session carries a real token and runnerd always verifies; there is no unsigned mode. +The Docker backend also launches each session container with +`TALE_RUNNERD_INCARNATION` set to its creation stamp, the value of its +`tale.created` label; the env patch route refuses the reserved `TALE_RUNNERD_` +prefix, so session code cannot rename it. runnerd names that stamp as +`incarnation` in `/healthz` and in every activity answer (release ticket, +acquire, release, reclaim, pin). An activity request whose +`x-tale-runnerd-incarnation` header names another stamp is refused with +`409 incarnation_mismatch`, naming runnerd's own, before anything changes, so a +replacement under the session's name is never acquired, released, claimed or +pinned for a stale registry entry. Once runnerd has named the registered stamp +(in the create's readiness answer, an activity answer or a sweep probe), +acquire, release ticket and release skip the backend's `sessionExists` check (a +`docker inspect`), and a session read asks `/healthz` instead. The sweep of a +pinned session asks `/healthz` first and needs no backend check when the answer +names the registered stamp; both probes are bounded at 1.5 s. An answer naming +another stamp evicts the stale entry and keeps its workspace. A failed runnerd +call, or an answer naming no stamp (an older runtime image), falls back to the +backend check. Kubernetes keeps the backend check throughout: a terminating Pod +still answers through its IP after the backend counts it gone. + Image warming runs beside control startup and session adoption. While a cold runtime image is being pulled, new local creates return `429 runtime_image` with `Retry-After: 5`; health, limits and existing-session operations remain @@ -213,7 +233,10 @@ metadata and endpoint together. Late probes and cleanup from the old incarnation cannot launch an exec or clear the replacement's activity or exec state. Liveness checks distinguish the registered incarnation from another running object under its name, while an -unreadable identity remains unknown. Linger stops also use the creation fence. +unreadable identity remains unknown. On Docker a runnerd answer naming the +registered creation stamp counts as such a check, and one naming another stamp +shows the registered incarnation is gone. Linger stops also use the creation +fence. A maintenance pass every minute (`SessionRoutes.maintain`: adoption, then the reaper `sweepExpired`; a pass still running is joined, never stacked) **stops**: diff --git a/services/sandbox/src/backend/docker/docker-session-backend.ts b/services/sandbox/src/backend/docker/docker-session-backend.ts index b3b6273c9f..aa0c52b936 100644 --- a/services/sandbox/src/backend/docker/docker-session-backend.ts +++ b/services/sandbox/src/backend/docker/docker-session-backend.ts @@ -44,7 +44,10 @@ import { runnerdEnvPatch, runnerdHealth, } from '../../session/runnerd-client.ts'; -import { RUNNERD_PORT } from '../../session/runnerd-protocol.ts'; +import { + RUNNERD_PORT, + type RunnerdHealth, +} from '../../session/runnerd-protocol.ts'; import { belongsToInstance, deriveRunnerdToken, @@ -508,9 +511,10 @@ export class DockerSessionBackend implements SessionBackend { // Poll runnerd until ready; on failure tear down only this attempt's // container and preserve any workspace files already written. + let readiness: RunnerdHealth; try { const baseUrl = await this.resolveEndpoint(spec.sessionId); - await this.waitForRunnerdOrExit( + readiness = await this.waitForRunnerdOrExit( containerName, { baseUrl, token }, this.cfg.session.createHealthTimeoutMs, @@ -549,7 +553,15 @@ export class DockerSessionBackend implements SessionBackend { await this.cleanupCreateAttempt(spec.sessionId, createAttemptId); throw err; } - return { resumed: preexisting }; + // The readiness answer came through the container of the session's name, + // launched just above with this create's stamp: the incarnation it names + // spares the session's first activity call a `docker inspect`. + return { + resumed: preexisting, + ...(readiness.incarnation === undefined + ? {} + : { incarnation: readiness.incarnation }), + }; } /** A failed create owns only the container carrying its random attempt @@ -603,6 +615,7 @@ export class DockerSessionBackend implements SessionBackend { * container's last log lines in the error instead. A null status (daemon * hiccup) is "unknown", never a death verdict; only a definitively dead * container (exited/dead โ€” isReapableContainerStatus) aborts the wait. + * Returns the answer that found it ready. */ private async waitForRunnerdOrExit( containerName: string, @@ -613,7 +626,7 @@ export class DockerSessionBackend implements SessionBackend { // inspected for an early exit only every fifth miss. pollIntervalMs = 100, signal?: AbortSignal, - ): Promise { + ): Promise { const start = Date.now(); for (let miss = 1; ; miss += 1) { try { @@ -621,7 +634,7 @@ export class DockerSessionBackend implements SessionBackend { signal?.throwIfAborted(); if (health.dockerReady === false) throw new Error('runnerd is live but inner Docker is not ready'); - return; + return health; } catch { signal?.throwIfAborted(); const status = diff --git a/services/sandbox/src/backend/docker/docker-session-create-calls.test.ts b/services/sandbox/src/backend/docker/docker-session-create-calls.test.ts index af82605a20..d35c30af07 100644 --- a/services/sandbox/src/backend/docker/docker-session-create-calls.test.ts +++ b/services/sandbox/src/backend/docker/docker-session-create-calls.test.ts @@ -7,10 +7,13 @@ async function createTwo(plant: { dirs?: string[]; files?: string[]; dockerInitiallyUnavailable?: boolean; + /** The incarnation runnerd's readiness answers name, if any. */ + incarnation?: string; }): Promise<{ calls: string[][][]; error: string | null; healthChecks: number; + results: Array<{ resumed: boolean; incarnation?: string }>; }> { const sourceRoot = resolve(import.meta.dir, '../..'); const script = ` @@ -34,7 +37,7 @@ mock.module(spawnPath, () => ({...realSpawn, }, })); mock.module(join(source,'session/runnerd-client.ts'), () => ({ - runnerdHealth: async () => ({dockerReady: !(++healthChecks === 1 && planted.dockerInitiallyUnavailable)}), + runnerdHealth: async () => ({dockerReady: !(++healthChecks === 1 && planted.dockerInitiallyUnavailable), ...(planted.incarnation === undefined ? {} : {incarnation: planted.incarnation})}), runnerdEnvPatch: async () => [], })); const {DockerSessionBackend} = await import(join(source,'backend/docker/docker-session-backend.ts')); @@ -50,16 +53,17 @@ const cfg = { }; const backend = new DockerSessionBackend(cfg); const perCreate = []; +const results = []; let error = null; try { for (const sessionId of ['calls-a','calls-b']) { calls = []; - await backend.createSession({sessionId,organizationId:'org-calls',profile:'agent',env:{},createdAtMs:0,ttlMs:1000,idleTimeoutMs:1000}); + results.push(await backend.createSession({sessionId,organizationId:'org-calls',profile:'agent',env:{},createdAtMs:0,ttlMs:1000,idleTimeoutMs:1000})); perCreate.push(calls); } } catch (e) { error = e.message; } await rm(root,{recursive:true,force:true}); -console.log(JSON.stringify({calls:perCreate,error,healthChecks})); +console.log(JSON.stringify({calls:perCreate,error,healthChecks,results})); `; const child = Bun.spawn([process.execPath, '-e', script], { stdout: 'pipe', @@ -87,6 +91,18 @@ describe('what a session create asks the docker daemon', () => { expect(error).toBeNull(); expect(healthChecks).toBe(3); }); + test('the create reports the incarnation its readiness answer named', async () => { + const named = await createTwo({ incarnation: '0' }); + expect(named.error).toBeNull(); + expect(named.results).toEqual([ + { resumed: false, incarnation: '0' }, + { resumed: false, incarnation: '0' }, + ]); + // An older runtime image names none, and the create reports none. + const unnamed = await createTwo({}); + expect(unnamed.error).toBeNull(); + expect(unnamed.results).toEqual([{ resumed: false }, { resumed: false }]); + }); test('the cache volumes once per organization, and no legacy mount lookup on a flat root', async () => { const { calls, error } = await createTwo({}); expect(error).toBeNull(); diff --git a/services/sandbox/src/backend/types.ts b/services/sandbox/src/backend/types.ts index af079eb427..7c7c29aeb0 100644 --- a/services/sandbox/src/backend/types.ts +++ b/services/sandbox/src/backend/types.ts @@ -159,6 +159,14 @@ export interface CreateSessionResult { * the half-made workspace it provisioned itself. */ resumed: boolean; + /** + * The incarnation runnerd named in the readiness answer this create waited + * for (see RUNNERD_INCARNATION_ENV): the route layer records it like any + * later runnerd answer, so a fresh or resumed session's first activity call + * needs no backend existence check. Absent when runnerd named none (an + * older runtime image, or a backend that launches without the stamp). + */ + incarnation?: string; } /** diff --git a/services/sandbox/src/session/docker-session-args.test.ts b/services/sandbox/src/session/docker-session-args.test.ts index 646ce66b36..0c92ca19e4 100644 --- a/services/sandbox/src/session/docker-session-args.test.ts +++ b/services/sandbox/src/session/docker-session-args.test.ts @@ -249,6 +249,9 @@ describe('buildDockerSessionRunArgs', () => { ); // Runnerd token in env. expect(args).toContain(`TALE_RUNNERD_TOKEN=${'a'.repeat(64)}`); + // runnerd names the incarnation it serves: the `tale.created` stamp. + expect(args).toContain('tale.created=1700000000000'); + expect(args).toContain('TALE_RUNNERD_INCARNATION=1700000000000'); // Container + workspace mount. expect(args).toContain('tale-sbx-ses-ses-abc-123'); expect(args).toContain( diff --git a/services/sandbox/src/session/docker-session-args.ts b/services/sandbox/src/session/docker-session-args.ts index 13413eef16..ceab906a45 100644 --- a/services/sandbox/src/session/docker-session-args.ts +++ b/services/sandbox/src/session/docker-session-args.ts @@ -21,6 +21,7 @@ import { } from '../runtime-tier.ts'; import type { SessionAgentProfileConfig, SpawnerConfig } from '../types.ts'; import type { SandboxSessionProfile } from '../wire.ts'; +import { RUNNERD_INCARNATION_ENV } from './runnerd-protocol.ts'; import { SESSION_INSTANCE_LABEL, sessionContainerName, @@ -439,6 +440,11 @@ export function buildDockerSessionRunArgs( // required, so deriveRunnerdToken has something to derive from. '--env', `TALE_RUNNERD_TOKEN=${inp.runnerdToken}`, + // The incarnation runnerd names in its answers: the `tale.created` stamp + // above, so a runnerd answer can prove this container is the one the + // spawner registered without a `docker inspect`. + '--env', + `${RUNNERD_INCARNATION_ENV}=${inp.createdAtMs}`, // DinD signal + tier for the entrypoint (empty when DinD is off). ...dindEnv, // Transparent egress signal + drop-uid for the entrypoint (empty when off). diff --git a/services/sandbox/src/session/runnerd-client.test.ts b/services/sandbox/src/session/runnerd-client.test.ts index dcaa6b8503..51a95a4a17 100644 --- a/services/sandbox/src/session/runnerd-client.test.ts +++ b/services/sandbox/src/session/runnerd-client.test.ts @@ -11,6 +11,9 @@ import { rejects } from 'node:assert/strict'; import { withOperationBudget } from '../operation-budget.ts'; import { + answeringIncarnation, + RunnerdActivityError, + runnerdActivity, runnerdAttach, runnerdExec, runnerdReadFile, @@ -890,3 +893,81 @@ test('replay delivery awaits downstream consumers before parsing later events', await pumping; expect(seen).toEqual([1, 2]); }); + +describe('session incarnation', () => { + test('an activity request names the incarnation it is meant for', async () => { + const named: Array = []; + const daemon = Bun.serve({ + port: 0, + fetch(request) { + named.push(request.headers.get('x-tale-runnerd-incarnation')); + return Response.json({ generation: 'g1', incarnation: '1700' }); + }, + }); + try { + const target = { baseUrl: daemon.url.origin, token: 'test' }; + expect( + await runnerdActivity({ ...target, incarnation: 1700 }, 'acquire'), + ).toEqual({ generation: 'g1', incarnation: '1700' }); + await runnerdActivity(target, 'ticket'); + expect(named).toEqual(['1700', null]); + } finally { + await daemon.stop(true); + } + }); + + test('a refusal by another incarnation names the one that answered', async () => { + const answers = [ + Response.json( + { error: 'incarnation_mismatch', incarnation: '1800' }, + { status: 409 }, + ), + Response.json({ error: 'conflict' }, { status: 409 }), + new Response('not json', { status: 409 }), + Response.json({ error: 'reclaiming' }, { status: 503 }), + ]; + const daemon = Bun.serve({ + port: 0, + fetch: () => answers.shift() ?? new Response(null, { status: 500 }), + }); + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + const target = { + baseUrl: daemon.url.origin, + token: 'test', + incarnation: 1700, + }; + const failures: unknown[] = []; + for (let i = 0; i < 4; i += 1) + failures.push( + await runnerdActivity(target, 'release', { generation: 'g1' }).catch( + (error: unknown) => error, + ), + ); + expect(failures.every((f) => f instanceof RunnerdActivityError)).toBe( + true, + ); + expect(failures).toMatchObject([ + { status: 409, incarnation: '1800' }, + { status: 409, incarnation: undefined }, + { status: 409, incarnation: undefined }, + { status: 503, incarnation: undefined }, + ]); + } finally { + warn.mockRestore(); + await daemon.stop(true); + } + }); + + test.each([ + ['1700', 'registered'], + ['1800', 'replaced'], + [undefined, 'unnamed'], + ['', 'unnamed'], + [1700, 'unnamed'], + ['17e2', 'unnamed'], + ['12345678901234567', 'unnamed'], + ] as const)('an answer naming %p is %s', (named, expected) => { + expect(answeringIncarnation(1700, named)).toBe(expected); + }); +}); diff --git a/services/sandbox/src/session/runnerd-client.ts b/services/sandbox/src/session/runnerd-client.ts index e711cd3497..ca03983af3 100644 --- a/services/sandbox/src/session/runnerd-client.ts +++ b/services/sandbox/src/session/runnerd-client.ts @@ -9,6 +9,7 @@ import { operationSignal } from '../operation-budget.ts'; import { RUNNERD_CHECKPOINT_MAX_BYTES, isRunnerdExecEvent, + RUNNERD_INCARNATION_HEADER, RUNNERD_TOKEN_HEADER, type RunnerdExecEvent, type RunnerdExecRequest, @@ -20,6 +21,9 @@ interface RunnerdClientOptions { baseUrl: string; /** Per-session token (deriveRunnerdToken), or '' in unsigned dev mode. */ token: string; + /** Creation stamp of the incarnation an activity request is meant for: + * runnerd refuses the request when it serves another one. */ + incarnation?: number; } function authHeaders(token: string): Record { @@ -133,11 +137,54 @@ export class RunnerdActivityError extends Error { constructor( readonly status: number, path: string, + /** The incarnation runnerd serves, when it refused a request meant for + * another one. */ + readonly incarnation?: string, ) { super(`runnerd /${path} ${status}`); } } +/** The incarnation a 409 `incarnation_mismatch` refusal names, if it is one. */ +async function refusedIncarnation(res: Response): Promise { + if (res.status !== 409) return undefined; + let value: unknown; + try { + value = await res.json(); + } catch (error) { + console.warn('[sandbox.session] runnerd 409 answer is not JSON:', error); + return undefined; + } + if ( + value === null || + typeof value !== 'object' || + !('error' in value) || + value.error !== 'incarnation_mismatch' || + !('incarnation' in value) || + typeof value.incarnation !== 'string' + ) + return undefined; + return value.incarnation; +} + +/** A creation stamp as runnerd names it: a millisecond epoch in decimal. */ +const INCARNATION_RE = /^[0-9]{1,16}$/; + +/** Which incarnation a runnerd answer names, against the creation stamp of + * the one the caller registered: `registered` when it names that one, + * `replaced` when it names another (a replacement under the same name + * answered), `unnamed` when it names none โ€” an older runtime image, a + * container launched without the stamp, or a malformed value โ€” which proves + * nothing either way. */ +export function answeringIncarnation( + createdAtMs: number, + named: unknown, +): 'registered' | 'replaced' | 'unnamed' { + if (typeof named !== 'string' || !INCARNATION_RE.test(named)) + return 'unnamed'; + return named === String(createdAtMs) ? 'registered' : 'replaced'; +} + export async function runnerdActivity( opts: RunnerdClientOptions, action: 'ticket' | 'acquire' | 'release' | 'reclaim' | 'pin', @@ -149,11 +196,22 @@ export async function runnerdActivity( const path = action === 'ticket' ? 'release' : action; const res = await fetch(`${opts.baseUrl}/${path}`, { method: action === 'ticket' ? 'GET' : 'POST', - headers: { ...authHeaders(opts.token), 'content-type': 'application/json' }, + headers: { + ...authHeaders(opts.token), + 'content-type': 'application/json', + ...(opts.incarnation === undefined + ? {} + : { [RUNNERD_INCARNATION_HEADER]: String(opts.incarnation) }), + }, ...(body === undefined ? {} : { body: JSON.stringify(body) }), signal: AbortSignal.timeout(RUNNERD_HEALTH_TIMEOUT_MS), }); - if (!res.ok) throw new RunnerdActivityError(res.status, path); + if (!res.ok) + throw new RunnerdActivityError( + res.status, + path, + await refusedIncarnation(res), + ); const value: unknown = await res.json(); if (value === null || typeof value !== 'object' || Array.isArray(value)) { throw new Error(`runnerd /${path} invalid response`); diff --git a/services/sandbox/src/session/runnerd-protocol.ts b/services/sandbox/src/session/runnerd-protocol.ts index 4019d7f97b..082635dca1 100644 --- a/services/sandbox/src/session/runnerd-protocol.ts +++ b/services/sandbox/src/session/runnerd-protocol.ts @@ -8,6 +8,16 @@ export const RUNNERD_PORT = 8200; export const RUNNERD_TOKEN_HEADER = 'x-tale-runnerd-token'; export const RUNNERD_TOKEN_CONTEXT = 'runnerd-v1:'; +/** Environment variable carrying the session's creation stamp (the value of + * the backend object's `tale.created` label) into the container. runnerd names + * it in /healthz and in its activity answers, so the spawner can tell the + * incarnation it registered from a replacement under the same name without + * asking the backend. A container launched without it names none. */ +export const RUNNERD_INCARNATION_ENV = 'TALE_RUNNERD_INCARNATION'; +/** Request header naming the incarnation an activity request is meant for. + * runnerd refuses the request with 409 `incarnation_mismatch` (naming its own) + * when it serves another, before anything changes. */ +export const RUNNERD_INCARNATION_HEADER = 'x-tale-runnerd-incarnation'; export const RUNNERD_MAX_LIVE_EXECS = 4; /** Per-consumer in-flight write ceiling. A slow/stalled (but still attached) @@ -54,6 +64,9 @@ export interface RunnerdHealth { /** Sustained probe failure or observed terminal Docker state; permits fenced idle recovery. */ dockerRecoveryRequired?: boolean; bootedAtMs: number; + /** The creation stamp the container was launched with (see + * RUNNERD_INCARNATION_ENV); absent when it was launched without one. */ + incarnation?: string; lastActivityAtMs: number; liveExecs: number; /** Optional dependency diagnostics; do not affect daemon liveness. */ diff --git a/services/sandbox/src/session/session-routes.test.ts b/services/sandbox/src/session/session-routes.test.ts index d05e89b525..63d823f504 100644 --- a/services/sandbox/src/session/session-routes.test.ts +++ b/services/sandbox/src/session/session-routes.test.ts @@ -108,6 +108,12 @@ const deadDaemons = new Set(); const healthProbes = new Map(); // Per-daemon activity clocks (by session token), over fakeHealth's shared one. const daemonLastActivity = new Map(); +// Daemons (by session token) launched with a creation stamp: they name it in +// their answers and refuse an activity request meant for another incarnation, +// as runnerd does. The rest name none, like an older runtime image. +const daemonIncarnations = new Map(); +// The incarnation each activity request named, in arrival order. +const activityIncarnations: Array<{ path: string; named: string | null }> = []; function ndjson( lines: Array<{ t: string; seq?: number; [key: string]: unknown }>, @@ -177,6 +183,17 @@ beforeAll(() => { if (deadDaemons.has(token)) { return new Response('runnerd is gone', { status: 503 }); } + const served = daemonIncarnations.get(token); + const incarnation = served === undefined ? {} : { incarnation: served }; + if (['/acquire', '/release', '/reclaim', '/pin'].includes(url.pathname)) { + const named = req.headers.get('x-tale-runnerd-incarnation'); + activityIncarnations.push({ path: url.pathname, named }); + if (served !== undefined && named !== null && named !== served) + return Response.json( + { error: 'incarnation_mismatch', ...incarnation }, + { status: 409 }, + ); + } if (url.pathname === '/healthz') { healthProbes.set(token, (healthProbes.get(token) ?? 0) + 1); const dockerReady = fakeHealth.dockerReady; @@ -187,6 +204,7 @@ beforeAll(() => { return Response.json({ ok: true, bootedAtMs: 0, + ...incarnation, lastActivityAtMs: daemonLastActivity.get(token) ?? fakeHealth.lastActivityAtMs, liveExecs: fakeHealth.liveExecs, @@ -205,7 +223,7 @@ beforeAll(() => { ); const generation = activity.acquire(); return Response.json( - { generation }, + { generation, ...incarnation }, { status: generation === null ? 503 : 200 }, ); } @@ -214,7 +232,10 @@ beforeAll(() => { url.pathname === '/release' && req.method === 'GET' ) { - return Response.json({ generation: activity.snapshot().generation }); + return Response.json({ + generation: activity.snapshot().generation, + ...incarnation, + }); } if ( !legacyDaemon && @@ -228,6 +249,7 @@ beforeAll(() => { if (url.pathname === '/release') return Response.json({ released: activity.release(String(body.generation)), + ...incarnation, }); if (url.pathname === '/reclaim') { const idleBeforeMs = @@ -242,10 +264,14 @@ beforeAll(() => { String(body.generation), idleBeforeMs, ), + ...incarnation, }); } const applied = activity.setPinned(body.pinned === true); - return Response.json({ ok: applied }, { status: applied ? 200 : 503 }); + return Response.json( + { ok: applied, ...incarnation }, + { status: applied ? 200 : 503 }, + ); } if (activity.snapshot().reclaiming) return new Response('reclaiming', { status: 503 }); @@ -651,6 +677,8 @@ beforeEach(() => { deadDaemons.clear(); healthProbes.clear(); daemonLastActivity.clear(); + daemonIncarnations.clear(); + activityIncarnations.length = 0; }); describe('SessionRoutes (fake runnerd)', () => { @@ -7067,3 +7095,427 @@ test('cancelling a backpressured HTTP replay detaches its upstream producer', as await proxy.stop(true); } }, 10000); + +describe('runnerd answers naming the session incarnation', () => { + const tokenOf = (sessionId: string) => + deriveRunnerdToken(cfg.sandboxToken, sessionId); + + /** A Docker backend whose containers carry their creation stamp to runnerd + * (docker-session-args), counting its existence checks. Its create reports + * the incarnation its readiness answer named, as DockerSessionBackend does. */ + function stampedBackend( + checks: string[], + overrides: Partial = {}, + ): SessionBackend { + return { + ...fakeBackend, + async createSession(spec) { + daemonIncarnations.set( + tokenOf(spec.sessionId), + String(spec.createdAtMs), + ); + return { + ...(await fakeBackend.createSession(spec)), + incarnation: String(spec.createdAtMs), + }; + }, + async sessionExists(sessionId, expected) { + checks.push(sessionId); + return fakeBackend.sessionExists(sessionId, expected); + }, + ...overrides, + }; + } + + async function provenSession( + routes: SessionRoutes, + sessionId: string, + checks: string[], + ): Promise { + expect( + ( + await routes.handleCreate( + JSON.stringify({ sessionId, organizationId: 'org_inc' }), + ) + ).status, + ).toBe(201); + // The create's readiness answer named the incarnation, so even the first + // acquire asks the backend nothing. + expect((await routes.handleActivity(sessionId, 'acquire')).status).toBe( + 200, + ); + expect(checks).toEqual([]); + activityIncarnations.length = 0; + const stamp = daemonIncarnations.get(tokenOf(sessionId)); + if (stamp === undefined) throw new Error('the create named no stamp'); + return stamp; + } + + test('a turn on a proven session forks no backend check', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + const stamp = await provenSession(routes, 'inc-turn', checks); + + expect((await routes.handleGet('inc-turn')).status).toBe(200); + const acquired = await routes.handleActivity('inc-turn', 'acquire'); + expect(acquired.status).toBe(200); + // oxlint-disable-next-line typescript-eslint/no-unsafe-type-assertion + const { generation } = (await acquired.json()) as { generation: string }; + expect((await routes.handleActivity('inc-turn', 'ticket')).status).toBe( + 200, + ); + const released = await routes.handleActivity( + 'inc-turn', + 'release', + JSON.stringify({ generation }), + ); + expect(await released.json()).toEqual({ released: true }); + + expect(checks).toEqual([]); + expect(activityIncarnations).toEqual([ + { path: '/acquire', named: stamp }, + { path: '/release', named: stamp }, + { path: '/release', named: stamp }, + ]); + }); + + test('a create proves its session: the ticket and release after it fork no backend check', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + // A resume as the platform's agent flow runs it: the acquire missed, so + // it creates and goes straight to the release ticket. + expect( + ( + await routes.handleCreate( + JSON.stringify({ + sessionId: 'inc-created', + organizationId: 'org_inc', + }), + ) + ).status, + ).toBe(201); + const ticket = await routes.handleActivity('inc-created', 'ticket'); + expect(ticket.status).toBe(200); + expect( + await ( + await routes.handleActivity( + 'inc-created', + 'release', + JSON.stringify(await ticket.json()), + ) + ).json(), + ).toEqual({ released: true }); + expect(checks).toEqual([]); + }); + + test('an older runtime naming no incarnation keeps the backend check on every call', async () => { + const checks: string[] = []; + // Its readiness answer names none either, so the create proves nothing. + const routes = new SessionRoutes( + cfg, + stampedBackend(checks, { + createSession: (spec) => fakeBackend.createSession(spec), + }), + ); + await routes.handleCreate( + JSON.stringify({ sessionId: 'inc-legacy', organizationId: 'org_inc' }), + ); + + expect((await routes.handleActivity('inc-legacy', 'acquire')).status).toBe( + 200, + ); + expect((await routes.handleGet('inc-legacy')).status).toBe(200); + expect((await routes.handleActivity('inc-legacy', 'ticket')).status).toBe( + 200, + ); + expect(checks).toEqual(['inc-legacy', 'inc-legacy', 'inc-legacy']); + }); + + test('a replacement under the name refuses the request and the stale entry is evicted', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + const stamp = await provenSession(routes, 'inc-replaced', checks); + const activity = fakeActivities.get(tokenOf('inc-replaced')); + const before = activity?.snapshot().generation; + daemonIncarnations.set(tokenOf('inc-replaced'), String(Number(stamp) + 1)); + + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect( + (await routes.handleActivity('inc-replaced', 'acquire')).status, + ).toBe(404); + } finally { + warn.mockRestore(); + } + // Refused before it changed anything; the replacement is not stopped or + // destroyed, only the stale entry goes. + expect(activity?.snapshot().generation).toBe(before); + expect(routes.holds('inc-replaced')).toBe(false); + expect(stopped.has('inc-replaced')).toBe(false); + expect(destroyed.has('inc-replaced')).toBe(false); + expect(checks).toEqual([]); + }); + + test('a proven session answering without a stamp gets the skipped check after all', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + await provenSession(routes, 'inc-unstamped', checks); + // A replacement launched without a stamp now holds the name. + daemonIncarnations.delete(tokenOf('inc-unstamped')); + backendGone.add('inc-unstamped'); + + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect( + (await routes.handleActivity('inc-unstamped', 'ticket')).status, + ).toBe(404); + } finally { + warn.mockRestore(); + } + expect(checks).toEqual(['inc-unstamped']); + expect(routes.holds('inc-unstamped')).toBe(false); + }); + + test('a proven session read asks runnerd, and the backend only when runnerd fails', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + await provenSession(routes, 'inc-read', checks); + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect((await routes.handleGet('inc-read')).status).toBe(200); + expect(checks).toEqual([]); + + // runnerd fails: the backend decides โ€” alive keeps the entry... + deadDaemons.add(tokenOf('inc-read')); + expect((await routes.handleGet('inc-read')).status).toBe(200); + expect(checks).toEqual(['inc-read']); + // ...and gone evicts it. + backendGone.add('inc-read'); + expect((await routes.handleGet('inc-read')).status).toBe(404); + expect(routes.holds('inc-read')).toBe(false); + } finally { + warn.mockRestore(); + } + }); + + test('a proven session read answered by a replacement evicts without a backend check', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + const stamp = await provenSession(routes, 'inc-read-replaced', checks); + daemonIncarnations.set( + tokenOf('inc-read-replaced'), + String(Number(stamp) + 1), + ); + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect((await routes.handleGet('inc-read-replaced')).status).toBe(404); + } finally { + warn.mockRestore(); + } + expect(checks).toEqual([]); + expect(routes.holds('inc-read-replaced')).toBe(false); + }); + + test('the sweep takes a pinned session runnerd names as live and asks the backend only when runnerd fails', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + await routes.handleCreate( + JSON.stringify({ sessionId: 'inc-pinned', organizationId: 'org_inc' }), + ); + expect( + ( + await routes.handleSetPinned( + 'inc-pinned', + JSON.stringify({ pinned: true }), + ) + ).status, + ).toBe(200); + checks.length = 0; + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect(await routes.sweepExpired()).toBe(0); + expect(checks).toEqual([]); + + deadDaemons.add(tokenOf('inc-pinned')); + expect(await routes.sweepExpired()).toBe(0); + expect(checks).toEqual(['inc-pinned']); + expect(routes.holds('inc-pinned')).toBe(true); + + backendGone.add('inc-pinned'); + expect(await routes.sweepExpired()).toBe(1); + expect(routes.holds('inc-pinned')).toBe(false); + expect(destroyed.has('inc-pinned')).toBe(false); + } finally { + warn.mockRestore(); + } + }); + + test('the sweep asks the backend for a pinned session whose runnerd names no incarnation', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + await routes.handleCreate( + JSON.stringify({ + sessionId: 'inc-pinned-old', + organizationId: 'org_inc', + }), + ); + await routes.handleSetPinned( + 'inc-pinned-old', + JSON.stringify({ pinned: true }), + ); + daemonIncarnations.clear(); + checks.length = 0; + expect(await routes.sweepExpired()).toBe(0); + expect(checks).toEqual(['inc-pinned-old']); + }); + + test('the sweep evicts an unpinned session a replacement answers for, stopping nothing', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + await routes.handleCreate( + JSON.stringify({ + sessionId: 'inc-sweep-replaced', + organizationId: 'org_inc', + }), + ); + const token = tokenOf('inc-sweep-replaced'); + const stamp = daemonIncarnations.get(token); + if (stamp === undefined) throw new Error('the create named no stamp'); + // Recently active, so nothing but the replacement can end the entry. + daemonLastActivity.set(token, Date.now()); + daemonIncarnations.set(token, String(Number(stamp) + 1)); + checks.length = 0; + + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect(await routes.sweepExpired()).toBe(1); + } finally { + warn.mockRestore(); + } + // The stale entry goes without a backend check; the replacement holding + // the name is neither stopped nor destroyed. + expect(routes.holds('inc-sweep-replaced')).toBe(false); + expect(checks).toEqual([]); + expect(stopped.has('inc-sweep-replaced')).toBe(false); + expect(destroyed.has('inc-sweep-replaced')).toBe(false); + }); + + test('the sweep proves an unpinned session runnerd names, so its next ticket forks no backend check', async () => { + const checks: string[] = []; + // Re-adopted after a spawner restart: registered from the backend's + // listing, so no runnerd answer has named its incarnation yet. + const createdAtMs = Date.now(); + const routes = new SessionRoutes( + cfg, + stampedBackend(checks, { + async listSessions(): Promise { + return [ + { + ...mkBackendSession('inc-sweep-proves', 'org_inc'), + createdAtMs, + ttlMs: 3_600_000, + idleTimeoutMs: 3_600_000, + }, + ]; + }, + }), + ); + const token = tokenOf('inc-sweep-proves'); + daemonIncarnations.set(token, String(createdAtMs)); + daemonLastActivity.set(token, Date.now()); + await routes.adoptExisting(); + expect(routes.holds('inc-sweep-proves')).toBe(true); + checks.length = 0; + + expect(await routes.sweepExpired()).toBe(0); + expect( + (await routes.handleActivity('inc-sweep-proves', 'ticket')).status, + ).toBe(200); + expect(checks).toEqual([]); + }); + + test('a pin refused by a replacement drops the stale entry and leaves the durable pin alone', async () => { + const checks: string[] = []; + const routes = new SessionRoutes(cfg, stampedBackend(checks)); + const stamp = await provenSession(routes, 'inc-pin-replaced', checks); + daemonIncarnations.set( + tokenOf('inc-pin-replaced'), + String(Number(stamp) + 1), + ); + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + expect( + ( + await routes.handleSetPinned( + 'inc-pin-replaced', + JSON.stringify({ pinned: true }), + ) + ).status, + ).toBe(503); + } finally { + warn.mockRestore(); + } + expect(backendPins.has('inc-pin-replaced')).toBe(false); + expect(routes.holds('inc-pin-replaced')).toBe(false); + }); + + test('a pressure reclaim refused by a replacement claims and stops nothing, and drops the stale entry', async () => { + const checks: string[] = []; + const routes = new SessionRoutes( + { ...cfg, session: { ...cfg.session, maxSessions: 1 } }, + stampedBackend(checks), + ); + const stamp = await provenSession(routes, 'inc-warm', checks); + const ticket: unknown = await ( + await routes.handleActivity('inc-warm', 'ticket') + ).json(); + expect( + await ( + await routes.handleActivity( + 'inc-warm', + 'release', + JSON.stringify(ticket), + ) + ).json(), + ).toEqual({ released: true }); + daemonIncarnations.set(tokenOf('inc-warm'), String(Number(stamp) + 1)); + + const warn = spyOn(console, 'warn').mockImplementation(() => {}); + try { + await routes.handleCreate( + JSON.stringify({ sessionId: 'inc-next', organizationId: 'org_inc' }), + ); + } finally { + warn.mockRestore(); + } + expect(activityIncarnations).toContainEqual({ + path: '/reclaim', + named: stamp, + }); + expect(fakeActivities.get(tokenOf('inc-warm'))?.snapshot().reclaiming).toBe( + false, + ); + expect(stopped.has('inc-warm')).toBe(false); + expect(routes.holds('inc-warm')).toBe(false); + }); + + test('Kubernetes keeps the backend check even when runnerd names the incarnation', async () => { + const checks: string[] = []; + const routes = new SessionRoutes( + { ...cfg, backend: 'kubernetes' }, + stampedBackend(checks, { kind: 'kubernetes' }), + ); + await routes.handleCreate( + JSON.stringify({ sessionId: 'inc-k8s', organizationId: 'org_inc' }), + ); + checks.length = 0; + expect((await routes.handleActivity('inc-k8s', 'acquire')).status).toBe( + 200, + ); + expect((await routes.handleGet('inc-k8s')).status).toBe(200); + expect((await routes.handleActivity('inc-k8s', 'acquire')).status).toBe( + 200, + ); + expect(checks).toEqual(['inc-k8s', 'inc-k8s', 'inc-k8s']); + }); +}); diff --git a/services/sandbox/src/session/session-routes.ts b/services/sandbox/src/session/session-routes.ts index 344b9ff566..de76953a46 100644 --- a/services/sandbox/src/session/session-routes.ts +++ b/services/sandbox/src/session/session-routes.ts @@ -42,6 +42,7 @@ import { RunnerdOutputGapError, RunnerdProtocolError, RunnerdStageBusyError, + answeringIncarnation, runnerdActivity, runnerdAttach, runnerdCancelExec, @@ -138,6 +139,12 @@ const SWEEP_CONCURRENCY = 8; * by a container nothing can use. */ const WEDGED_PROBE_FAILURES = 5; +/** How long a liveness read waits for runnerd to name its incarnation before + * asking the backend instead. A live daemon answers in milliseconds; one that + * vanished can hold a connect for seconds (a cached address of a removed + * container), and the platform's pre-turn probe must not wait that out. */ +const LIVENESS_PROBE_TIMEOUT_MS = 1_500; + /** How long removing an ended container/Pod that failed waits to be tried * again. */ const ENDED_REAP_BACKOFF_MS = 10 * 60_000; @@ -377,6 +384,16 @@ export class SessionRoutes { RegistrySession, Promise >(); + // Entries whose runnerd has named the incarnation they registered. Docker + // reaches runnerd only through the running container of the session's + // name, so such an answer proves what the backend's existence check would + // (a `docker inspect` fork, 30-60 ms and ~28 MB): their acquire, release + // ticket and release skip that check, since the request names the + // incarnation and runnerd refuses it when a replacement answers, and their + // session reads ask runnerd instead. Keyed by the entry, so a replacement + // starts unproven; an answer naming none (an older runtime image) leaves + // the entry on the backend's check. + private readonly namingIncarnation = new WeakSet(); // The maintenance pass in flight: a pass slower than its interval (hung // daemons, a slow dockerd) must not stack copies of itself. @@ -967,7 +984,11 @@ export class SessionRoutes { idle?: IdleReclaim, ): Promise { const sessionId = session.sessionId; - const opts = { baseUrl: session.endpoint, token: this.tokenFor(sessionId) }; + const opts = { + baseUrl: session.endpoint, + token: this.tokenFor(sessionId), + incarnation: session.createdAtMs, + }; const held = this.reclaimClaims.get(sessionId); if (held !== undefined && held.createdAtMs !== session.createdAtMs) { // Taken on an incarnation that has since left the registry; it must @@ -1023,7 +1044,10 @@ export class SessionRoutes { this.noteReclaimable(sessionId, false); return false; } - if (error instanceof SessionIncarnationChangedError) { + if ( + error instanceof SessionIncarnationChangedError || + this.refusedAsReplaced(session, error) + ) { // The backend object under this id is no longer the incarnation we // registered (a peer replica recreated it): our entry and claim are // stale, and the replacement is not ours to count as freed โ€” the @@ -1543,7 +1567,7 @@ export class SessionRoutes { this.probeFailures.delete(s.sessionId); // Always-on exempts live compute from idle stops, not confirmed-dead // backend objects from reconciliation. Unknown still stays held. - return this.evictIfBackendGone(s.sessionId); + return this.evictUnlessRunnerdNamesIt(s); } if (this.activating.has(s.sessionId)) return false; // A session with a live exec is NEVER reaped โ€” a long, QUIET tool (no @@ -1571,6 +1595,10 @@ export class SessionRoutes { token: this.tokenFor(s.sessionId), }); if (this.registry.get(s.sessionId) !== s) return false; + // A replacement under the session's name answered: what it reports is + // not this entry's to act on, and the entry's incarnation is gone. + if (this.noteIncarnation(s, health.incarnation) === 'replaced') + return this.evictStale(s, 'was replaced under its name'); this.probeFailures.delete(s.sessionId); s.lastActivityAtMs = health.lastActivityAtMs; // What the sweep saw spares a create at capacity a probe of its own. @@ -1726,7 +1754,8 @@ export class SessionRoutes { * errors instead of the definitive 404 its phantom self-heal keys on, so * every turn fails without recovery. * - * Called from runnerd-failure paths and the aliveness probe: verifies the + * Called from runnerd-failure paths, and wherever a runnerd answer cannot + * prove the registered incarnation (provesIncarnation): verifies the * backend object with the DEFINITIVE `sessionExists` check; on * confirmed-gone it evicts only the stale registry entry so this and * subsequent calls resolve to 404 โ†’ `SessionNotFoundError` โ†’ the platform @@ -1765,16 +1794,22 @@ export class SessionRoutes { ); return false; } - // The probe describes the entry captured before the await. A destroy / - // recreate or a stop that began meanwhile owns its new state and capacity. + return !alive && this.evictStale(session, 'backend object gone'); + } + + /** Drop a registry entry whose incarnation is confirmed gone, keeping its + * workspace for the resume; true when it did. The verdict describes the + * entry it was reached for: a destroy / recreate or a stop that began + * meanwhile owns its new state and capacity. */ + private evictStale(session: RegistrySession, reason: string): boolean { + const { sessionId } = session; if ( - alive || this.registry.get(sessionId) !== session || this.stopping.has(sessionId) ) return false; console.warn( - `[sandbox.session] ${sessionId} backend object gone; evicting stale registry entry (workspace preserved for resume)`, + `[sandbox.session] ${sessionId} ${reason}; evicting stale registry entry (workspace preserved for resume)`, ); this.registry.delete(sessionId); const unregistered = this.unregistered.get(sessionId); @@ -1787,6 +1822,72 @@ export class SessionRoutes { return true; } + /** Record which incarnation a runnerd answer named for this entry. Only an + * answer that names the registered one proves it; one naming none (an older + * runtime image, or a replacement launched without a stamp) returns the + * entry to the backend's check. */ + private noteIncarnation( + session: RegistrySession, + named: unknown, + ): ReturnType { + const answer = answeringIncarnation(session.createdAtMs, named); + if (answer === 'registered') this.namingIncarnation.add(session); + else this.namingIncarnation.delete(session); + return answer; + } + + /** Can a runnerd answer stand in for the backend's existence check of this + * entry? Only on Docker, where runnerd is reached through the running + * container of the session's name, and only once runnerd has named the + * registered incarnation. A terminating Pod still answers through its IP + * while the backend already counts it gone, so Kubernetes keeps the + * backend's check. */ + private provesIncarnation(session: RegistrySession): boolean { + return ( + this.backend.kind === 'docker' && this.namingIncarnation.has(session) + ); + } + + /** Did runnerd refuse a request for this entry because it serves another + * incarnation of the session? */ + private refusedAsReplaced(session: RegistrySession, error: unknown): boolean { + return ( + error instanceof RunnerdActivityError && + answeringIncarnation(session.createdAtMs, error.incarnation) === + 'replaced' + ); + } + + /** Liveness of a registered entry, evicting it when its incarnation is + * gone; true when it did. On Docker a /healthz answer naming the + * registered incarnation proves it without the backend's check, and one + * naming another proves it gone. No answer, or one naming none, asks the + * backend. */ + private async evictUnlessRunnerdNamesIt( + session: RegistrySession, + ): Promise { + const { sessionId } = session; + if (this.backend.kind === 'docker' && !this.stopping.has(sessionId)) { + try { + const health = await runnerdHealth( + { baseUrl: session.endpoint, token: this.tokenFor(sessionId) }, + AbortSignal.timeout(LIVENESS_PROBE_TIMEOUT_MS), + ); + const named = this.noteIncarnation(session, health.incarnation); + if (named === 'registered') return false; + if (named === 'replaced') + return this.evictStale(session, 'was replaced under its name'); + } catch (error) { + console.warn( + `[sandbox.session] runnerd of ${sessionId} did not answer a liveness probe; asking the backend:`, + error instanceof Error ? error.message : error, + ); + } + } + if (this.registry.get(sessionId) !== session) return false; + return this.evictIfBackendGone(sessionId); + } + async handleCreate(body: string, signal?: AbortSignal): Promise { try { return await withOperationBudget( @@ -1942,7 +2043,7 @@ export class SessionRoutes { 502, ); } - this.registry.set({ + const entry: RegistrySession = { sessionId: req.sessionId, organizationId: req.organizationId, profile: req.profile, @@ -1953,7 +2054,12 @@ export class SessionRoutes { idleTimeoutMs: req.idleTimeoutMs, endpoint, liveExecs: new Map(), - }); + }; + this.registry.set(entry); + // The readiness answer the create waited for is a runnerd answer like + // any other: naming this incarnation, it spares the session's first + // ticket or acquire the backend's existence check. + this.noteIncarnation(entry, created.incarnation); const planned = this.creatingBytes.get(req.sessionId); if (planned !== undefined) { this.youngBytes.set(req.sessionId, { @@ -1973,14 +2079,20 @@ export class SessionRoutes { /** GET /v1/sessions/:id โ€” the platform's pre-turn aliveness probe keys its * phantom-recreate on this route's 404, so a registry hit must be verified * against the backend object: answering from the cache alone turns a dead - * container into "alive" and the turn then fails on a dead address. */ + * container into "alive" and the turn then fails on a dead address. Once + * runnerd has named the registered incarnation, its /healthz verifies it + * instead. */ async handleGet(sessionId: string): Promise { const session = await this.ensureRegistered(sessionId); if (session instanceof Response) return session; if (session === undefined) { return jsonResponse({ error: 'not_found' }, 404); } - if (await this.evictIfBackendGone(sessionId)) { + if ( + this.provesIncarnation(session) + ? await this.evictUnlessRunnerdNamesIt(session) + : await this.evictIfBackendGone(sessionId) + ) { return jsonResponse({ error: 'not_found' }, 404); } const info = this.toInfo(sessionId); @@ -2214,12 +2326,21 @@ export class SessionRoutes { // its session. Its health and request must never acquire that successor. if (knownHealth !== undefined && knownHealth.session !== session) return jsonResponse({ error: 'not_found' }, 404); - if (!session || (await this.evictIfBackendGone(sessionId))) { + if (!session) return jsonResponse({ error: 'not_found' }, 404); + // An entry whose runnerd already named the registered incarnation skips + // the backend's check: the request below names that incarnation, runnerd + // refuses it when a replacement answers, and a failure asks the backend. + const proven = this.provesIncarnation(session); + if (!proven && (await this.evictIfBackendGone(sessionId))) { return jsonResponse({ error: 'not_found' }, 404); } if (this.registry.get(sessionId) !== session) return jsonResponse({ error: 'not_found' }, 404); - const opts = { baseUrl: session.endpoint, token: this.tokenFor(sessionId) }; + const opts = { + baseUrl: session.endpoint, + token: this.tokenFor(sessionId), + incarnation: session.createdAtMs, + }; try { if (action === 'acquire') { this.activating.set(sessionId, session); @@ -2238,6 +2359,20 @@ export class SessionRoutes { ); if (this.registry.get(sessionId) !== session) return jsonResponse({ error: 'not_found' }, 404); + const named = this.noteIncarnation(session, result.incarnation); + if (named === 'replaced') { + this.evictStale(session, 'was replaced under its name'); + return jsonResponse({ error: 'not_found' }, 404); + } + // The check skipped above runs after all when the answer proves + // nothing: a replacement launched without a stamp answered instead. + if ( + named === 'unnamed' && + proven && + ((await this.evictIfBackendGone(sessionId)) || + this.registry.get(sessionId) !== session) + ) + return jsonResponse({ error: 'not_found' }, 404); if (action === 'release') { if (typeof result.released !== 'boolean') throw new Error('invalid runnerd release response'); @@ -2271,6 +2406,13 @@ export class SessionRoutes { } catch (error) { if (this.registry.get(sessionId) !== session) return jsonResponse({ error: 'not_found' }, 404); + // Refused by a replacement under the session's name: the registered + // incarnation is gone, and the request changed nothing. + if ( + this.refusedAsReplaced(session, error) && + this.evictStale(session, 'was replaced under its name') + ) + return jsonResponse({ error: 'not_found' }, 404); if (await this.evictIfBackendGone(sessionId)) return jsonResponse({ error: 'not_found' }, 404); if (this.registry.get(sessionId) !== session) @@ -3186,7 +3328,11 @@ export class SessionRoutes { try { try { const applied = await runnerdActivity( - { baseUrl: session.endpoint, token: this.tokenFor(sessionId) }, + { + baseUrl: session.endpoint, + token: this.tokenFor(sessionId), + incarnation: session.createdAtMs, + }, 'pin', { pinned }, ); @@ -3215,7 +3361,10 @@ export class SessionRoutes { `[sandbox.session] pin=${pinned} for ${sessionId} was not durably acknowledged:`, error, ); - if (error instanceof SessionIncarnationChangedError) { + if ( + error instanceof SessionIncarnationChangedError || + this.refusedAsReplaced(session, error) + ) { this.forgetReclaimed(session); return jsonResponse({ error: 'session_unavailable' }, 503, { 'retry-after': '1',