Skip to content

Commit 9292fe7

Browse files
mchwangclaude
andauthored
D: return start handles at once and run Docker setup inside them (#51 item 2) (#70)
* 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> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 0cf8141 commit 9292fe7

21 files changed

Lines changed: 1190 additions & 571 deletions

‎agents/adapters/claude.ts‎

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import type { InvocationHandle } from '../contract.ts';
2-
import { createContainerProfile, ProfileCreationCleanupError } from '../container/profile.ts';
3-
import { createVendorNetwork, removeVendorNetwork, VendorNetworkCreationCleanupError,
4-
type VendorNetwork } from '../network/network.ts';
52
import { createClaudeCommand, createPhasePolicy } from '../policy.ts';
6-
import { retainNetworkCleanup, retainSetupCleanup, startProfileInvocation } from './supervisor.ts';
3+
import { launchInvocation } from './supervisor.ts';
4+
import { setUpProfile } from './setup.ts';
5+
import { assertBuiltAgentImage } from '../container/image.ts';
76
import { createAdapterInvocationBudget, type AgentAdapterOptions, type AgentAdapterRequest } from './types.ts';
87

98
export function parseClaudeOutput(raw: Buffer): { text: string; providerFailed: boolean } {
@@ -19,29 +18,11 @@ export function startClaudeInvocation(request: AgentAdapterRequest,
1918
if (!oauthToken || oauthToken.includes('\0')) throw new Error('Claude OAuth token is malformed.');
2019
const policy = createPhasePolicy(request.invocation);
2120
const remaining = createAdapterInvocationBudget(request.invocation, options.timeoutMs);
22-
let network: VendorNetwork;
23-
try { network = createVendorNetwork(request.invocation, request.imageId, Math.min(60_000, remaining())); }
24-
catch (error) {
25-
if (error instanceof VendorNetworkCreationCleanupError)
26-
return retainSetupCleanup(request.invocation, budget => error.retryCleanup(budget), error.startupError, error,
27-
'network creation cleanup', () => error.resources);
28-
throw error;
29-
}
30-
try {
31-
const profile = createContainerProfile({ ...request, policy, network,
32-
command: createClaudeCommand(policy, request.prompt), claudeToken: oauthToken,
33-
timeoutMs: Math.min(60_000, remaining()) });
34-
return startProfileInvocation(profile, { ...options, secrets: { CLAUDE_CODE_OAUTH_TOKEN: oauthToken },
35-
invocationBudget: remaining,
36-
decode: (_profile, raw) => parseClaudeOutput(raw) });
37-
} catch (error) {
38-
// Profile creation already tried its cleanup, which removes this network too, and failed. Retry that one cleanup
39-
// with the window's budget; removing the network again here would start a second full deadline per retry.
40-
if (error instanceof ProfileCreationCleanupError)
41-
return retainSetupCleanup(request.invocation, budget => error.retryCleanup(budget), error.startupError, error,
42-
'profile and network cleanup', () => error.resources);
43-
try { removeVendorNetwork(network, Math.min(30_000, remaining())); }
44-
catch (cleanupError) { return retainNetworkCleanup(request.invocation, network, error, cleanupError); }
45-
throw error;
46-
}
21+
// Invalid input throws here, before anything is allocated; only Docker setup runs inside the handle.
22+
assertBuiltAgentImage(request.imageId);
23+
return launchInvocation(request.invocation, remaining, (signal, start) => setUpProfile(request, remaining, signal,
24+
network => ({ ...request, policy, network, command: createClaudeCommand(policy, request.prompt),
25+
claudeToken: oauthToken }),
26+
profile => start(profile, { ...options, secrets: { CLAUDE_CODE_OAUTH_TOKEN: oauthToken },
27+
invocationBudget: remaining, decode: (_profile, raw) => parseClaudeOutput(raw) })));
4728
}

‎agents/adapters/codex.ts‎

Lines changed: 12 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
import type { InvocationHandle } from '../contract.ts';
2-
import { createContainerProfile, ProfileCreationCleanupError, type ContainerProfile } from '../container/profile.ts';
2+
import type { ContainerProfile } from '../container/profile.ts';
33
import { agentContainerId } from '../container/run.ts';
4-
import { createVendorNetwork, removeVendorNetwork, VendorNetworkCreationCleanupError,
5-
type VendorNetwork } from '../network/network.ts';
64
import { createCodexCommand, createPhasePolicy } from '../policy.ts';
7-
import { readBoundedContainerFile, retainNetworkCleanup, retainSetupCleanup,
8-
startProfileInvocation } from './supervisor.ts';
5+
import { launchInvocation, readBoundedContainerFile } from './supervisor.ts';
6+
import { setUpProfile } from './setup.ts';
7+
import { assertBuiltAgentImage } from '../container/image.ts';
98
import { createAdapterInvocationBudget, type AgentAdapterOptions, type AgentAdapterRequest } from './types.ts';
109

1110
export const CODEX_OUTPUT_FILE = '/run/codeboost-output/final.txt';
@@ -28,30 +27,12 @@ export function startCodexInvocation(request: AgentAdapterRequest,
2827
if (!authFile || authFile.includes('\0')) throw new Error('Codex auth path is malformed.');
2928
const policy = createPhasePolicy(request.invocation);
3029
const remaining = createAdapterInvocationBudget(request.invocation, options.timeoutMs);
31-
let network: VendorNetwork;
32-
try { network = createVendorNetwork(request.invocation, request.imageId, Math.min(60_000, remaining())); }
33-
catch (error) {
34-
if (error instanceof VendorNetworkCreationCleanupError)
35-
return retainSetupCleanup(request.invocation, budget => error.retryCleanup(budget), error.startupError, error,
36-
'network creation cleanup', () => error.resources);
37-
throw error;
38-
}
39-
try {
40-
const profile = createContainerProfile({ ...request, policy, network,
41-
command: createCodexCommand(policy, request.prompt), codexAuthFile: authFile, deferredOutput: true,
42-
timeoutMs: Math.min(60_000, remaining()) });
43-
return startProfileInvocation(profile, { ...options,
44-
invocationBudget: remaining,
45-
decode: (current, _raw, maximum, timeoutMs, signal) =>
46-
readCodexOutput(codexContainer(current), maximum, timeoutMs, signal) });
47-
} catch (error) {
48-
// Profile creation already tried its cleanup, which removes this network too, and failed. Retry that one cleanup
49-
// with the window's budget; removing the network again here would start a second full deadline per retry.
50-
if (error instanceof ProfileCreationCleanupError)
51-
return retainSetupCleanup(request.invocation, budget => error.retryCleanup(budget), error.startupError, error,
52-
'profile and network cleanup', () => error.resources);
53-
try { removeVendorNetwork(network, Math.min(30_000, remaining())); }
54-
catch (cleanupError) { return retainNetworkCleanup(request.invocation, network, error, cleanupError); }
55-
throw error;
56-
}
30+
// Invalid input throws here, before anything is allocated; only Docker setup runs inside the handle.
31+
assertBuiltAgentImage(request.imageId);
32+
return launchInvocation(request.invocation, remaining, (signal, start) => setUpProfile(request, remaining, signal,
33+
network => ({ ...request, policy, network, command: createCodexCommand(policy, request.prompt),
34+
codexAuthFile: authFile, deferredOutput: true }),
35+
profile => start(profile, { ...options, invocationBudget: remaining,
36+
decode: (current, _raw, maximum, timeoutMs, decodeSignal) =>
37+
readCodexOutput(codexContainer(current), maximum, timeoutMs, decodeSignal) })));
5738
}

‎agents/adapters/setup.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import type { InvocationHandle } from '../contract.ts';
2+
import { containerProfileResources, createContainerProfile, disposeContainerProfile, ProfileCreationCleanupError,
3+
type ContainerProfile, type ProfileOptions } from '../container/profile.ts';
4+
import { createVendorNetwork, removeVendorNetwork, vendorNetworkResources,
5+
type VendorNetwork } from '../network/network.ts';
6+
import { AdapterSetupCleanupError } from './supervisor.ts';
7+
import type { AgentAdapterRequest } from './types.ts';
8+
9+
/** Cleanup is never cancelled and does not share the invocation budget, so an expired budget cannot skip it. */
10+
const CLEANUP_TIMEOUT_MS = 30_000;
11+
12+
/**
13+
* Adapter setup inside `launchInvocation`: create the vendor network, then the container profile, then hand the
14+
* profile to the supervisor. `signal` cancels whichever Docker call is in flight. A failure that removed everything
15+
* rejects with the startup error; one whose cleanup failed rejects with an error carrying `retryCleanup`, which the
16+
* launcher keeps retrying (a `VendorNetworkCreationCleanupError`, `ProfileCreationCleanupError` or
17+
* `AdapterSetupCleanupError`).
18+
*/
19+
export async function setUpProfile(request: AgentAdapterRequest, remaining: () => number, signal: AbortSignal,
20+
profileOptions: (network: VendorNetwork) => Omit<ProfileOptions, 'timeoutMs' | 'signal'>,
21+
start: (profile: ContainerProfile) => InvocationHandle): Promise<InvocationHandle> {
22+
const network = await createVendorNetwork(request.invocation, request.imageId, Math.min(60_000, remaining()), signal);
23+
let profile: ContainerProfile;
24+
try {
25+
profile = await createContainerProfile({ ...profileOptions(network), timeoutMs: Math.min(60_000, remaining()),
26+
signal });
27+
} catch (error) {
28+
// Profile creation removes the network itself once it has claimed it; otherwise the network is still ours.
29+
if (error instanceof ProfileCreationCleanupError) throw error;
30+
try { await removeVendorNetwork(network, CLEANUP_TIMEOUT_MS); }
31+
catch (cleanupError) {
32+
throw new AdapterSetupCleanupError(error, cleanupError,
33+
(budgetMs = CLEANUP_TIMEOUT_MS) => removeVendorNetwork(network, budgetMs), () => vendorNetworkResources(network));
34+
}
35+
throw error;
36+
}
37+
// The profile owns the network and staging now. If the handoff itself throws, nothing else will release them:
38+
// the launcher treats a failure without a handle as "nothing allocated", so release (or retain) them here.
39+
try { return start(profile); }
40+
catch (error) {
41+
try { await disposeContainerProfile(profile, CLEANUP_TIMEOUT_MS); }
42+
catch (cleanupError) {
43+
throw new AdapterSetupCleanupError(error, cleanupError,
44+
(budgetMs = CLEANUP_TIMEOUT_MS) => disposeContainerProfile(profile, budgetMs), () => containerProfileResources(profile));
45+
}
46+
throw error;
47+
}
48+
}

0 commit comments

Comments
 (0)