diff --git a/agents/container/storage.ts b/agents/container/storage.ts index de5c718b..e8d8f60e 100644 --- a/agents/container/storage.ts +++ b/agents/container/storage.ts @@ -12,7 +12,7 @@ import { MAXIMUM_TIMER_MS, runInProcessGroup, type ProcessGroup } from '../proce import { runTrackedProcess, type ProcessGroupLifecycle } from '../tracked-docker.ts'; import { MAXIMUM_DECLARED_LINKS, MAXIMUM_NAME_BYTES, TREE_SCRIPT } from './tree-script.ts'; import { ALLOCATION_IN_USE, allocationListCommands, assertResourceOwner, claimAllocationId, hasOwnerLabels, - ownerLabelArgs, releaseAllocationId, type ResourceOwner, UUID_V4 } from '../labels.ts'; + ownerLabelArgs, releaseAllocationId, type ResourceOwner, runnerProbeLabels, UUID_V4 } from '../labels.ts'; export interface TaskFilesystems { readonly keeper: string; @@ -415,24 +415,37 @@ export const PROBE_VOLUME_MOUNT = ['type=volume', 'target=/work', 'volume-driver ...TASK_VOLUME_OPTIONS(1024 * 1024, 64).map(option => `volume-opt=${option}`)] // A field with a comma in it is quoted, as Docker's CSV parser reads --mount. .map(field => field.includes(',') ? `"${field}"` : field).join(','); +/** The probe container's name: a fixed prefix and a UUID v4. Recovery removes only containers named so (#181). */ +export const PATH_IDENTITY_PROBE_NAME = new RegExp(`^codeboost-path-identity-${UUID_V4}$`); +/** + * The probe's `docker run` arguments. The container and its anonymous volume carry the runner's token and the probe kind + * (#181), so startup recovery finds what a failed or interrupted probe left. + */ +export function pathIdentityProbeArgs(imageId: string, runnerOwner: string, name: string): string[] { + if (!PATH_IDENTITY_PROBE_NAME.test(name)) throw new Error('The path identity probe needs a name D generates.'); + const labels = Object.entries(runnerProbeLabels(runnerOwner, 'path-identity')).map(([key, value]) => `${key}=${value}`); + return ['run', '--rm', '--name', name, ...labels.flatMap(label => ['--label', label]), '--read-only', '--user', '10001:10001', + '--network=none', '--cap-drop=ALL', '--security-opt=no-new-privileges', '--security-opt=seccomp=builtin', '--runtime=runc', + '--pids-limit=32', '--memory=128m', '--cpus=.25', + '--mount', [PROBE_VOLUME_MOUNT, ...labels.map(label => `volume-label=${label}`)].join(','), + '--entrypoint', 'sh', imageId, '-c', PATH_IDENTITY_SCRIPT, 'probe', '/work']; +} /** * The path identity of task storage (#162): how the filesystem the agent edits, where the runner inspects task changes, tells two * spellings of a path apart. It is measured, not assumed, on a volume made with the same options as task storage, in a * container hardened like the keeper. The volume is anonymous and the container `--rm`, so once the container has * started the daemon removes both when it exits, even if this process does not live to see it. A client stopped before * then (its deadline) may leave the container created but never started, which `--rm` does not remove, so a failed - * probe removes its container, and that volume, by the unique name it gave it. Not covered: a create that lands after - * the settle window, or a runner that dies before the removal; the probe carries no owner labels, so recovery does not - * find that container (`docker ps -a --filter name=codeboost-path-identity-` does). Nor is the volume's removal - * confirmed: `--rm` and `rm --volumes` remove it, but Docker only logs a failure to. A recovery sweep is #181. + * probe removes its container, and that volume, by the unique name it gave it. What this cannot remove (a create that + * lands after the settle window, a runner that dies before the removal, a volume Docker failed to remove, since it + * only logs that) carries `runnerOwner` and the probe kind, and the next startup's recovery removes it (#181). */ -export async function probeTaskStoragePathIdentity(imageId: string, timeoutMs = 60_000, cleanupMs = PROBE_CLEANUP_MS): Promise { +export async function probeTaskStoragePathIdentity(imageId: string, runnerOwner: string, timeoutMs = 60_000, + cleanupMs = PROBE_CLEANUP_MS): Promise { if (!/^sha256:[0-9a-f]{64}$/.test(imageId)) throw new Error('The path identity probe requires the immutable built image ID.'); assertBuiltAgentImage(imageId); const name = `codeboost-path-identity-${randomUUID()}`; - const args = ['run', '--rm', '--name', name, '--read-only', '--user', '10001:10001', '--network=none', '--cap-drop=ALL', - '--security-opt=no-new-privileges', '--security-opt=seccomp=builtin', '--runtime=runc', '--pids-limit=32', '--memory=128m', - '--cpus=.25', '--mount', PROBE_VOLUME_MOUNT, '--entrypoint', 'sh', imageId, '-c', PATH_IDENTITY_SCRIPT, 'probe', '/work']; + const args = pathIdentityProbeArgs(imageId, runnerOwner, name); const outcome = await runInProcessGroup('docker', args, { env: dockerEnvironment(), timeoutMs }); if (outcome.status !== 0) { const failure = new DockerError(args, outcome); @@ -445,7 +458,7 @@ export async function probeTaskStoragePathIdentity(imageId: string, timeoutMs = const refused = outcome.status !== null && /Error response from daemon/.test(outcome.stderr); try { await removeProbeContainer(name, refused ? 0 : PROBE_SETTLE_MS, cleanupMs); } catch (error) { - throw new Error(`${failure.message} The probe container ${name} may remain; remove it with docker rm --force --volumes ${name}.`, + throw new Error(`${failure.message} The probe container ${name} may remain; the next start's recovery removes it, or remove it now with docker rm --force --volumes ${name}.`, { cause: new AggregateError([failure, error]) }); } throw failure; diff --git a/agents/labels.ts b/agents/labels.ts index f791c0ab..ffd6bf3e 100644 --- a/agents/labels.ts +++ b/agents/labels.ts @@ -87,3 +87,23 @@ export const ownerLabels = (owner: ResourceOwner): Readonly> | null | undefined, owner: ResourceOwner) => labels?.[RUNNER_LABEL] === owner.runnerOwner && labels?.[ATTEMPT_LABEL] === owner.attemptId && labels?.[ALLOCATION_LABEL] === owner.allocationId; + +/** + * The kind label of a runner-scoped probe (#181): an object that belongs to the runner as a whole, not to an attempt or + * an allocation, so it carries only the runner label and this one. Startup recovery, under the runner lock, removes + * this runner's leftover probes: only one process holds a runner token at a time, so a probe found then is an earlier + * process's. A Docker client that process left may still be creating one: if the create lands between recovery's + * container and volume listings, recovery keeps its volume, which the new container holds; if it lands after them, + * recovery does not see it. Either way the next start removes both. + */ +export const PROBE_LABEL = 'io.codeboost.probe'; +/** The probes D runs: `path-identity` measures task storage's path identity at runner startup (#162). */ +export type RunnerProbe = 'path-identity'; +export const RUNNER_PROBES: readonly RunnerProbe[] = Object.freeze(['path-identity']); + +/** The labels on a runner-scoped probe's container and volume. Validates the runner token before it is written. */ +export function runnerProbeLabels(runnerOwner: string, probe: RunnerProbe): Readonly> { + if (!isRunnerOwner(runnerOwner)) throw new Error('runnerOwner must be 32 lowercase hex characters.'); + if (!RUNNER_PROBES.includes(probe)) throw new Error('Unknown runner probe.'); + return Object.freeze({ [RUNNER_LABEL]: runnerOwner, [PROBE_LABEL]: probe }); +} diff --git a/agents/recovery.ts b/agents/recovery.ts index 99366cef..a1c59703 100644 --- a/agents/recovery.ts +++ b/agents/recovery.ts @@ -1,9 +1,9 @@ import { DOCKER_ID } from './client-outcome.ts'; import { runDocker, type DockerOutcome } from './docker.ts'; -import { ALLOCATION_LABEL, ATTEMPT_LABEL, isAllocationId, isLabelAttemptId, isRunnerOwner, +import { ALLOCATION_LABEL, ATTEMPT_LABEL, isAllocationId, isLabelAttemptId, isRunnerOwner, PROBE_LABEL, RUNNER_LABEL } from './labels.ts'; -import { adoptRecoveredTaskStorage, hasLiveTaskStorage, RecoveredStorageRejected, type RecoveredTaskStorage, - type TaskStorageParts } from './container/storage.ts'; +import { adoptRecoveredTaskStorage, hasLiveTaskStorage, PATH_IDENTITY_PROBE_NAME, RecoveredStorageRejected, + type RecoveredTaskStorage, type TaskStorageParts } from './container/storage.ts'; /** A Docker object recovery found, with the labels it carries. */ export interface RecoveredResource { @@ -26,7 +26,7 @@ export interface UnownedResource extends RecoveredResource { export interface RecoveryReport { /** * Agent containers, egress proxies, seeders, export, inspection and commit containers and networks of this runner, - * now confirmed gone. + * and its path identity probes' containers and volumes (#181), now confirmed gone. */ readonly removed: readonly RecoveredResource[]; /** One handle per task-storage allocation of this runner, kept whole for export and `removeTaskFilesystems`. */ @@ -48,7 +48,9 @@ const EGRESS_LABEL = 'io.codeboost.egress'; const STORAGE_LABEL = 'io.codeboost.task-storage'; // Every label a codeboost object of any build carries at least one of; used to find objects without a runner label. // Each Docker type is scanned for all of them, so an object with a label D never puts on its type is still found. -const KIND_LABELS = [INVOCATION_LABEL, EGRESS_LABEL, STORAGE_LABEL, ALLOCATION_LABEL] as const; +const KIND_LABELS = [INVOCATION_LABEL, EGRESS_LABEL, STORAGE_LABEL, ALLOCATION_LABEL, PROBE_LABEL] as const; +// The name Docker gives an anonymous volume, as the path identity probe's is. +const ANONYMOUS_VOLUME = /^[0-9a-f]{64}$/; const DIAGNOSTIC_LIMIT = 1_000; // Objects per `docker inspect`, so a daemon with many codeboost objects cannot exceed the argument-size limit. const INSPECT_BATCH = 200; @@ -111,19 +113,27 @@ const inspectBatch = async (kind: Kind, refs: readonly string[], remaining: () = }; const unique = (values: readonly string[]) => [...new Set(values)]; -// Remove one object by its full ID, and confirm it is gone. +/** Docker refused to remove a volume because a container mounts it. */ +class VolumeInUse extends Error {} +// Remove one object by its full ID (a volume by its name, which is its identity), and confirm it is gone. const removeById = async (resource: RecoveredResource, remaining: () => number) => { - const args = resource.kind === 'network' ? ['network', 'rm', resource.id!] : ['rm', '--force', resource.id!]; + const ref = resource.kind === 'volume' ? resource.name : resource.id!; + const args = resource.kind === 'network' ? ['network', 'rm', ref] + : resource.kind === 'volume' ? ['volume', 'rm', '--force', ref] : ['rm', '--force', ref]; const result = await runDocker(args, { timeoutMs: remaining() }); if (result.status === 0) return; - if (!absent(await runDocker([resource.kind, 'inspect', resource.id!], { timeoutMs: remaining() }))) + // Docker 27.4: "remove : volume is in use - []". + if (resource.kind === 'volume' && /volume is in use/.test(result.stderr)) + throw new VolumeInUse(`Volume ${resource.name} is in use.`); + if (!absent(await runDocker([resource.kind, 'inspect', ref], { timeoutMs: remaining() }))) throw new Error(`Could not remove ${resource.kind} ${resource.name}: ${(result.error?.message ?? result.stderr).trim()}`); }; /** * Crash recovery for one database (#51 item 4). Acts only on objects whose `io.codeboost.runner` label is * `runnerOwner`: it removes agent containers, egress proxies, seeders, export, inspection and commit containers and - * networks, and resolves only once they are gone. It keeps task storage whole (volumes and keeper) and returns a recovery handle per + * networks, and the containers and anonymous volumes path identity probes left (#181), and resolves only once they are + * gone. It keeps task storage whole (volumes and keeper) and returns a recovery handle per * allocation. Objects from older builds without a runner label, and anything it does not recognise, are reported and * never touched. * @@ -151,7 +161,10 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000, network: [] as RecoveredResource[] }; const unowned: UnownedResource[] = []; for (const kind of ['container', 'volume', 'network'] as const) { - owned[kind] = await inspect(kind, await list(kind, `${RUNNER_LABEL}=${runnerOwner}`, remaining), remaining); + // An object inspected with another runner's label replaced the one listed under the same name: it is not this + // runner's, so recovery neither removes nor reports it. + owned[kind] = (await inspect(kind, await list(kind, `${RUNNER_LABEL}=${runnerOwner}`, remaining), remaining)) + .filter(resource => resource.labels[RUNNER_LABEL] === runnerOwner); if (options.unowned === false) continue; // Docker cannot filter on a missing label, so list every codeboost object of this kind and keep those without one. const candidates = unique((await Promise.all(KIND_LABELS.map(label => list(kind, label, remaining)))).flat()); @@ -173,10 +186,15 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000, storage.set(allocationId, group); }; // D writes exactly one kind label on each object, with complete owner labels (an egress object's egress label is its - // allocation ID). Recovery has no creation-time ID to prove ownership, so any other combination is not D's and is - // left alone. + // allocation ID). A runner-scoped probe carries the probe label and no attempt or allocation. Recovery has no + // creation-time ID to prove ownership, so any other combination is not D's and is left alone. const kindOf = (resource: RecoveredResource) => { const labels = resource.labels; + if (PROBE_LABEL in labels) { + const scoped = [ATTEMPT_LABEL, ALLOCATION_LABEL, INVOCATION_LABEL, EGRESS_LABEL, STORAGE_LABEL] + .some(label => label in labels); + return !scoped && labels[PROBE_LABEL] === 'path-identity' ? 'probe:path-identity' : undefined; + } if (!isLabelAttemptId(labels[ATTEMPT_LABEL]) || !isAllocationId(labels[ALLOCATION_LABEL])) return undefined; const present = [INVOCATION_LABEL, EGRESS_LABEL, STORAGE_LABEL].filter(label => label in labels); if (present.length !== 1) return undefined; @@ -196,12 +214,17 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000, || kind === 'storage:inspect' || kind === 'storage:commit' || kind === 'agent' || kind === 'egress') remove.push(resource); + // Only one process holds this runner's token, and it probes after recovery, so a probe found now is an earlier + // process's. One that process's orphaned client creates during the scan holds its volume, which is then kept. + else if (kind === 'probe:path-identity' && PATH_IDENTITY_PROBE_NAME.test(resource.name)) remove.push(resource); else unknown(resource); } + const probeVolumes: RecoveredResource[] = []; for (const resource of owned.volume) { const kind = kindOf(resource); if (kind === 'storage:work') keep(resource, 'workVolume'); else if (kind === 'storage:metadata') keep(resource, 'metadataVolume'); + else if (kind === 'probe:path-identity' && ANONYMOUS_VOLUME.test(resource.name)) probeVolumes.push(resource); else unknown(resource); } const networks = owned.network.filter(resource => { @@ -210,14 +233,20 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000, return false; }); - // Containers first: a network cannot be removed while an agent or proxy is still attached to it. + // Containers first: a network cannot be removed while an agent or proxy is still attached to it, nor a probe's + // volume while its container still exists. const failures: unknown[] = [], confirmed: RecoveredResource[] = []; - for (const resource of [...remove, ...networks]) { - if (resource.kind === 'network' && failures.length) break; + for (const resource of [...remove, ...probeVolumes, ...networks]) { + if (resource.kind !== 'container' && failures.length) break; try { await removeById(resource, remaining); confirmed.push(resource); - } catch (error) { failures.push(error); } + } catch (error) { + // Every container recovery removes is gone by now, so the holder is one it leaves: one it reports as unowned, one + // without codeboost labels, or a probe an earlier process's orphaned client created between the container and + // volume listings. The volume is kept, and a start after its holder is gone removes it. + if (!(error instanceof VolumeInUse)) failures.push(error); + } } if (failures.length) { const detail = failures.map(error => (error as Error).message).join('; '); @@ -242,6 +271,6 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000, for (const resource of group.resources) unowned.push(Object.freeze({ ...resource, reason: 'inconsistent-storage' })); } } - return Object.freeze({ removed: Object.freeze([...remove, ...networks]), storage: Object.freeze(handles), + return Object.freeze({ removed: Object.freeze(confirmed), storage: Object.freeze(handles), unowned: Object.freeze(unowned) }); } diff --git a/docs/architecture.md b/docs/architecture.md index ae4d909d..86ebafbc 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -410,7 +410,7 @@ These are the decisions that shape the structure. The full list, with the reason | C, K | Guarded merge, merge queue | Done | | D | Agent isolation boundary | Done. Since the isolation gate: bounded diff export (#76), task-change inspection and the runner's commit as a bounded `git bundle` (#66, PRs #85 and #90), owner-scoped recovery for Ask (#95), the plan schema for Claude (#97), gitlink mounts and the pre-launch tree check (#81, #99, PR #100), and Codex refused in every phase (#93, PR #106). The runner-contract follow-ups (#51) are closed: the last one tests export and removal through a recovery handle after a real restart (PR #109). | | E | Planning library and suggestions | Done. E4 (PR #45) added the acceptance fixtures and recorded Claude output that replays through validation and the Store. | -| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). F5 (PR #149) added current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6a (PR #153) added the durable push of the rewritten head. F6b (PR #161) added the one-click merge job (preparation, the required-check wait, the exact-pair guarded merge) and an already-fixed scan covering every base commit after the task's original base. F6c (PR #177) runs every T9 suite in CI and adds the nightly live Claude probes; T9 is complete (PR #178) and #22 is closed; its follow-ups are #174 and #176. T6 and T12 are verified, and PR #183 also fixed reading required checks that come only from a ruleset, for a token with admin rights. Required checks are now also read for a token without admin rights (observed with read-only access to public repositories): classic checks then come from the branch read, which has no `strict`, so classic protection gives such a token no atomic base guard. Startup now refuses a `pathIdentity` that disagrees with a probe of task storage (#162, PR #180); follow-ups #179 and #181. | +| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). F5 (PR #149) added current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6a (PR #153) added the durable push of the rewritten head. F6b (PR #161) added the one-click merge job (preparation, the required-check wait, the exact-pair guarded merge) and an already-fixed scan covering every base commit after the task's original base. F6c (PR #177) runs every T9 suite in CI and adds the nightly live Claude probes; T9 is complete (PR #178) and #22 is closed; its follow-ups are #174 and #176. T6 and T12 are verified, and PR #183 also fixed reading required checks that come only from a ruleset, for a token with admin rights. Required checks are now also read for a token without admin rights (observed with read-only access to public repositories): classic checks then come from the branch read, which has no `strict`, so classic protection gives such a token no atomic base guard. Startup now refuses a `pathIdentity` that disagrees with a probe of task storage (#162, PR #180), and its recovery removes what an interrupted probe left (#181); follow-up #179. | | Hardening | Allowlisted subprocess environments | Done: every Git command (#82, #83) and every `gh` runner (#84, #86). | | Tests | Test suite reliability | Done. The full suite runs ordinary tests first and Docker-backed files serially; CI and the real-Docker gate passed on PR #132 (#129). | | G | Planning screen | Done (#145). The screen displays/imports the current plan, requests and renders a next-revision draft or suggestion cards, and applies results with revision-bound exact ambiguous replay. G4 composes the browser with the production planning setup, D-backed provider boundary, F endpoints and SQLite persistence; focused adapter, recorded-output and exact-head real-Docker planning suites cover the lower boundary. | diff --git a/docs/implementation/agent-isolation.md b/docs/implementation/agent-isolation.md index 48460295..827e3b92 100644 --- a/docs/implementation/agent-isolation.md +++ b/docs/implementation/agent-isolation.md @@ -195,8 +195,13 @@ Use only these entry points to run an agent: same storage gives the same `head`. 7. After a crash or restart, call `recoverLeftovers(runnerOwner)` (`agents/recovery.ts`) while holding the database's single-runner lock and before admitting work. It touches only objects labelled with that runner - token. It removes agent containers, egress proxies, seeders, export, inspection and commit containers and networks, and - resolves once they are gone. It keeps task storage whole (both volumes and the keeper) and returns one recovery + token. It removes agent containers, egress proxies, seeders, export, inspection and commit containers and networks, + and the container and anonymous volume a path identity probe left (#181), and resolves once they are gone. A probe + is runner-scoped: it carries only the runner label and `io.codeboost.probe=path-identity`, no attempt or allocation, + and recovery removes it only with the runner's own token, the probe kind and no attempt, allocation or other + kind label (other labels, such as the image's or Docker's anonymous-volume label, are allowed), and a name the + probe gives (`codeboost-path-identity-`, or Docker's 64-hex name for its volume). Containers go first; a + probe volume another container still mounts is kept, and the first start after that container is gone removes it. It keeps task storage whole (both volumes and the keeper) and returns one recovery handle per allocation, carrying its attempt and allocation IDs. `removeTaskFilesystems` accepts a handle as it accepts the value `prepareTaskFilesystems` returned. D issues a handle only after checking every part's owner labels. Objects without a runner label (from older builds), objects of this runner that D does not create, and storage whose parts diff --git a/docs/implementation/runner-lifecycle.md b/docs/implementation/runner-lifecycle.md index 0b91ebf5..75109e79 100644 --- a/docs/implementation/runner-lifecycle.md +++ b/docs/implementation/runner-lifecycle.md @@ -281,7 +281,7 @@ codeboost never writes the user's repository. The commits it makes live in a **r | Runner owner token | F | A random ID of exactly 32 lowercase hexadecimal characters (`^[0-9a-f]{32}$`), stored in the database (`app_settings`). A stored value that does not match is refused before it is used as a label or in any path, and startup exits. It is stored together with the database file's device and inode numbers. It is read, or created, in startup recovery step 2, after the lock is taken and the `Store` is opened. If the stored device and inode don't match the open file (the database was copied), F creates a new token. So a copy never shares a token with its original. | | Token in every request | D contract | Done in #51 item 3. `InvocationInput` has a required `runnerOwner` (32 lowercase hex characters), and so does every allocator that runs before an invocation exists: `prepareTaskFilesystems` takes an owner (`runnerOwner`, `attemptId`, and a caller-chosen `allocationId`, a lowercase UUID v4), and the adapters take a caller-chosen `networkAllocationId` for the vendor network. Every resource D creates carries `io.codeboost.runner`, `io.codeboost.attempt` and `io.codeboost.allocation` (`agents/labels.ts`): task-storage volumes, the keeper and the seeder with the storage owner; the network and proxy with the invocation's runner and attempt and the network allocation; the agent container with its own attempt and the storage allocation it mounts. D checks these labels whenever it validates a resource, and refuses task storage that belongs to another runner. Each allocation ID names one allocation: before creating anything, D refuses an ID held by another allocation in the process or still carried by any Docker object (so a reuse after a restart is caught). That check and the create are not atomic across processes, so right after the first create D checks again that its object is the only one with the ID, and backs out if not. Cleanup proves ownership in one of two ways. A Docker ID captured when D created the object is proof on its own (Docker never reuses IDs), so the vendor network, proxy and agent container are removed by that ID even if their labels are wrong. An object found only by name (a create whose ID never came back, or task storage, which has no captured IDs) is removed only when its runner, attempt and allocation labels all match. The adapters check `networkAllocationId` before returning a handle, and `captureInvocation` refuses an attempt ID that cannot be written as a label. | | Recovered storage handles | D | Done in #51 item 4: the report's `storage` lists one `RecoveredTaskStorage` handle per allocation, with its runner, attempt and allocation IDs and whichever parts exist, and both `removeTaskFilesystems` and `exportTaskDiff` accept it. D tracks task-storage ownership in a process-local `WeakMap`, which is empty after a restart. So `recoverLeftovers` returns, for each kept volume set, an **authenticated recovery handle** that D registers in its ownership map after inspecting every part (volumes and keeper) and checking its task-storage kind and all three owner labels. It refuses storage this process allocated and has not removed. **Binding to an attempt:** task-storage volumes and the keeper also carry `io.codeboost.attempt=`, from the owner passed to `prepareTaskFilesystems`. **F chooses the allocation ID, not D.** F generates it (a lowercase UUID v4) and saves it as `attempts.allocation_id` **before** it starts the asynchronous allocation, then passes it to `prepareTaskFilesystems`, which labels the volumes and keeper with it (`io.codeboost.allocation`). So a crash in the middle of allocation still leaves a row that matches whatever D created. The D prerequisite includes accepting a caller-provided allocation ID. The handle carries both IDs. F uses a handle only if its attempt ID and allocation ID both match one attempt row of this database. A handle that matches no row, or disagrees with the row, is reported at startup (the CLI prints its attempt ID) for the user to remove by hand; it is never exported or removed automatically. `exportTaskDiff` and `removeTaskFilesystems` accept these handles. F never builds a `TaskFilesystems` value from report fields. | -| Scoped recovery | D | Done in #51 item 4 (`agents/recovery.ts`). `recoverLeftovers(runnerOwner): Promise` acts only on resources whose `io.codeboost.runner` label equals the token. It stops and removes agent containers, egress proxies, unfinished seeders, export and inspection containers and networks, by ID and networks last, and resolves only when they are gone. It removes an object only when it carries exactly one kind label with the value D writes and complete owner labels. **It keeps task storage whole: the volumes and their keeper container**, which it recognises by their task-storage labels, and lists one handle per allocation so F can export partial output first. A handle names whichever parts exist, since a crash during allocation can leave any subset; export needs the keeper. F then calls `removeTaskFilesystems` for each one (startup recovery step 4). It refuses to run while this process holds task storage of the runner. It rejects with a `RecoveryError` (bounded message, plus the objects it did remove) if anything cannot be stopped or removed, or if a storage check cannot reach Docker, and then issues no handles. | +| Scoped recovery | D | Done in #51 item 4 (`agents/recovery.ts`). `recoverLeftovers(runnerOwner): Promise` acts only on resources whose `io.codeboost.runner` label equals the token. It stops and removes agent containers, egress proxies, unfinished seeders, export and inspection containers and networks, by ID (probe volumes by name, after the containers) and networks last, and resolves only when they are gone. It removes an object only when it carries exactly one kind label with the value D writes and complete owner labels, or, for the runner-scoped path identity probe (#181), the probe label and no attempt, allocation or other kind label. **It keeps task storage whole: the volumes and their keeper container**, which it recognises by their task-storage labels, and lists one handle per allocation so F can export partial output first. A handle names whichever parts exist, since a crash during allocation can leave any subset; export needs the keeper. F then calls `removeTaskFilesystems` for each one (startup recovery step 4). It refuses to run while this process holds task storage of the runner. It rejects with a `RecoveryError` (bounded message, plus the objects it did remove) if anything cannot be stopped or removed, or if a storage check cannot reach Docker, and then issues no handles. | | Unowned resources | D and F | Done in D by #51 item 4: the report's `unowned` lists them with the reason `no-runner-label`, alongside objects of this runner that D does not create (`unknown-kind`) and storage whose parts disagree (`inconsistent-storage`); none is removed. Resources with codeboost labels but no `io.codeboost.runner` label (from builds before this change) fail closed. The report lists them, and F refuses to open admission while any exist. **codeboost never removes them itself**, because it cannot prove that no older-build runner is alive: older builds take no lock this build can see, and an idle older runner may have no container running at the moment of a check. So startup stops with a message that names each resource as the exact `docker container rm -f` / `docker network rm` / `docker volume rm` command that removes it (`runner/production.ts`, #91). The user stops every older codeboost process, runs those commands, and starts again. There is no machine-wide lock. | | When it runs | F | Only while holding this database's single-runner lock (startup recovery step 1 comes first). | | Ask's owner | Ask | Ask labels its objects with a second per-database token, `ask_owner` (`Store.askOwnerToken`, same rules as the runner token), and recovers it under the review's Ask lock (#65, `agent-isolation.md`). The runner's recovery therefore never sees Ask's storage, and Ask's recovery never removes the runner's agents. Unlike the runner, Ask does not refuse because of `no-runner-label` objects: it calls `recoverLeftovers(askOwner, timeoutMs, { unowned: false })`, which skips the daemon-wide search for them. | @@ -320,7 +320,7 @@ This runs before the coordinator opens. 7. **Unowned preparation check.** If any attempt has `preparation_started_at` set and `preparation_pgid` null (see "Launch"), stop here: do not open the coordinator, name each attempt and directory, and wait for `--release-preparation`. Step 4 does not remove those directories. 8. Open the coordinator. -**Production wiring (#91, #22).** The runner is opt-in: the review configuration's `runner` block names its root, an optional diagnostics directory and cap, the committer and optional storage limits (`runner/production.ts`, `parseRunnerConfig`). Without the block, or in a demo, there is no coordinator and runner actions answer that the runner is not configured. With it, `web/cli.ts` takes the lock, `startServer` opens the `Store` and then, before it listens, runs the planning setup when planning is on (`web/planning.ts`: verify the lock, then fail every planning request an earlier process left `pending`, #124). That step runs with or without a runner block: every start takes the lock, and planning is on for any non-demo review with a `github` block. Then it runs `setUpRunner`: it checks for the github block (the prompt carries the issue) and `CLAUDE_CODE_OAUTH_TOKEN`, calls the lock's `verify()` (decision 1, step 4), reads the runner token for the locked file, and runs `recoverStartup` with D's own `recoverLeftovers`, `exportTaskDiff` and `removeTaskFilesystemsAsync`. The agent image is built after D's recovery has stopped every leftover agent, so no agent keeps writing to its storage during a long build, and before the first export's deadline starts (the build blocks the event loop, so a deadline armed before it would expire as soon as it returned). With the built image, it then probes task storage's path identity and refuses to start when `pathIdentity` disagrees (#162, `read-only-review.md`). Any failure closes the `Store`, releases the lock and exits with its message. To export a recovered storage, F saves the storage's `metadataBaseline` and the commit it was seeded from (`attempts.metadata_baseline`, `attempts.storage_base`, schema v9) as soon as D's allocation returns; a row without them gets "Partial output could not be exported: its storage baseline was never saved". Diagnostics are kept per database, in `//diagnostics`: retention reads references only from its own `Store`, so runners of databases sharing a directory must never see each other's files. Recovered diffs are saved through `saveDiagnostic`, with the same owner-only file and retention as the live path; retention keeps every diff this recovery saved, because none is referenced until the finalization transaction. A diff D cut at its byte limit ends with a `codeboost:` line that says so, on this path and the live one. The issue text a prompt carries (title, body and collaborators' comments) is read once and reused for later items for up to five minutes; the base tree listing is read once per base commit. A first Ctrl+C during startup waits for startup to finish; a second stops the process at once, which recovery treats like a crash at the next start. The composed deps accept `execute` and credential-free `check` attempts (`RunnerDeps.kinds`); admission refuses every other kind before it writes anything. A plan item runs only through `ItemExecutor`, so `/api/runner` refuses to retry an execute attempt on its own; a task's plan runs again through `resume` (below). +**Production wiring (#91, #22).** The runner is opt-in: the review configuration's `runner` block names its root, an optional diagnostics directory and cap, the committer and optional storage limits (`runner/production.ts`, `parseRunnerConfig`). Without the block, or in a demo, there is no coordinator and runner actions answer that the runner is not configured. With it, `web/cli.ts` takes the lock, `startServer` opens the `Store` and then, before it listens, runs the planning setup when planning is on (`web/planning.ts`: verify the lock, then fail every planning request an earlier process left `pending`, #124). That step runs with or without a runner block: every start takes the lock, and planning is on for any non-demo review with a `github` block. Then it runs `setUpRunner`: it checks for the github block (the prompt carries the issue) and `CLAUDE_CODE_OAUTH_TOKEN`, calls the lock's `verify()` (decision 1, step 4), reads the runner token for the locked file, and runs `recoverStartup` with D's own `recoverLeftovers`, `exportTaskDiff` and `removeTaskFilesystemsAsync`. The agent image is built after D's recovery has stopped every leftover agent, so no agent keeps writing to its storage during a long build, and before the first export's deadline starts (the build blocks the event loop, so a deadline armed before it would expire as soon as it returned). With the built image, it then probes task storage's path identity and refuses to start when `pathIdentity` disagrees (#162, `read-only-review.md`). The probe's container and volume carry the runner's token, so a probe a killed runner left is removed by the next startup's recovery (#181). Any failure closes the `Store`, releases the lock and exits with its message. To export a recovered storage, F saves the storage's `metadataBaseline` and the commit it was seeded from (`attempts.metadata_baseline`, `attempts.storage_base`, schema v9) as soon as D's allocation returns; a row without them gets "Partial output could not be exported: its storage baseline was never saved". Diagnostics are kept per database, in `//diagnostics`: retention reads references only from its own `Store`, so runners of databases sharing a directory must never see each other's files. Recovered diffs are saved through `saveDiagnostic`, with the same owner-only file and retention as the live path; retention keeps every diff this recovery saved, because none is referenced until the finalization transaction. A diff D cut at its byte limit ends with a `codeboost:` line that says so, on this path and the live one. The issue text a prompt carries (title, body and collaborators' comments) is read once and reused for later items for up to five minutes; the base tree listing is read once per base commit. A first Ctrl+C during startup waits for startup to finish; a second stops the process at once, which recovery treats like a crash at the next start. The composed deps accept `execute` and credential-free `check` attempts (`RunnerDeps.kinds`); admission refuses every other kind before it writes anything. A plan item runs only through `ItemExecutor`, so `/api/runner` refuses to retry an execute attempt on its own; a task's plan runs again through `resume` (below). **F5 production extension (#22).** The production dependency set also accepts `check` attempts. `prepare-merge` resolves the same task-owned pull request as merge admission, fetches its exact base/head without moving a runner ref, and stops for refreshed review when the head moved. A moved base is handled through the durable F3/F4 rebase marker and mapping. Attribution and approvals are recomputed before each allowed `cmd:` list runs sequentially in a fresh read-only workspace at the exact resulting head. The fixed image dispatcher reads bounded JSON argv arrays from the input mount, invokes them without a shell, and receives no provider credential or allowed external host. Its completed result records both the head and argv digest; failure, cancellation, interruption, a changed head, or changed argv does not count. A final fresh remote read turns movement during preparation into `review-required`, never ready. The saved user-action replay is settled with the background result even during shutdown; startup marks an interrupted preparation failed after resource recovery, so it never replays `preparing` forever. `GET /api/runner` exposes the active and last in-memory pre-merge result without rebuilding history during polling. diff --git a/runner/production.ts b/runner/production.ts index d276ef71..b4a528b1 100644 --- a/runner/production.ts +++ b/runner/production.ts @@ -207,7 +207,7 @@ export async function setUpRunner(o: { service: ReviewService; capability: Shutd /** Told before the slow part (recovery and the image build) starts. */ onSlowStart?: () => void; /** Measures task storage's path identity with the built image; Docker's own by default. */ - probeTaskStorage?: (imageId: string) => Promise }): Promise { + probeTaskStorage?: (imageId: string, runnerOwner: string) => Promise }): Promise { const { service, config } = o, review = service.config; if (review.demo) throw new Error('Demos never run the runner.'); if (!review.github) throw new Error(RUNNER_NEEDS_GITHUB); @@ -275,8 +275,9 @@ export async function setUpRunner(o: { service: ReviewService; capability: Shutd // #162: plan path keys follow pathIdentity, so it must match the filesystem the agent edits, where the runner inspects // and commits task changes. That is task storage in the Docker daemon's kernel, not the host folder the staging clone // is made in, so it is probed there, with the built image, before anything is admitted. Host checkouts (the staging - // clone, the rebase worktree) are #179. - assertPathIdentity(review.pathIdentity, await (o.probeTaskStorage ?? probeTaskStoragePathIdentity)(imageId), + // clone, the rebase worktree) are #179. The probe is labelled with this runner's token, so whatever an interrupted + // probe leaves, the next startup's recovery removes (#181). + assertPathIdentity(review.pathIdentity, await (o.probeTaskStorage ?? probeTaskStoragePathIdentity)(imageId, runnerOwner), 'the task storage the agent edits'); const workspace = createTaskWorkspace({ store: service.store, runnerRoot: config.root, runnerOwner, repository, imageId, limits: { ...EXECUTE_STORAGE, ...config.limits }, committer: config.committer }); diff --git a/test/agent-recovery.test.ts b/test/agent-recovery.test.ts index 84ed0ead..61bf22a8 100644 --- a/test/agent-recovery.test.ts +++ b/test/agent-recovery.test.ts @@ -1,8 +1,14 @@ import { randomUUID } from 'node:crypto'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -// A fake Docker daemon behind both CLI entry points recovery uses: execFile (runDocker) and spawnSync (storage). -interface FakeObject { kind: 'container' | 'volume' | 'network'; id?: string; name: string; labels: Record } +// A fake Docker daemon behind both CLI entry points recovery uses: execFile (runDocker) and spawnSync (storage). Its +// removals answer as Docker 27.4.0 does: `rm --force` of a missing container exits 0 and says "No such container" on +// stderr; `volume rm --force` of a missing name exits 0 and prints the name, and of a volume a container mounts exits 1 +// with "volume is in use"; `volume inspect` of a missing name exits 1 with "no such volume". Its other errors are +// shorter than Docker's; the code reads only the exit status and, through `absent`, "No such" or "not found". +interface FakeObject { kind: 'container' | 'volume' | 'network'; id?: string; name: string; labels: Record; + /** For a container: the names of the volumes it mounts. */ + mounts?: string[] } const daemon = vi.hoisted(() => ({ objects: [] as FakeObject[], refuseRemoval: new Set(), calls: [] as string[][], failSingleInspect: new Set(), vanishOnInspect: new Set(), brokenInspect: new Set(), beforeSingleInspect: undefined as undefined | ((ref: string) => void) })); @@ -36,11 +42,18 @@ const answer = (args: string[]): { status: number; stdout: string; stderr: strin ? { Id: object!.id, Name: `/${object!.name}`, Config: { Labels: object!.labels } } : { ...(object!.kind === 'network' ? { Id: object!.id } : {}), Name: object!.name, Labels: object!.labels }); return found.every(Boolean) ? { status: 0, stdout: JSON.stringify(json), stderr: '' } - : { status: 1, stdout: JSON.stringify(json), stderr: 'Error: No such object' }; + : { status: 1, stdout: JSON.stringify(json), stderr: args[0] === 'volume' + ? `Error response from daemon: get ${args.slice(2).find(ref => !find('volume', ref))}: no such volume` + : 'Error: No such object' }; } const [kind, ref] = args[0] === 'rm' ? ['container', args[2]!] : [args[0]!, args[args.length - 1]!]; const object = find(kind, ref); + if (!object && kind === 'volume' && args.includes('--force')) return { status: 0, stdout: ref, stderr: '' }; + if (!object && kind === 'container' && args.includes('--force')) + return { status: 0, stdout: '', stderr: `Error response from daemon: No such container: ${ref}` }; if (!object) return { status: 1, stdout: '', stderr: 'Error: No such object' }; + const holder = kind === 'volume' && daemon.objects.find(other => other.mounts?.includes(object.name)); + if (holder) return { status: 1, stdout: '', stderr: `Error response from daemon: remove ${object.name}: volume is in use - [${holder.id}]` }; if (daemon.refuseRemoval.has(object.name)) return { status: 1, stdout: '', stderr: 'Error: daemon refused removal' }; daemon.objects.splice(daemon.objects.indexOf(object), 1); return { status: 0, stdout: ref, stderr: '' }; @@ -341,4 +354,114 @@ describe('recoverLeftovers', () => { await expect(recoverLeftovers('A'.repeat(32))).rejects.toThrow('32 lowercase hex'); expect(daemon.calls).toEqual([]); }); + + // #181: what an interrupted path identity probe leaves, labelled as `pathIdentityProbeArgs` labels it: a container + // created but never started, and its anonymous volume. + const probe = (runner: string, labels: Record = {}, names: { container?: string; volume?: string } = {}) => { + const probeLabels = { 'io.codeboost.runner': runner, 'io.codeboost.probe': 'path-identity', ...labels }; + const volume = names.volume ?? id(); + const objects: [FakeObject, FakeObject] = [ + { kind: 'container', id: id(), name: names.container ?? `codeboost-path-identity-${randomUUID()}`, labels: probeLabels, + mounts: [volume] }, + { kind: 'volume', name: volume, labels: { 'com.docker.volume.anonymous': '', ...probeLabels } }, + ]; + daemon.objects.push(...objects); + return objects; + }; + + it('removes its own leftover path identity probe and its volume, container first, and no other runner\'s (#181)', async () => { + const [container, volume] = probe(A), theirs = probe(B), mine = attempt(A, 'attempt-a'); + const report = await recoverLeftovers(A); + const [work, metadata, keeper, seeder, agent, proxy, network] = mine.objects; + expect(report.removed.map(resource => resource.name).sort()).toEqual([container.name, volume.name, seeder!.name, + agent!.name, proxy!.name, network!.name].sort()); + expect(names()).toEqual([work!.name, metadata!.name, keeper!.name, ...theirs.map(object => object.name)].sort()); + expect(report.unowned).toEqual([]); + // The container by its full ID, then its volume by name, which it holds until the container is gone. + const removals = daemon.calls.filter(args => args[0] === 'rm' || args[1] === 'rm').map(args => args[args.length - 1]); + expect(removals.indexOf(container.id!)).toBeGreaterThanOrEqual(0); + expect(removals.indexOf(container.id!)).toBeLessThan(removals.indexOf(volume.name)); + expect(daemon.calls).toContainEqual(['volume', 'rm', '--force', volume.name]); + }); + + it.each([ + ['another probe kind', { 'io.codeboost.probe': 'other' }, {}], + ['an attempt label', { 'io.codeboost.attempt': 'attempt-a' }, {}], + ['an allocation label', { 'io.codeboost.allocation': randomUUID() }, {}], + ['a task-storage label', { 'io.codeboost.task-storage': 'work' }, {}], + ['names D does not give a probe', {}, { container: `codeboost-agent-${randomUUID()}`, volume: `codeboost-work-${randomUUID()}` }], + ] as const)('leaves probe-labelled objects of its runner with %s, and reports them (#181)', async (_label, labels, named) => { + const objects = probe(A, labels, named); + const report = await recoverLeftovers(A); + expect(report.removed).toEqual([]); + expect(names()).toEqual(objects.map(object => object.name).sort()); + expect(report.unowned.map(resource => [resource.name, resource.reason]).sort()) + .toEqual(objects.map(object => [object.name, 'unknown-kind']).sort()); + }); + + it('reports a path identity probe without a runner label and never removes it (#181)', async () => { + const objects = probe(A).map(object => { + const { 'io.codeboost.runner': _runner, ...labels } = object.labels; + object.labels = labels; + return object; + }); + const report = await recoverLeftovers(A); + expect(names()).toEqual(objects.map(object => object.name).sort()); + expect(report.unowned.map(resource => [resource.name, resource.reason]).sort()) + .toEqual(objects.map(object => [object.name, 'no-runner-label']).sort()); + }); + + it('removes a probe volume whose container is already gone (#181)', async () => { + const [container, volume] = probe(A); + daemon.objects.splice(daemon.objects.indexOf(container), 1); + const report = await recoverLeftovers(A); + expect(report.removed.map(resource => resource.name)).toEqual([volume.name]); + expect(names()).toEqual([]); + }); + + it('keeps a probe volume a container it leaves still mounts, and reports that container (#181)', async () => { + const [, volume] = probe(A); + const holder: FakeObject = { kind: 'container', id: id(), name: `codeboost-other-${randomUUID()}`, + labels: { 'io.codeboost.runner': A }, mounts: [volume.name] }; + daemon.objects.push(holder); + const report = await recoverLeftovers(A); + expect(report.removed.map(resource => resource.name)).not.toContain(volume.name); + expect(names()).toContain(volume.name); + expect(report.unowned.map(resource => [resource.name, resource.reason])).toEqual([[holder.name, 'unknown-kind']]); + }); + + it('leaves a probe volume whose runner label changed between the list and the inspect (#181)', async () => { + const [container, volume] = probe(A); + daemon.objects.splice(daemon.objects.indexOf(container), 1); + // Listed as A's; by the inspect, a same-named volume of another runner holds the name. + daemon.beforeSingleInspect = ref => { + if (ref === volume.name) volume.labels = { ...volume.labels, 'io.codeboost.runner': B }; + }; + const report = await recoverLeftovers(A, 120_000, { unowned: false }); + expect(report.removed).toEqual([]); + expect(names()).toEqual([volume.name]); + // Another runner's now: neither removed nor reported as this runner's. + expect(report.unowned).toEqual([]); + }); + + it('removes no probe volume while its container\'s removal is not confirmed (#181)', async () => { + const [container, volume] = probe(A); + daemon.refuseRemoval.add(container.name); + const failure = await recoverLeftovers(A).catch((error: unknown) => error); + expect(failure).toBeInstanceOf(RecoveryError); + expect((failure as Error).message).toContain(`Could not remove container ${container.name}`); + expect(daemon.calls.filter(args => args[0] === 'volume' && args[1] === 'rm')).toEqual([]); + expect(names()).toEqual([container.name, volume.name].sort()); + }); + + it('fails recovery, and removes no network after it, when a probe volume\'s removal is not confirmed (#181)', async () => { + const [container, volume] = probe(A), mine = attempt(A, 'attempt-a'), network = mine.objects[6]!; + daemon.refuseRemoval.add(volume.name); + const failure = await recoverLeftovers(A).catch((error: unknown) => error); + expect(failure).toBeInstanceOf(RecoveryError); + expect((failure as Error).message).toContain(`Could not remove volume ${volume.name}`); + expect((failure as InstanceType).removed.map(resource => resource.name)).toContain(container.name); + expect(names()).toContain(volume.name); + expect(names()).toContain(network.name); + }); }); diff --git a/test/path-identity-probe.test.ts b/test/path-identity-probe.test.ts index a0198512..fac148fe 100644 --- a/test/path-identity-probe.test.ts +++ b/test/path-identity-probe.test.ts @@ -11,6 +11,8 @@ const root = mkdtempSync(join(tmpdir(), 'codeboost-probe-')), bin = join(root, ' const calls = join(root, 'calls'), mode = join(root, 'mode'), landAt = join(root, 'land-at'), removed = join(root, 'removed'); const slowLand = join(root, 'slow-land'), slowAnswered = join(root, 'slow-answered'), inspects = join(root, 'inspects'), lateMark = join(root, 'late'), rmLog = join(root, 'rm-log'), slow = join(root, 'slow'), busy = join(root, 'busy'), oldCli = join(root, 'old-cli'), hangAt = join(root, 'hang-at'), hung = join(root, 'hung'), unreach = join(root, 'unreachable'), identity = join(root, 'identity'); const savedPath = process.env.PATH; +// The database's runner token, which labels the probe's container and volume (#181). +const OWNER = 'c'.repeat(32); let imageId = ''; beforeAll(() => { mkdirSync(bin, { mode: 0o700 }); @@ -90,33 +92,41 @@ const removals = () => logged().slice(1); const left = () => existsSync(landAt) && !existsSync(removed); describe('task storage probe against Docker answers', () => { + it('refuses a malformed runner token before it calls Docker (#181)', async () => { + await expect(probeTaskStoragePathIdentity(imageId, 'C'.repeat(32))).rejects.toThrow('32 lowercase hex'); + expect(logged().filter(Boolean)).toEqual([]); + }); it('reads the identity from a probe that ran, and removes nothing by hand (--rm did)', async () => { writeFileSync(mode, 'answer'); - expect(await probeTaskStoragePathIdentity(imageId)).toEqual({ caseSensitive: true, unicodeNormalization: 'none' }); + expect(await probeTaskStoragePathIdentity(imageId, OWNER)).toEqual({ caseSensitive: true, unicodeNormalization: 'none' }); expect(removals()).toEqual([]); }); it('runs on a volume made like task storage, in a container hardened like the keeper', async () => { // Any Linux volume would answer `true none`, so only the arguments show the probe measures what task storage is. writeFileSync(mode, 'answer'); - await probeTaskStoragePathIdentity(imageId); + await probeTaskStoragePathIdentity(imageId, OWNER); const run = logged()[0]!, script = run.slice(run.indexOf(' -c ') + 4); - expect(run.slice(0, run.indexOf(' -c '))).toBe(`run --rm --name ${probeName()} --read-only --user 10001:10001 --network=none --cap-drop=ALL` + expect(run.slice(0, run.indexOf(' -c '))).toBe(`run --rm --name ${probeName()}` + // The runner's token and the probe kind, on the container and on its volume, so recovery finds them (#181). + + ` --label io.codeboost.runner=${OWNER} --label io.codeboost.probe=path-identity` + + ' --read-only --user 10001:10001 --network=none --cap-drop=ALL' + ' --security-opt=no-new-privileges --security-opt=seccomp=builtin --runtime=runc --pids-limit=32 --memory=128m --cpus=.25' + ' --mount type=volume,target=/work,volume-driver=local,volume-opt=type=tmpfs,volume-opt=device=tmpfs,' - + '"volume-opt=o=size=1048576,nr_inodes=64,uid=10001,gid=10001,mode=0755,nosuid,nodev"' + + '"volume-opt=o=size=1048576,nr_inodes=64,uid=10001,gid=10001,mode=0755,nosuid,nodev",' + + `volume-label=io.codeboost.runner=${OWNER},volume-label=io.codeboost.probe=path-identity` + ` --entrypoint sh ${imageId}`); // The measuring script itself: a constant printed instead would pass on any Linux volume. expect(script).toBe(`${PATH_IDENTITY_SCRIPT} probe /work`); }); it('removes its container and volume by name when the daemon refuses the run, and confirms it is gone', async () => { writeFileSync(mode, 'refused'); - await expect(probeTaskStoragePathIdentity(imageId)).rejects + await expect(probeTaskStoragePathIdentity(imageId, OWNER)).rejects .toThrow(/^docker run failed \(exit 125\): docker: Error response from daemon: unknown or invalid runtime name: x\.\nSee 'docker run --help'\.$/); expect(removals()).toEqual([`rm --force --volumes ${probeName()}`, `container inspect ${probeName()}`]); }); it('removes a container a killed client left created but never started', async () => { writeFileSync(mode, 'created'); - await expect(probeTaskStoragePathIdentity(imageId, 2_000)).rejects.toThrow('docker run failed (client killed or not started)'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER, 2_000)).rejects.toThrow('docker run failed (client killed or not started)'); // The interleaving this test is about: the create landed before the client was killed. expect(existsSync(landAt)).toBe(true); expect(removals()).toContain(`rm --force --volumes ${probeName()}`); @@ -127,7 +137,7 @@ describe('task storage probe against Docker answers', () => { // rm included, until the window closes removes it. writeFileSync(mode, 'late'); const started = performance.now(); - await expect(probeTaskStoragePathIdentity(imageId, 2_000)).rejects.toThrow('client killed or not started'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER, 2_000)).rejects.toThrow('client killed or not started'); // The window starts when the client is stopped and ends 10 s later: the first absent answer after it is accepted. const elapsed = performance.now() - started; expect(elapsed).toBeGreaterThanOrEqual(2_000 + 10_000); @@ -144,7 +154,7 @@ describe('task storage probe against Docker answers', () => { const empty = join(root, 'empty'); mkdirSync(empty, { recursive: true }); process.env.PATH = empty; const started = performance.now(); - try { await expect(probeTaskStoragePathIdentity(imageId)).rejects.toThrow('docker run failed (client killed or not started)'); } + try { await expect(probeTaskStoragePathIdentity(imageId, OWNER)).rejects.toThrow('docker run failed (client killed or not started)'); } finally { process.env.PATH = `${bin}:${savedPath}`; } expect(performance.now() - started).toBeLessThan(2_000); expect(existsSync(calls)).toBe(false); @@ -152,14 +162,14 @@ describe('task storage probe against Docker answers', () => { it('gives up when Docker keeps reporting the container, and names it for the person to remove', async () => { writeFileSync(mode, 'created'); writeFileSync(busy, '100000'); const started = performance.now(); - const failure = await probeTaskStoragePathIdentity(imageId, 2_000, 1_000).catch((error: Error) => error); + const failure = await probeTaskStoragePathIdentity(imageId, OWNER, 2_000, 1_000).catch((error: Error) => error); // Giving up is counted from the end of the settle window, which starts when the client is stopped. expect(performance.now() - started).toBeGreaterThanOrEqual(2_000 + 10_000 + 1_000); // The interleaving this test is about: the create landed before the client was killed. expect(existsSync(landAt)).toBe(true); const name = probeName(); expect((failure as Error).message).toBe('docker run failed (client killed or not started): docker run ETIMEDOUT after 2000 ms.' - + ` The probe container ${name} may remain; remove it with docker rm --force --volumes ${name}.`); + + ` The probe container ${name} may remain; the next start's recovery removes it, or remove it now with docker rm --force --volumes ${name}.`); const cause = (failure as Error).cause as AggregateError; expect(cause.errors.map((error: Error) => error.message)).toEqual([expect.stringMatching(/^docker run failed/), `Docker did not confirm that ${name} is gone.`]); // The last thing Docker said: the inspect still found the container, or (when giving up fell due during the rm) @@ -170,12 +180,12 @@ describe('task storage probe against Docker answers', () => { it('removes nothing when the client was refused before it started', async () => { writeFileSync(mode, 'answer'); // A deadline runInProcessGroup refuses (NOT_STARTED): nothing reached Docker. - await expect(probeTaskStoragePathIdentity(imageId, 0)).rejects.toThrow('Process deadline must be a positive integer'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER, 0)).rejects.toThrow('Process deadline must be a positive integer'); expect(existsSync(calls)).toBe(false); }); it('asks again while Docker refuses the removal of a container that exists', async () => { writeFileSync(mode, 'created'); writeFileSync(busy, '2'); - await expect(probeTaskStoragePathIdentity(imageId, 2_000)).rejects.toThrow('client killed or not started'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER, 2_000)).rejects.toThrow('client killed or not started'); // The interleaving this test is about: the create landed before the client was killed. expect(existsSync(landAt)).toBe(true); expect(removals().filter(call => call.startsWith('rm ')).length).toBeGreaterThanOrEqual(3); @@ -187,7 +197,7 @@ describe('task storage probe against Docker answers', () => { it('settles at once on a CLI whose rm exits non-zero for a missing name', async () => { writeFileSync(mode, 'refused'); writeFileSync(oldCli, ''); const started = performance.now(); - await expect(probeTaskStoragePathIdentity(imageId)).rejects.toThrow('docker run failed (exit 125)'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER)).rejects.toThrow('docker run failed (exit 125)'); expect(performance.now() - started).toBeLessThan(5_000); expect(removals()).toEqual([`rm --force --volumes ${probeName()}`, `container inspect ${probeName()}`]); }); @@ -195,7 +205,7 @@ describe('task storage probe against Docker answers', () => { // Every rm and inspect hangs: each call must be cut at the time left, not at its own 10 s deadline. writeFileSync(mode, 'refused'); writeFileSync(hangAt, '0'); const started = performance.now(); - await expect(probeTaskStoragePathIdentity(imageId, 60_000, 1_000)).rejects.toThrow('may remain'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER, 60_000, 1_000)).rejects.toThrow('may remain'); // One hung call, cut at the 1 s floor, then the give-up: a floor much over 1 s at the call site fails here. expect(performance.now() - started).toBeLessThan(2_500); }, 60_000); @@ -204,7 +214,7 @@ describe('task storage probe against Docker answers', () => { // call that was running then may hang; starting another would stretch the bound by a second floor. writeFileSync(mode, 'created'); writeFileSync(busy, '100000'); writeFileSync(hangAt, String(Math.floor(Date.now() / 1000) + 12)); - const failure = await probeTaskStoragePathIdentity(imageId, 2_000, 4_000).catch((error: Error) => error); + const failure = await probeTaskStoragePathIdentity(imageId, OWNER, 2_000, 4_000).catch((error: Error) => error); // The interleaving this test is about: the create landed before the client was killed. expect(existsSync(landAt)).toBe(true); expect(readFileSync(hung, 'utf8').trim().split('\n')).toHaveLength(1); @@ -214,7 +224,7 @@ describe('task storage probe against Docker answers', () => { // Absent at every answered ask (a killed client whose create never landed), then no answer before the window ends: // absence inside the settle window proves nothing, so this is a give-up whose cause is the unanswered call. writeFileSync(mode, 'hang'); writeFileSync(hangAt, String(Math.floor(Date.now() / 1000) + 5)); - const failure = await probeTaskStoragePathIdentity(imageId, 500, 2_000).catch((error: Error) => error); + const failure = await probeTaskStoragePathIdentity(imageId, OWNER, 500, 2_000).catch((error: Error) => error); // The interleaving this test is about: an ask was answered "absent" before Docker stopped answering. expect(readFileSync(rmLog, 'utf8').trim().split('\n')[0]).toBe('missing'); const cleanup = ((failure as Error).cause as AggregateError).errors[1] as Error; @@ -226,13 +236,13 @@ describe('task storage probe against Docker answers', () => { // daemon still unreachable nothing can confirm the container is gone. writeFileSync(mode, 'unreachable'); const started = performance.now(); - const failure = await probeTaskStoragePathIdentity(imageId, 60_000, 1_000).catch((error: Error) => error); + const failure = await probeTaskStoragePathIdentity(imageId, OWNER, 60_000, 1_000).catch((error: Error) => error); expect((failure as Error).message).toMatch(/^docker run failed \(exit 125\): docker: Cannot connect to the Docker daemon[^]* The probe container codeboost-path-identity-[0-9a-f-]+ may remain;/); expect(performance.now() - started).toBeGreaterThanOrEqual(10_000); }, 60_000); it.each(['garbled', 'preceded'])('refuses a probe that exits 0 without exactly one identity line (%s)', async garble => { writeFileSync(mode, garble); - await expect(probeTaskStoragePathIdentity(imageId)).rejects.toThrow('The path identity probe did not report an identity.'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER)).rejects.toThrow('The path identity probe did not report an identity.'); }); it.each([ ['false none', { caseSensitive: false, unicodeNormalization: 'none' }], @@ -241,26 +251,26 @@ describe('task storage probe against Docker answers', () => { ] as const)('reports what the container measured: %s', async (line, expected) => { // Each field on its own: a probe that returned tmpfs's answer whatever the container said would fail here. writeFileSync(mode, 'answer'); writeFileSync(identity, `codeboost-path-identity ${line}\n`); - expect(await probeTaskStoragePathIdentity(imageId)).toEqual(expected); + expect(await probeTaskStoragePathIdentity(imageId, OWNER)).toEqual(expected); }); it('cuts each cleanup call at 10 s, so a daemon that answers again is asked again', async () => { // A killed client (10 s settle window), then every rm and inspect hangs. With the cap the first call is cut at // 10 s and a second runs until giving up; without it the first would hang all the way to the give-up time. writeFileSync(mode, 'hang'); writeFileSync(hangAt, '0'); - await expect(probeTaskStoragePathIdentity(imageId, 500, 2_000)).rejects.toThrow('may remain'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER, 500, 2_000)).rejects.toThrow('may remain'); expect(readFileSync(hung, 'utf8').trim().split('\n')).toEqual(['rm', 'container']); }, 60_000); it('gives each cleanup call at least 1 s, so a slow daemon is still heard at the give-up time', async () => { // Almost no time left: without the floor the first call would be cut before Docker could answer it. writeFileSync(mode, 'refused'); writeFileSync(slow, ''); - const failure = await probeTaskStoragePathIdentity(imageId, 60_000, 1).catch((error: Error) => error); + const failure = await probeTaskStoragePathIdentity(imageId, OWNER, 60_000, 1).catch((error: Error) => error); const cleanup = ((failure as Error).cause as AggregateError).errors[1] as Error; expect((cleanup.cause as { status: number | null }).status).not.toBeNull(); }, 60_000); it('judges the settle window by when an inspect was asked, not when it answered', async () => { // An absent answer asked inside the window, but arriving after it, while the create lands: not proof of absence. writeFileSync(mode, 'hang'); writeFileSync(slowLand, ''); - await expect(probeTaskStoragePathIdentity(imageId, 500)).rejects.toThrow('client killed or not started'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER, 500)).rejects.toThrow('client killed or not started'); // The interleaving this test is about: the slow inspect answered "absent" (it was not cut at its deadline), the // create landed while it ran, and cleanup asked again after it. expect(existsSync(slowAnswered)).toBe(true); @@ -273,7 +283,7 @@ describe('task storage probe against Docker answers', () => { // The production default: an unreachable daemon (no daemon answer, so the 10 s window applies), then 30 s of asks. writeFileSync(mode, 'unreachable'); const started = performance.now(); - await expect(probeTaskStoragePathIdentity(imageId)).rejects.toThrow('may remain'); + await expect(probeTaskStoragePathIdentity(imageId, OWNER)).rejects.toThrow('may remain'); const elapsed = performance.now() - started; // 10 s settle + 30 s of asks; each fake call answers at once, so the overshoot is the last pause and call. expect(elapsed).toBeGreaterThanOrEqual(40_000); @@ -296,7 +306,7 @@ describe('task storage probe against Docker answers', () => { process.env.DOCKER_HOST = 'x'.repeat(4 * 1024 * 1024); const started = performance.now(); let failure: Error | undefined; - try { await probeTaskStoragePathIdentity(imageId); } catch (error) { failure = error as Error; } + try { await probeTaskStoragePathIdentity(imageId, OWNER); } catch (error) { failure = error as Error; } finally { if (saved === undefined) delete process.env.DOCKER_HOST; else process.env.DOCKER_HOST = saved; } // The path this test is about: the synchronous throw, not the asynchronous `spawn docker` error event. expect((failure!.cause as NodeJS.ErrnoException).syscall).toBe('spawn'); diff --git a/test/runner-production.test.ts b/test/runner-production.test.ts index bf33693d..6da9ebf3 100644 --- a/test/runner-production.test.ts +++ b/test/runner-production.test.ts @@ -73,11 +73,13 @@ function fixture() { return { root, service, config, demo }; } const recovery = (calls: string[], over: Partial = {}): RecoveryDeps => ({ - recoverLeftovers: async () => { calls.push('recover'); return { storage: [], unowned: [] }; }, + recoverLeftovers: async runnerOwner => { calls.push('recover'); owners.push(runnerOwner); return { storage: [], unowned: [] }; }, exportTaskDiff: async () => ({ diff: Buffer.alloc(0), truncated: false }), removeTaskFilesystems: async () => undefined, ...over, }); +/** The runner tokens recovery ran under, in order. */ +const owners: string[] = []; const lock = (calls: string[], fail = false) => ({ file: { dev: 1n, ino: 2n }, verify: () => { calls.push('verify'); if (fail) throw new Error('The database path changed while opening. Refusing to start.'); } }); const capability = { run: (fn: () => T) => fn() }; // Task storage that agrees with the configured identity, so these tests need no Docker (the real probe is in runner-workspace). @@ -107,11 +109,12 @@ describe('runner startup', () => { const calls: string[] = []; await expect(setUpRunner({ service, capability, config: { root: join(root, 'runner'), committer }, lock: lock(calls), env: { CLAUDE_CODE_OAUTH_TOKEN: 't' }, buildImage: () => { calls.push('image'); return 'sha256:x'; }, recovery: () => { calls.push('deps'); return recovery(calls); }, - probeTaskStorage: async imageId => { calls.push(`probe ${imageId}`); return probed; } })) + probeTaskStorage: async (imageId, runnerOwner) => { calls.push(`probe ${imageId} ${runnerOwner}`); return probed; } })) .rejects.toThrow(`pathIdentity (caseSensitive ${configured.caseSensitive}, unicodeNormalization ${configured.unicodeNormalization}) does not match ` + `the task storage the agent edits (caseSensitive ${probed.caseSensitive}, unicodeNormalization ${probed.unicodeNormalization})`); - // Probed with the built image, after recovery and the image build. - expect(calls).toEqual(['verify', 'deps', 'recover', 'image', 'probe sha256:x']); + // Probed with the built image, after recovery and the image build, labelled with the token recovery swept (#181). + expect(calls).toEqual(['verify', 'deps', 'recover', 'image', `probe sha256:x ${owners.at(-1)}`]); + expect(owners.at(-1)).toMatch(/^[0-9a-f]{32}$/); } // A probe that fails stops startup too: an identity that cannot be measured is not assumed. await expect(setUpRunner({ service, capability, config: { root: join(root, 'runner'), committer }, lock: lock([]), env: { CLAUDE_CODE_OAUTH_TOKEN: 't' }, diff --git a/test/runner-workspace.test.ts b/test/runner-workspace.test.ts index 91e78548..edf7bdcf 100644 --- a/test/runner-workspace.test.ts +++ b/test/runner-workspace.test.ts @@ -6,7 +6,8 @@ import { join } from 'node:path'; import { promisify } from 'node:util'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { buildAgentImage } from '../agents/container/image.ts'; -import { PROBE_VOLUME_MOUNT, probeTaskStoragePathIdentity, type TaskFilesystems } from '../agents/container/storage.ts'; +import { pathIdentityProbeArgs, probeTaskStoragePathIdentity, type TaskFilesystems } from '../agents/container/storage.ts'; +import { recoverLeftovers } from '../agents/recovery.ts'; import type { Plan, PlanContext } from '../core/plan.ts'; import { RunnerCoordinator } from '../runner/coordinator.ts'; import { ItemExecutor, SafetyFindings, executionDeps, type ExecutionSources } from '../runner/execution.ts'; @@ -35,8 +36,8 @@ afterAll(() => { for (const store of stores) store.close(); for (const root of roots) rmSync(root, { recursive: true, force: true }); } finally { - // Then remove the task storage this run still owns, even when a test failed or the cleanup above threw. The path - // identity probe carries no owner labels, so it is not found here. It may not run before a hook timeout. + // Then remove the task storage and path identity probes this run still owns, even when a test failed or the cleanup + // above threw. It may not run before a hook timeout. removeRunnerObjects(RUNNER_OWNER); } }, 4 * 60_000); @@ -156,10 +157,10 @@ describe('real task workspace (#87)', () => { }); describe('task storage path identity (#162)', () => { - // The probe names its container itself and gives it an anonymous volume, with no labels. This test finds both from - // the name the probe gave: a shim on PATH records it, and the daemon's events give that container's ID and the - // volumes mounted in it. Another probe on the daemon (another runner's startup, or this file run from another - // checkout) or a container a dead runner left behind does not count. + // The probe names its container itself and gives it an anonymous volume. This test finds both from the name the probe + // gave, not by the runner label they carry (#181): a shim on PATH records it, and the daemon's events give that + // container's ID and the volumes mounted in it. Another probe on the daemon (another runner's startup, or this file + // run from another checkout) or a container a dead runner left behind does not count. async function watchedProbe() { const shim = mkdtempSync(join(tmpdir(), 'docker-shim-')); roots.push(shim); const names = join(shim, 'names'), realDocker = execFileSync('sh', ['-c', 'command -v docker'], { encoding: 'utf8' }).trim(); @@ -183,7 +184,7 @@ describe('task storage path identity (#162)', () => { const path = process.env.PATH; process.env.PATH = `${shim}:${path}`; try { - const identity = await probeTaskStoragePathIdentity(imageId); + const identity = await probeTaskStoragePathIdentity(imageId, RUNNER_OWNER); const recorded = readFileSync(names, 'utf8').split('\n').filter(Boolean); expect(recorded).toEqual([expect.stringMatching(/^codeboost-path-identity-[0-9a-f-]{36}$/)]); const container = () => events.find(event => event.Type === 'container' && event.Action === 'create' @@ -214,22 +215,51 @@ describe('task storage path identity (#162)', () => { expect(() => docker('container', 'inspect', probe.container!)).toThrow(/No such container/); expect(() => docker('volume', 'inspect', probe.volumes[0]!)).toThrow(/no such volume/i); }, 120_000); + // A probe container created but never started, as a killed client leaves it: the probe's own arguments, with `create` + // in place of `run` (`--rm` stays). Returns its name and the anonymous volume its mount made. + const leftoverProbe = (runnerOwner: string) => { + const name = `codeboost-path-identity-${randomUUID()}`, args = pathIdentityProbeArgs(imageId, runnerOwner, name); + expect(args.slice(0, 2)).toEqual(['run', '--rm']); + docker('create', ...args.slice(1)); + return { name, volume: docker('container', 'inspect', '--format', '{{range .Mounts}}{{.Name}}{{end}}', name) }; + }; it('removes a never-started probe container and its volume with the cleanup\'s own rm', () => { // The failure path's claim: a container created but never started (a killed client) loses its anonymous volume to - // `rm --force --volumes`. Made with the probe's exact mount; removed with the exact command the cleanup runs. - const name = `codeboost-path-identity-${randomUUID()}`; - docker('create', '--name', name, '--mount', PROBE_VOLUME_MOUNT, '--entrypoint', 'true', imageId); - let volume = ''; + // `rm --force --volumes`. Made with the probe's exact arguments; removed with the exact command the cleanup runs. + let probe: { name: string; volume: string } | undefined; try { - // This container's own volume, by name: the anonymous volume the mount made. - volume = docker('container', 'inspect', '--format', '{{range .Mounts}}{{.Name}}{{end}}', name); - expect(volume).toMatch(/^[0-9a-f]{64}$/); - expect(docker('volume', 'inspect', '--format', '{{json .Options}}', volume)).toContain('size=1048576,nr_inodes=64,'); - } finally { docker('rm', '--force', '--volumes', name); } - expect(() => docker('volume', 'inspect', volume)).toThrow(); + probe = leftoverProbe(RUNNER_OWNER); + expect(probe.volume).toMatch(/^[0-9a-f]{64}$/); + expect(docker('volume', 'inspect', '--format', '{{json .Options}}', probe.volume)).toContain('size=1048576,nr_inodes=64,'); + } finally { if (probe) docker('rm', '--force', '--volumes', probe.name); } + expect(() => docker('volume', 'inspect', probe.volume)).toThrow(); + }, 120_000); + it('removes a probe a killed runner left, with its volume, in the next startup\'s recovery, and no other runner\'s (#181)', async () => { + // Tokens of their own: recovery removes every agent container of its runner, and this file's may have some. + const mine = randomBytes(16).toString('hex'), theirs = randomBytes(16).toString('hex'); + try { + const left = leftoverProbe(mine), other = leftoverProbe(theirs); + // Both parts carry the runner's token and the probe kind. + for (const labels of [docker('container', 'inspect', '--format', '{{json .Config.Labels}}', left.name), + docker('volume', 'inspect', '--format', '{{json .Labels}}', left.volume)]) + expect(JSON.parse(labels)).toMatchObject({ 'io.codeboost.runner': mine, 'io.codeboost.probe': 'path-identity' }); + // A container of this runner, named like a probe, without the probe kind label: reported, never removed. + const unmarked = `codeboost-path-identity-${randomUUID()}`; + docker('create', '--name', unmarked, '--label', `io.codeboost.runner=${mine}`, '--entrypoint', 'true', imageId); + const report = await recoverLeftovers(mine, 120_000, { unowned: false }); + expect(report.removed.map(resource => resource.name).sort()).toEqual([left.name, left.volume].sort()); + expect(() => docker('container', 'inspect', left.name)).toThrow(/No such container/); + expect(() => docker('volume', 'inspect', left.volume)).toThrow(/no such volume/i); + expect(report.unowned).toEqual([expect.objectContaining({ name: unmarked, reason: 'unknown-kind' })]); + for (const ref of [other.name, unmarked]) expect(docker('container', 'inspect', '--format', '{{.State.Status}}', ref)).toBe('created'); + expect(docker('volume', 'inspect', '--format', '{{.Name}}', other.volume)).toBe(other.volume); + } finally { + removeRunnerObjects(mine); + removeRunnerObjects(theirs); + } }, 120_000); it('runs only the image the trusted builder produced', async () => { - await expect(probeTaskStoragePathIdentity(`sha256:${'0'.repeat(64)}`)).rejects.toThrow('Agent image was not produced by the trusted validated builder.'); - await expect(probeTaskStoragePathIdentity('alpine')).rejects.toThrow('requires the immutable built image ID'); + await expect(probeTaskStoragePathIdentity(`sha256:${'0'.repeat(64)}`, RUNNER_OWNER)).rejects.toThrow('Agent image was not produced by the trusted validated builder.'); + await expect(probeTaskStoragePathIdentity('alpine', RUNNER_OWNER)).rejects.toThrow('requires the immutable built image ID'); }); });