D: bound cleanup retries so settlement always ends (#51 item 1) (#69)

* 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>

* 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>

* 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>

* 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>

* 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>

* 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>

* 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>

* 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>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: mchwang

agents/adapters/claude.ts

@@ -23,8 +23,8 @@ export function startClaudeInvocation(request: AgentAdapterRequest,
   try { network = createVendorNetwork(request.invocation, request.imageId, Math.min(60_000, remaining())); }
   catch (error) {
     if (error instanceof VendorNetworkCreationCleanupError)
-      return retainSetupCleanup(request.invocation, error.retryCleanup, error.startupError, error,
-        'network creation cleanup');
+      return retainSetupCleanup(request.invocation, budget => error.retryCleanup(budget), error.startupError, error,
+        'network creation cleanup', () => error.resources);
     throw error;
   }
   try {
@@ -35,18 +35,11 @@ export function startClaudeInvocation(request: AgentAdapterRequest,
       invocationBudget: remaining,
       decode: (_profile, raw) => parseClaudeOutput(raw) });
   } catch (error) {
-    if (error instanceof ProfileCreationCleanupError) {
-      const retryCleanup = (networkTimeoutMs = 30_000) => {
-        const failures: unknown[] = [];
-        try { error.retryCleanup(); } catch (cleanupError) { failures.push(cleanupError); }
-        try { removeVendorNetwork(network, networkTimeoutMs); } catch (cleanupError) { failures.push(cleanupError); }
-        if (failures.length) throw new AggregateError(failures, 'Adapter setup cleanup did not settle.');
-      };
-      try { retryCleanup(Math.min(30_000, remaining())); }
-      catch (cleanupError) { return retainSetupCleanup(request.invocation, () => retryCleanup(),
-        error.startupError, cleanupError, 'profile and network cleanup'); }
-      throw error.startupError;
-    }
+    // Profile creation already tried its cleanup, which removes this network too, and failed. Retry that one cleanup
+    // with the window's budget; removing the network again here would start a second full deadline per retry.
+    if (error instanceof ProfileCreationCleanupError)
+      return retainSetupCleanup(request.invocation, budget => error.retryCleanup(budget), error.startupError, error,
+        'profile and network cleanup', () => error.resources);
     try { removeVendorNetwork(network, Math.min(30_000, remaining())); }
     catch (cleanupError) { return retainNetworkCleanup(request.invocation, network, error, cleanupError); }
     throw error;

agents/adapters/codex.ts

@@ -1,5 +1,6 @@
 import type { InvocationHandle } from '../contract.ts';
-import { createContainerProfile, ProfileCreationCleanupError } from '../container/profile.ts';
+import { createContainerProfile, ProfileCreationCleanupError, type ContainerProfile } from '../container/profile.ts';
+import { agentContainerId } from '../container/run.ts';
 import { createVendorNetwork, removeVendorNetwork, VendorNetworkCreationCleanupError,
   type VendorNetwork } from '../network/network.ts';
 import { createCodexCommand, createPhasePolicy } from '../policy.ts';
@@ -8,6 +9,12 @@ import { readBoundedContainerFile, retainNetworkCleanup, retainSetupCleanup,
 import { createAdapterInvocationBudget, type AgentAdapterOptions, type AgentAdapterRequest } from './types.ts';
 
 export const CODEX_OUTPUT_FILE = '/run/codeboost-output/final.txt';
+// Read the output from the container this invocation created, by ID; a same-named replacement must not answer.
+const codexContainer = (profile: ContainerProfile) => {
+  const id = agentContainerId(profile);
+  if (!id) throw new Error('The Codex container ID is unknown.');
+  return id;
+};
 
 export async function readCodexOutput(container: string, maximumBytes: number, timeoutMs = 30_000,
   signal?: AbortSignal) {
@@ -25,8 +32,8 @@ export function startCodexInvocation(request: AgentAdapterRequest,
   try { network = createVendorNetwork(request.invocation, request.imageId, Math.min(60_000, remaining())); }
   catch (error) {
     if (error instanceof VendorNetworkCreationCleanupError)
-      return retainSetupCleanup(request.invocation, error.retryCleanup, error.startupError, error,
-        'network creation cleanup');
+      return retainSetupCleanup(request.invocation, budget => error.retryCleanup(budget), error.startupError, error,
+        'network creation cleanup', () => error.resources);
     throw error;
   }
   try {
@@ -36,20 +43,13 @@ export function startCodexInvocation(request: AgentAdapterRequest,
     return startProfileInvocation(profile, { ...options,
       invocationBudget: remaining,
       decode: (current, _raw, maximum, timeoutMs, signal) =>
-        readCodexOutput(current.name, maximum, timeoutMs, signal) });
+        readCodexOutput(codexContainer(current), maximum, timeoutMs, signal) });
   } catch (error) {
-    if (error instanceof ProfileCreationCleanupError) {
-      const retryCleanup = (networkTimeoutMs = 30_000) => {
-        const failures: unknown[] = [];
-        try { error.retryCleanup(); } catch (cleanupError) { failures.push(cleanupError); }
-        try { removeVendorNetwork(network, networkTimeoutMs); } catch (cleanupError) { failures.push(cleanupError); }
-        if (failures.length) throw new AggregateError(failures, 'Adapter setup cleanup did not settle.');
-      };
-      try { retryCleanup(Math.min(30_000, remaining())); }
-      catch (cleanupError) { return retainSetupCleanup(request.invocation, () => retryCleanup(),
-        error.startupError, cleanupError, 'profile and network cleanup'); }
-      throw error.startupError;
-    }
+    // Profile creation already tried its cleanup, which removes this network too, and failed. Retry that one cleanup
+    // with the window's budget; removing the network again here would start a second full deadline per retry.
+    if (error instanceof ProfileCreationCleanupError)
+      return retainSetupCleanup(request.invocation, budget => error.retryCleanup(budget), error.startupError, error,
+        'profile and network cleanup', () => error.resources);
     try { removeVendorNetwork(network, Math.min(30_000, remaining())); }
     catch (cleanupError) { return retainNetworkCleanup(request.invocation, network, error, cleanupError); }
     throw error;