Skip to content

Commit 90d8cec

Browse files
mchwangclaude
andauthored
D: scoped crash recovery with storage recovery handles (#51 item 4) (#72)
* Bound D cleanup retries so settlement always ends (#51 item 1) The supervisor retried unconfirmed container, network and setup cleanup every second with no limit, so an unreachable Docker daemon kept `settled` pending forever. Retries now stop 60 s after the first failure, and the handle settles with its stop reason and an `unreleased` list of resources whose removal was not confirmed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Fail closed on unreleased resources and report them precisely Fixes from two review rounds on the bounded-settlement change: - Ask counted a result with `unreleased` as an ordinary failure and could start another invocation. It now marks the resources as untracked leftovers, which turns Ask off until a restart finds none. - Container cleanup ran `spawnSync` without SIGKILL, so a Docker client that ignores SIGTERM could block past its deadline and keep `settled` pending. Cleanup subprocesses are now killed, and retries after the first get only what is left of the 60 s window. - The agent container was reported whenever container-level cleanup ran, including after a create the daemon refused because another invocation holds the name. It is now reported only if this profile's create ran. - Each unreleased Docker object now carries its creation-time ID and its ownership label, so recovery can check both instead of trusting a name. - A profile that settled with unconfirmed cleanup can no longer launch again. - Docs no longer claim startup recovery already removes these resources. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Return D start handles at once and run Docker setup inside them (#51 item 2) The Claude and Codex start calls created the vendor network, the container profile and the container with execFileSync/spawnSync, so the server could not begin shutdown or cancel while they ran. Now: - agents/docker.ts runs Docker calls asynchronously, killing the client with SIGKILL at its deadline or when its AbortSignal fires. - Network, profile and container create/validate/dispose are async; setup calls take an AbortSignal, and cleanup is never cancelled. - startProfileInvocation returns its handle immediately and creates and validates the container inside it; a stop during setup kills the in-flight Docker call, cleans up, then settles. - launchInvocation gives the adapters the same shape for network and profile creation; setUpProfile replaces the duplicated adapter setup code. - A start call throws only when it allocated nothing; every other setup failure settles the handle with a stop reason and a diagnostic. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep cleanup retries inside the window and report only unconfirmed parts Review fixes (Copilot, #69): - Retained setup cleanup ignored the budget the retry loop computed, so a retry could start near the end of the window and run a fresh 30 s. The budget now reaches every cleanup callback (network creation, profile creation, adapter setup), and no attempt starts once the window has ended. - A vendor network was reported as proxy plus network even after one part was confirmed removed, or when a create was refused. Removal is now tracked per object, and cleanup-error inventories are read when the window ends. - Ask turned off only for a non-empty `unreleased`; an empty list also means cleanup was not confirmed, so presence is now the signal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Stop the handed-off handle when the launch budget ends Review fix (Copilot, #70): launchInvocation cleared its deadline timer once setup handed off to the supervisor, so a supervisor started without the adapter's invocationBudget could outlive the launch budget. The timer now lives until the handle settles and cancels the handed-off handle with `timeout`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Remove and control the agent container by its ID, never its name Review fix (Copilot, #69): cleanup inspected the agent container by name and then ran `rm --force` by name, so a same-named replacement created between the two calls could be deleted. Cleanup now looks the container up by the ID that `docker create` returned (by name only when the create's outcome is unknown), checks the ownership label, and removes and confirms by the ID the daemon reported. The supervisor's validate, start, exec, stop, kill and rm calls and the Codex output read also target that ID. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Validate and start the agent container by its ID on every launch path Review fix (Copilot, #69): createValidatedContainer and the exported startValidatedContainer/runContainer path still validated and started the container by `profile.name` after the create, so a same-named replacement could run. Both now use the ID `docker create` returned and fail if it is unknown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Release a profile's own resources when its container create was refused Review fix (Copilot, #70 summary; the code is on this branch): when `docker create` was refused because another invocation holds the name, the profile owned no container, but cleanup still inspected that name, found the foreign container, and refused. The profile's network and staging (including the Codex credential copy) were then retried and reported as unreleased instead of being removed. A profile whose create never ran or was refused now skips the container step and removes only its own resources. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Await profile cleanup when no container create ran Review fix (Copilot, #70): the merge of the refused-create fix brought its synchronous `disposeContainerProfile` call into this async branch without an `await`, so removeContainerOrThrow resolved before the profile's network and staging were removed, and a cleanup failure was detached. It is now awaited, so a refused create releases the profile before the handle settles. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Do not clean up a vendor network or proxy whose create the daemon refused Review fix (Copilot, #70 summary; the code is on this branch): after a daemon-confirmed create failure, network setup cleanup still tried to remove the object by name. If the name belonged to someone else, cleanup refused, and the refused create came back as an unsettled cleanup to retry. Cleanup and the unreleased inventory now consider an object only if its create succeeded or its client was killed before the daemon answered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Report a setup failure caused by the deadline as a timeout Review fix (Copilot, #70 summary): during asynchronous setup, a Docker client killed at the invocation deadline can reject a moment before the supervisor's or launcher's own deadline timer fires, so setup was reported as `capture-failure` instead of `timeout`. A setup error caused by a deadline (ETIMEDOUT, or an expired budget, including through `cause` and the startup half of a cleanup aggregate) within 1 s of the invocation deadline is now a timeout. Earlier Docker timeouts, such as a 60 s step cap, stay failures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Pin the container identity rule for profile-only cleanup retries Review follow-up (Copilot, #69): cleanup already falls back to the container name only for a create that ran with an unknown ID, and drops the ID record only after the container's removal is confirmed, so a retry of the profile's network or staging cleanup is profile-only. Document that invariant and add a real-Docker regression: our container is removed, the network removal fails, a same-named, same-labelled container appears, and the retry leaves it alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Remove egress objects by inspected ID; keep a proxy whose start failed Review fixes (Copilot, #69): - Network and proxy cleanup inspected an object (by name when its create ID was unknown), checked the label, then removed and confirmed by that name, so a same-named replacement created in between could be deleted. Removal and confirmation now use the ID from that inspect, which must match the target when the target is an ID. - `docker run --detach` can create the proxy and then exit nonzero because the start failed. The refused-create rule treated that answered failure as "nothing created" and leaked the container. A failed proxy run now counts as possibly created, so cleanup looks it up by name behind the label check and reports it if removal is not confirmed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Refuse every launch path for a profile that settled with leftovers Review fix (Copilot, #69): the guard against relaunching a profile whose invocation settled with `unreleased` resources lived only in startProfileInvocation. createValidatedContainer, startValidatedContainer and runContainer could still create or start the leftover container that recovery owns. The retired-profile state now lives in run.ts and every launch entry point checks it before doing any work; cleanup remains allowed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Create and start the egress proxy separately; document Ask's host recovery Review fixes (Copilot, #69): - `docker run --detach` mixed two outcomes: a refused create (for example, a name held by another container) made nothing, while a failed start left our container. Treating both as "created" reported and inspected a foreign same-named container. The proxy is now created, then started: a refused create is absence, a failed start leaves a container owned by its ID. - Ask's handling of D's host staging directories was questioned. They are recovered through the Ask root (the worker's TMPDIR, recorded durably before setup and deleted at startup before Ask is enabled); say so at the point where Ask marks unreleased results. - Refresh the isolation doc's `unreleased` and retired-profile rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Refuse an untrusted image synchronously; fix stale launch docs Review fixes (Copilot, #70 summary): - The start-call rule is "throw only when nothing was allocated, for example invalid input", but an untrusted image ID was only rejected inside asynchronous setup, so the handle settled with a failure instead. Both adapters now check the image before launching. - runner-lifecycle.md still said D's adapters throw after their setup cleanup succeeds, and that shutdown may await a start call in progress. Both now describe the asynchronous start: it returns at once, and setup failures settle the handle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Treat a Docker client that never started as having created nothing Review fix (Copilot, #69 summary): a failed create with no exit status was read as "the request may have reached the daemon". A spawn failure (ENOENT, EACCES, EAGAIN and similar) sent nothing, but it too was treated as unknown, so cleanup waited out a settle window and could report a never-created agent container, network or proxy as unreleased. One shared classifier (`createOutcomeUnknown`) now separates a daemon answer, a client that never started, and a killed client, for both the agent container and the egress objects. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Do not treat a create cancelled before it started as possibly created Review fix (Copilot, #70 summary: network cancellation race): when cancel() landed between two setup steps, the next Docker call saw an aborted signal and returned without starting a client. That refusal carried no exit status or spawn code, so the create classifier read it as "outcome unknown": the create that never ran (for example the proxy) counted as possibly created, the cancel waited out a settle window, and the object could be reported as unreleased. runDocker now marks calls it refuses before spawning with `ENOTSTARTED`, which the classifier treats as nothing created. A cancel that kills a client already in flight still leaves the outcome unknown. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep the daemon's exit status when a cancel lands after the client exited Review fix (Copilot, #70): runDocker checked the AbortSignal's current state before trusting a numeric exit code. If the Docker client had already exited with a daemon answer (for example a name conflict) and cancel() ran before the callback, the answer was discarded and the call was treated as killed with an unknown outcome, so a refused create counted as possibly created. Only Node's own abort error now marks a call as cancelled. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Retry the profile cleanup once per attempt in the adapters Review fix (Copilot, #69): when profile creation and its cleanup both failed, each adapter retry ran the profile's cleanup (which already removes the vendor network) and then removed that network again with a fresh full budget, so a retained retry could run past the shrinking cleanup window. The adapters now retain exactly the profile's cleanup with the window's budget, as the network-creation path already did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Make assertVendorNetwork async so a bad deadline rejects Fixes the real-docker CI failure on #70 (agent-network "rejects a copied network capability"): assertVendorNetwork returned a promise but was not declared async, so an invalid timeout threw synchronously from deadline() instead of rejecting. Every other async D entry point already rejects. 198 of 200 real-Docker tests passed in that run; this was the only failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Address review suggestions: shared Docker ID check, accurate bounds Review of #69: - disposeValidatedContainer's comment said the container and network steps had separate budgets; they share one deadline. Comment corrected. - The 64-hex Docker ID check was written three times across run.ts and network.ts. One DOCKER_ID in agents/client-outcome.ts now backs all ownership checks. - agent-isolation.md now says cleanup retries are still synchronous (up to 30 s each), so the window bounds the total wait, not responsiveness, until #51 item 2. - The settlement bound is stated as a worked number (about 90 s after cleanup starts: 30 s first attempt, then the 60 s window) in contract.ts, agent-isolation.md and runner-lifecycle.md. - agent-isolation.md documents that a start call whose profile creation and cleanup both fail now returns a retaining handle instead of re-throwing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Release the profile if the supervisor handoff throws; add signals to runContainer Review fixes (Copilot, #70): - setUpProfile returned `start(profile)` unguarded. If the handoff threw after the profile was created, the launcher treated it as a failure with nothing allocated and the profile's network and staging leaked. It now disposes the profile, or hands a retryable cleanup to the launcher. - startValidatedContainer and runContainer, the lower-level launch path, took no AbortSignal. Both now accept one and pass it to create, validate and start; cleanup still runs after a cancel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Label every D resource with its runner, attempt and allocation (#51 item 3) - InvocationInput gains a required runnerOwner (32 lowercase hex), checked by captureInvocation. - agents/labels.ts defines io.codeboost.runner, io.codeboost.attempt and io.codeboost.allocation, validates owners, and builds and checks the labels. - prepareTaskFilesystems takes an owner with a caller-chosen allocation ID (lowercase UUID v4) instead of generating one; its volumes, keeper and seeder carry all three labels. It also classifies failed allocations with createOutcomeUnknown. - createVendorNetwork takes a caller-chosen allocation ID (the adapters' request carries networkAllocationId); the network and proxy carry the invocation's runner and attempt and that allocation. - The agent container carries its own attempt and the task-storage allocation it mounts. Profile creation refuses storage from another runner. - Every validator checks the owner labels. - Ask passes a random per-session owner until F1d's per-database token exists, and fresh allocation IDs per question. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Refuse label-incompatible attempt IDs at capture; claim allocation IDs once Review fixes (#71 code review): - captureInvocation accepted any non-empty attempt ID, but the owner labels need a narrower form, so a bad ID was only rejected inside asynchronous setup and settled the handle instead of throwing. It is now refused at capture, after the existing identity check. - One allocation ID could name two allocations, which item 4's recovery would merge. claimAllocationId (labels.ts) now refuses a reused ID, shared by task storage and vendor networks, before anything is created. - Drop the stale doc comment on prepareTaskFilesystems and the duplicate allocationId field; the owner is the single source. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Check allocation reuse at the daemon; test every owner-label check Review fixes (#71 high-effort review): - Allocation-ID reuse was only caught within one process. Both allocators now ask the daemon (`allocationListCommands`) for any container, volume or network still labelled with the ID before creating anything, failing closed if Docker does not answer. The in-process claim now covers only allocations in progress (or whose cleanup did not settle), so it stays bounded. - UnreleasedResource reports every ownership label as `labels` (runner, attempt, allocation, and the invocation or egress label) instead of a single `owner` label, so recorded leftovers match what recovery will scan. The agent container's labels are captured at create time, so reporting cannot throw while a handle settles. - validateContainer computes the storage owner once and drops the allocation comparisons that hasOwnerLabels already makes. - New real-Docker tests strip the runner label from the real Docker call for the network, proxy, task volumes, keeper and agent container, and check that each validator rejects the result. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Test network allocation reuse; parallel, literal allocation lists Review suggestions (#71): - Add a real-Docker test that a second vendor network with a live network's allocation ID is refused before anything is created. - The network's three daemon list calls run in parallel. - allocationListCommands spells out its three commands instead of splitting strings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Check every owner label before cleanup; re-check allocation IDs after the first create Cleanup of task storage and of a name-resolved vendor network or proxy now requires the runner, attempt and allocation labels together. Right after its first create, each allocator checks that its object is the only one carrying the allocation ID, so two processes racing past the first check back out. The adapters validate networkAllocationId before returning a handle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Rewrap the network remove comment Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Remove by captured ID regardless of labels; require every label by name A captured Docker ID proves the object is the one D created, so network, proxy and agent-container cleanup removes it even when validation refused its labels. An object found only by name must carry every label D wrote, including the agent container's owner labels. Storage cleanup targets only names whose create succeeded or whose outcome is unknown; tests that build mislabelled storage now expect cleanup to refuse it and remove it by hand. Document the limits of the allocation uniqueness checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Let the Docker test shim's script run the command itself The killed-create tests edit the command before it runs; the shim ran the unedited command first, so a correctly labelled object was created and the edited create then failed on the name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Describe cleanup ownership as ID proof or all labels by name Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Remove task containers by the ID just inspected Storage cleanup inspected a keeper or seeder by name, then removed it by name, so a same-named replacement created in between could be deleted. It now removes and confirms by the inspected ID. Volumes have no ID beyond their name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document the ID-or-labels ownership rule on UnreleasedResource Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Skip label checks on captured-ID container cleanup; release claims after a settled retry The agent container's cleanup still required the invocation label when its captured ID already proved ownership. A network setup cleanup that failed and later settled on retry never released its allocation claim. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * State that unreleased labels are the labels D wrote, not observed ones Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Create and start the task keeper separately; clean up a run that failed after creating A keeper whose docker run created the container but failed to start it was tracked as never created, so cleanup skipped it and its volume references kept both volumes alive. The keeper is now created (tracked by name, with its ID returned) and started as a separate step. A seeder docker run that fails after the daemon answered may also have created its container, so cleanup checks it by name and owner labels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Recover a runner's leftovers after a crash, keeping task storage behind handles (#51 item 4) recoverLeftovers(runnerOwner) acts only on objects labelled with that runner token. It removes agent containers, egress proxies, seeders and networks by ID, networks last, and resolves once they are gone. Task storage (volumes and keeper) is kept whole and returned as one RecoveredTaskStorage handle per allocation, issued only after every part's owner labels are checked; removeTaskFilesystems accepts it. Objects without a runner label, objects of this runner that D does not create, and storage whose parts disagree are reported as unowned and never touched. An unconfirmed removal rejects with a bounded RecoveryError and adopts nothing. The seeder now carries a task-storage label so recovery can tell it from the keeper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Refuse recovery with live storage; adopt asynchronously; batch inspects; report partial removals recoverLeftovers refuses to run while this process holds task storage of the runner, and adoption refuses storage this process allocated, so recovery cannot remove storage or agents a live invocation uses. Storage adoption now runs asynchronously. Inspections go in batches of 200. RecoveryError lists what was confirmed removed. The real-Docker test builds the crashed runner's leftovers with the Docker CLI, including a stray seeder and storage with no keeper, and checks that recovering a runner with live storage is refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Recover only objects with exactly one D kind label; require full IDs Recovery removed any runner-owned container carrying the invocation or egress label, even alongside a task-storage label D never writes with it. Now each object must carry exactly one kind label with a value D writes, or it is reported as unknown-kind and left. A container or network inspected without a full ID fails recovery closed instead of being removed by an undefined ID. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Remove only runtime objects with complete owner labels Recovery has no creation-time ID to prove ownership, so an agent container, proxy, seeder or network is now removed only when it carries a label-safe attempt ID and a UUID v4 allocation ID, and an egress object's egress label equals its allocation. Anything else is reported as unknown-kind. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Require D's invocation label value; fail recovery closed when a storage check cannot reach Docker An agent container is recovered only when its invocation label is the UUID v4 D writes. Storage adoption now tells a daemon-answered rejection (RecoveredStorageRejected, reported as inconsistent-storage) from a failed or timed-out inspect, which rejects the whole recovery. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Bring the lifecycle recovery rows in line with the implementation Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Tolerate objects removed mid-scan; register recovery test cleanup as objects are made Recovery's daemon-wide scans list other runners' objects, which can be removed before the batch inspect runs; that aborted recovery even with nothing to recover. A failed batch is now inspected one object at a time, skipping objects Docker confirms are gone and still failing closed on any other error. The real-Docker recovery test registers each object for cleanup as soon as it exists and runs cleanup in reverse, covering setup failures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Remove a recovered keeper by the ID recovery checked Adoption now keeps the keeper's inspected ID with the handle, and removeTaskFilesystems removes the keeper by that ID, so a keeper removed and replaced under the same name before release is never touched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Adopt the keeper recovery scanned, by its ID, not whatever holds the name Recovery passes the keeper ID from its scan into adoption, which inspects that ID, requires it to still carry the keeper's name, and refuses the storage if the keeper is gone or changed. A keeper replaced under the same name between the scan and adoption is no longer adopted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Count task storage as live from its first create until removal is confirmed A failed allocation whose cleanup did not settle still owns what it made, but was never counted live, so recovery in the same process could adopt and remove it. Storage is now live from before the first create; a failed allocation stops being live only once its cleanup is confirmed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Make the client-outcome test's fake docker executable writeFileSync applies mode only on create, and an earlier test in the file had already created a non-executable docker there, so the real Docker CLI ran instead and the result depended on its daemon. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Adopt only storage names D generates: a fixed prefix and a UUID v4 The recovery name patterns accepted any 36 hex-or-hyphen characters. They now use the same lowercase UUID v4 pattern as allocation IDs (UUID_V4 in labels.ts). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Scan every Docker type for all codeboost labels; update the lifecycle status paragraph Objects without a runner label were found only by the labels D puts on their Docker type, so a volume carrying only an invocation label was never reported. Each type is now scanned for all four codeboost labels. The lifecycle doc's status paragraph now records scoped recovery (#51 item 4) as done. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep only a keeper ID the keeper inspection confirmed adoptRecoveredTaskStorage accepted a keeper ID with no keeper part and kept it for cleanup, so a storage handle could reach a container never checked as a keeper. A keeper ID now requires the keeper it names, and only the ID the keeper inspection confirms is kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Document every way recoverLeftovers rejects Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent e6557fe commit 90d8cec

8 files changed

Lines changed: 833 additions & 20 deletions

File tree

‎agents/container/storage.ts‎

Lines changed: 114 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ import type { TaskClone } from '../contract.ts';
66
import { assertTaskClone } from '../../git/clone.ts';
77
import { assertBuiltAgentImage } from './image.ts';
88
import { createOutcomeUnknown, DOCKER_ID } from '../client-outcome.ts';
9+
import { runDocker } from '../docker.ts';
910
import { ALLOCATION_IN_USE, allocationListCommands, assertResourceOwner, claimAllocationId, hasOwnerLabels,
10-
ownerLabelArgs, releaseAllocationId, type ResourceOwner } from '../labels.ts';
11+
ownerLabelArgs, releaseAllocationId, type ResourceOwner, UUID_V4 } from '../labels.ts';
1112

1213
export interface TaskFilesystems {
1314
readonly keeper: string;
@@ -34,6 +35,13 @@ interface AllocationIdentity {
3435
readonly limits: Readonly<TaskStorageLimits>;
3536
}
3637
const allocations = new WeakMap<TaskFilesystems, AllocationIdentity>();
38+
// Storage this process is allocating or has allocated, and has not confirmed removed (including a failed allocation
39+
// whose cleanup did not settle), as allocation ID to runner token. Recovery never runs for a
40+
// runner that has any, and never issues a handle for one: a running agent may mount it.
41+
const liveAllocations = new Map<string, string>();
42+
/** Whether this process holds task storage of this runner that it allocated and has not removed. */
43+
export const hasLiveTaskStorage = (runnerOwner: string): boolean =>
44+
[...liveAllocations.values()].includes(runnerOwner);
3745
const dockerEnvironment = () => ({ PATH: process.env.PATH, DOCKER_HOST: process.env.DOCKER_HOST });
3846
const validLimit = (value: number, name: string) => {
3947
if (!Number.isSafeInteger(value) || value < 1) throw new Error(`${name} must be a positive integer.`);
@@ -224,6 +232,8 @@ export function prepareTaskFilesystems(clone: TaskClone, limits: TaskStorageLimi
224232
// Nothing created yet: a reused ID (still labelling objects from any earlier process) is refused here.
225233
try { assertAllocationObjects(allocationId, 0, remaining); }
226234
catch (error) { releaseAllocationId(allocationId); throw error; }
235+
// Live from before the first create: a failed allocation whose cleanup does not settle still owns what it made.
236+
liveAllocations.set(allocationId, owner.runnerOwner);
227237
const workVolume = `codeboost-work-${randomUUID()}`, metadataVolume = `codeboost-metadata-${randomUUID()}`;
228238
const keeper = `codeboost-keeper-${randomUUID()}`, seeder = `codeboost-seeder-${randomUUID()}`;
229239
// Names whose create succeeded, or whose client was killed so the object may exist. Cleanup touches only these: a
@@ -269,7 +279,7 @@ export function prepareTaskFilesystems(clone: TaskClone, limits: TaskStorageLimi
269279
'--entrypoint', 'sleep', imageId, 'infinity']);
270280
if (!DOCKER_ID.test(keeperId)) throw new Error('Docker did not return the created keeper ID.');
271281
docker(['start', keeperId], remaining());
272-
allocate(seeder, ['run', '--rm', '--name', seeder, ...labels,
282+
allocate(seeder, ['run', '--rm', '--name', seeder, '--label', 'io.codeboost.task-storage=seeder', ...labels,
273283
'--read-only', '--user', '0:0', '--network=none', '--cap-drop=ALL', '--cap-add=CHOWN',
274284
'--cap-add=DAC_OVERRIDE', '--cap-add=FOWNER', '--security-opt=no-new-privileges', '--security-opt=seccomp=builtin', '--runtime=runc', '--pids-limit=32',
275285
'--memory=128m', '--cpus=.25', '--mount', `type=bind,source=${staging},target=/run/codeboost-staging,readonly`,
@@ -285,14 +295,112 @@ export function prepareTaskFilesystems(clone: TaskClone, limits: TaskStorageLimi
285295
return filesystems;
286296
} catch (error) {
287297
try { cleanup(mine([seeder, keeper]), mine([metadataVolume, workVolume]), owner, unsettled); }
298+
// Still live: this process owns whatever cleanup could not remove, and reports it through the error.
288299
catch (cleanupError) { throw new AggregateError([error, cleanupError], 'Task allocation failed and cleanup did not settle.'); }
300+
liveAllocations.delete(allocationId);
289301
releaseAllocationId(allocationId);
290302
throw error;
291303
}
292304
}
293305

294-
export function removeTaskFilesystems(filesystems: TaskFilesystems): void {
295-
assertTaskFilesystems(filesystems);
296-
cleanup([filesystems.keeper], [filesystems.metadataVolume, filesystems.workVolume], taskFilesystemOwner(filesystems));
297-
allocations.delete(filesystems);
306+
/**
307+
* Task storage that `recoverLeftovers` found after a restart, when the `TaskFilesystems` value that allocated it is
308+
* gone. A crash during allocation can leave any subset of the parts, so each is optional. D issues a handle only after
309+
* checking that every part carries this runner, attempt and allocation, and only a handle D issued is accepted.
310+
*/
311+
export interface RecoveredTaskStorage {
312+
readonly runnerOwner: string;
313+
readonly attemptId: string;
314+
readonly allocationId: string;
315+
readonly workVolume?: string;
316+
readonly metadataVolume?: string;
317+
readonly keeper?: string;
318+
}
319+
export type TaskStorageParts = Pick<RecoveredTaskStorage, 'workVolume' | 'metadataVolume' | 'keeper'>;
320+
// Each issued handle's owner, and the keeper's ID from the inspect that checked it: cleanup removes that keeper by ID,
321+
// so a same-named replacement created later is never touched.
322+
const recoveredStorage = new WeakMap<RecoveredTaskStorage, { owner: ResourceOwner; keeperId?: string }>();
323+
/** The daemon answered, and the storage is not what D creates for this owner; unlike a failed inspect, retrying won't help. */
324+
export class RecoveredStorageRejected extends Error {}
325+
const STORAGE_PARTS = Object.freeze([
326+
// Exactly the names `prepareTaskFilesystems` generates: a fixed prefix and a UUID v4.
327+
['workVolume', 'volume', 'work', new RegExp(`^codeboost-work-${UUID_V4}$`)],
328+
['metadataVolume', 'volume', 'metadata', new RegExp(`^codeboost-metadata-${UUID_V4}$`)],
329+
['keeper', 'container', 'keeper', new RegExp(`^codeboost-keeper-${UUID_V4}$`)],
330+
] as const);
331+
332+
/**
333+
* Issue a recovery handle for task storage left by an earlier process. Each named part is inspected now and must be
334+
* the task-storage object of its kind carrying all three owner labels; otherwise nothing is issued. Storage this
335+
* process allocated and has not removed is refused, since a running agent may still mount it.
336+
*/
337+
export async function adoptRecoveredTaskStorage(owner: ResourceOwner, parts: TaskStorageParts,
338+
timeoutMs = 30_000, keeperId?: string): Promise<RecoveredTaskStorage> {
339+
try { owner = assertResourceOwner(owner); }
340+
catch (error) { throw new RecoveredStorageRejected((error as Error).message); }
341+
if (liveAllocations.has(owner.allocationId))
342+
throw new Error('Task storage is still live in this process; only leftovers of an earlier process are recovered.');
343+
if (keeperId !== undefined && (!DOCKER_ID.test(keeperId) || parts.keeper === undefined))
344+
throw new RecoveredStorageRejected('A keeper ID needs the keeper it names, as a full ID.');
345+
const remaining = createDeadline(timeoutMs), found: Record<string, string> = {};
346+
// Only an ID the keeper inspection below confirms is kept for cleanup.
347+
let confirmedKeeperId: string | undefined;
348+
for (const [field, object, kind, pattern] of STORAGE_PARTS) {
349+
const name = parts[field];
350+
if (name === undefined) continue;
351+
if (typeof name !== 'string' || !pattern.test(name))
352+
throw new RecoveredStorageRejected(`Recovered task ${kind} has an unexpected name.`);
353+
// A keeper the caller already identified is looked up by that ID, never by its reusable name: a keeper removed and
354+
// recreated under the same name since then is a different object, and is refused.
355+
const target = object === 'container' && keeperId ? keeperId : name;
356+
const inspect = await runDocker([object, 'inspect', target], { timeoutMs: remaining() });
357+
if (inspect.status !== 0) {
358+
if (absent(inspect as unknown as ReturnType<typeof spawnSync>))
359+
throw new RecoveredStorageRejected(`Recovered task ${kind} is gone.`);
360+
throw new Error(`Recovered task ${kind} could not be inspected.`);
361+
}
362+
const inspected = JSON.parse(inspect.stdout || '[]')[0] as { Id?: string; Name?: string;
363+
Labels?: Record<string, string>; Config?: { Labels?: Record<string, string> } } | undefined;
364+
if (object === 'container' && inspected?.Name?.replace(/^\//, '') !== name)
365+
throw new RecoveredStorageRejected(`Recovered task ${kind} no longer has its name.`);
366+
const labels = inspected?.Labels ?? inspected?.Config?.Labels;
367+
if (labels?.['io.codeboost.task-storage'] !== kind || !hasOwnerLabels(labels, owner))
368+
throw new RecoveredStorageRejected(`Recovered task ${kind} does not carry this owner's task-storage labels.`);
369+
if (object === 'container') {
370+
if (!inspected?.Id || !DOCKER_ID.test(inspected.Id) || (keeperId && inspected.Id !== keeperId))
371+
throw new RecoveredStorageRejected(`Recovered task ${kind} has no full ID, or changed.`);
372+
confirmedKeeperId = inspected.Id;
373+
}
374+
found[field] = name;
375+
}
376+
if (!Object.keys(found).length) throw new RecoveredStorageRejected('Recovered task storage names no parts.');
377+
const handle: RecoveredTaskStorage = Object.freeze({ runnerOwner: owner.runnerOwner, attemptId: owner.attemptId,
378+
allocationId: owner.allocationId, ...found });
379+
recoveredStorage.set(handle, { owner, keeperId: confirmedKeeperId });
380+
return handle;
381+
}
382+
383+
/** Whether a value is a recovery handle D issued and has not yet released. */
384+
export const isRecoveredTaskStorage = (value: unknown): value is RecoveredTaskStorage =>
385+
typeof value === 'object' && value !== null && recoveredStorage.has(value as RecoveredTaskStorage);
386+
387+
/**
388+
* Remove task storage: the value `prepareTaskFilesystems` returned, or a recovery handle from `recoverLeftovers`.
389+
* Every part must still carry its owner labels; the handle stays valid until removal is confirmed.
390+
*/
391+
export function removeTaskFilesystems(filesystems: TaskFilesystems | RecoveredTaskStorage): void {
392+
const recovered = recoveredStorage.get(filesystems as RecoveredTaskStorage);
393+
if (recovered) {
394+
const handle = filesystems as RecoveredTaskStorage;
395+
cleanup(recovered.keeperId ? [recovered.keeperId] : [],
396+
[handle.metadataVolume, handle.workVolume].filter((name): name is string => name !== undefined), recovered.owner);
397+
recoveredStorage.delete(handle);
398+
return;
399+
}
400+
const allocated = filesystems as TaskFilesystems;
401+
assertTaskFilesystems(allocated);
402+
const owner = taskFilesystemOwner(allocated);
403+
cleanup([allocated.keeper], [allocated.metadataVolume, allocated.workVolume], owner);
404+
allocations.delete(allocated);
405+
liveAllocations.delete(owner.allocationId);
298406
}

‎agents/labels.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,15 @@ export interface ResourceOwner {
1717
}
1818

1919
const RUNNER_OWNER = /^[0-9a-f]{32}$/;
20-
const ALLOCATION_ID = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/;
20+
/** A lowercase UUID v4, as `crypto.randomUUID()` writes it; unanchored, for building patterns. */
21+
export const UUID_V4 = '[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}';
22+
const ALLOCATION_ID = new RegExp(`^${UUID_V4}$`);
2123
// Attempt IDs are UUIDs in the runner; any short printable value is accepted so other callers keep their own IDs.
2224
const ATTEMPT_ID = /^[A-Za-z0-9][A-Za-z0-9._:-]{0,199}$/;
2325

2426
export const isRunnerOwner = (value: unknown): value is string => typeof value === 'string' && RUNNER_OWNER.test(value);
27+
/** Whether a value is an allocation ID as D writes it: a lowercase UUID v4. */
28+
export const isAllocationId = (value: unknown): value is string => typeof value === 'string' && ALLOCATION_ID.test(value);
2529
/** Whether an attempt ID can be written as an ownership label. */
2630
export const isLabelAttemptId = (value: unknown): value is string => typeof value === 'string' && ATTEMPT_ID.test(value);
2731

@@ -30,7 +34,7 @@ export function assertResourceOwner(owner: ResourceOwner): ResourceOwner {
3034
if (!owner || !isRunnerOwner(owner.runnerOwner)) throw new Error('runnerOwner must be 32 lowercase hex characters.');
3135
if (!isLabelAttemptId(owner.attemptId))
3236
throw new Error('attemptId cannot be written as an ownership label.');
33-
if (typeof owner.allocationId !== 'string' || !ALLOCATION_ID.test(owner.allocationId))
37+
if (!isAllocationId(owner.allocationId))
3438
throw new Error('allocationId must be a lowercase UUID v4 chosen by the caller.');
3539
return Object.freeze({ runnerOwner: owner.runnerOwner, attemptId: owner.attemptId, allocationId: owner.allocationId });
3640
}

0 commit comments

Comments
 (0)