Repository navigation
Commit 15ee8fa
* 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>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9292fe7 commit 15ee8fa
27 files changed
Lines changed: 798 additions & 151 deletions
File tree
- agents
- adapters
- container
- network
- docs/implementation
- runner
- test
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
6 | | - | |
| 5 | + | |
7 | 6 | | |
8 | 7 | | |
9 | 8 | | |
| |||
19 | 18 | | |
20 | 19 | | |
21 | 20 | | |
22 | | - | |
| 21 | + | |
23 | 22 | | |
24 | 23 | | |
25 | 24 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
7 | | - | |
8 | | - | |
| 7 | + | |
9 | 8 | | |
10 | 9 | | |
11 | 10 | | |
| |||
28 | 27 | | |
29 | 28 | | |
30 | 29 | | |
31 | | - | |
| 30 | + | |
32 | 31 | | |
33 | 32 | | |
34 | 33 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
22 | | - | |
| 22 | + | |
| 23 | + | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
3 | 5 | | |
4 | 6 | | |
5 | 7 | | |
| |||
8 | 10 | | |
9 | 11 | | |
10 | 12 | | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
11 | 24 | | |
12 | 25 | | |
13 | 26 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
8 | | - | |
| 8 | + | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| |||
136 | 137 | | |
137 | 138 | | |
138 | 139 | | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
139 | 146 | | |
140 | 147 | | |
141 | 148 | | |
| |||
218 | 225 | | |
219 | 226 | | |
220 | 227 | | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
221 | 233 | | |
222 | 234 | | |
223 | 235 | | |
| |||
265 | 277 | | |
266 | 278 | | |
267 | 279 | | |
268 | | - | |
| 280 | + | |
269 | 281 | | |
270 | 282 | | |
271 | 283 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
| 5 | + | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
10 | 12 | | |
11 | 13 | | |
12 | 14 | | |
| |||
54 | 56 | | |
55 | 57 | | |
56 | 58 | | |
| 59 | + | |
| 60 | + | |
57 | 61 | | |
58 | 62 | | |
59 | 63 | | |
| |||
85 | 89 | | |
86 | 90 | | |
87 | 91 | | |
| 92 | + | |
88 | 93 | | |
89 | | - | |
| 94 | + | |
90 | 95 | | |
91 | 96 | | |
92 | 97 | | |
| |||
124 | 129 | | |
125 | 130 | | |
126 | 131 | | |
127 | | - | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
128 | 138 | | |
129 | 139 | | |
130 | 140 | | |
131 | | - | |
| 141 | + | |
132 | 142 | | |
133 | 143 | | |
134 | 144 | | |
| |||
201 | 211 | | |
202 | 212 | | |
203 | 213 | | |
| 214 | + | |
204 | 215 | | |
205 | 216 | | |
206 | 217 | | |
| |||
262 | 273 | | |
263 | 274 | | |
264 | 275 | | |
265 | | - | |
| 276 | + | |
| 277 | + | |
266 | 278 | | |
267 | 279 | | |
268 | 280 | | |
| |||
271 | 283 | | |
272 | 284 | | |
273 | 285 | | |
274 | | - | |
| 286 | + | |
275 | 287 | | |
276 | 288 | | |
277 | 289 | | |
| |||
286 | 298 | | |
287 | 299 | | |
288 | 300 | | |
289 | | - | |
| 301 | + | |
290 | 302 | | |
291 | 303 | | |
292 | 304 | | |
| |||
349 | 361 | | |
350 | 362 | | |
351 | 363 | | |
| 364 | + | |
| 365 | + | |
352 | 366 | | |
353 | 367 | | |
354 | 368 | | |
| |||
0 commit comments