fix(agents): reclaim the container a failed creation leaves behind (trinity-enterprise#313)

[dolho]

Fixes abilityai/trinity-enterprise#313

Fixes the leak reported in abilityai/trinity-enterprise#313. The issue is filed on the enterprise tracker but every affected file is OSS-core (crud.py, cleanup_service.py, agent_runtime_state.py), so the fix lands here.

Problem

_rollback_failed_creation rolled back DB/quota handles only, and its docstring justified the omission: the container and volumes were "left for the cleanup watchdog". No such watchdog exists for a non-ephemeral agent — the only Docker-as-truth container sweep, cleanup_service._sweep_ephemeral_agents, is gated on the trinity.ephemeral label.

So a failure after containers.run left a running container with no agent_ownership row, and two guards deadlocked:

• nothing removed the container, and
• because the container kept its workspace volume mounted, _sweep_orphan_agent_volumes could never advance its ORPHAN_VOLUME_UNATTACHED_STRIKES counter (bug: Docker volumes are never deleted — volume_remove has zero callers; agent purge leaks workspace/public/shared volumes forever #1581) — so the volume was unreclaimable too

Observed live: 13+ hours, ~66 MB, an SSH port and a 2g memory limit held by a phantom that still rendered in the fleet listing (Docker-as-truth, Invariant #11). Recovery took a manual docker rm -f, docker volume rm, and a Redis DEL.

Fix

The except-path now awaits _reclaim_failed_creation_container: remove the container, then clear the name-keyed Redis keyspace (#1560 — otherwise the next agent reusing that name inherits stale breaker verdicts and gets fast-failed as unhealthy without ever being contacted). Removing the container is also what unblocks the existing volume sweep, so AC #2 is met by the guard that already exists rather than a second destructive path.

The issue's option (a) doesn't cover the failure that reported it

Option (a) argues "the orchestrator holds the handle, so ownership is unambiguous". Not for a 60s Docker read timeout — the daemon created the container and the client never received the handle. So there are two arrival shapes:

Shape	Handling
Handle in hand (a later step raised)	ownership unambiguous — remove by handle
No handle (failure inside containers.run)	re-derive by name, behind three fail-closed gates

Reclaiming by name is a different security problem: on a shared Docker daemon (git worktrees, two stacks on one host) a name resolves to another install's live agent. The gates:

1. not a 409 name conflict — a 409 means the daemon created nothing, so whatever holds the name is definitionally not ours
2. no agent_ownership row
3. trinity.created at or after a floor stamped before the docker block

Anything unprovable refuses. A false negative costs one manual docker rm -f; a false positive deletes a running agent.

The ownership gate applies to both shapes — deliberately

_register_agent writes the row before the last creation step, so a failure in _materialize_agent_files arrives holding the handle of a container the DB already considers a created agent. Removing it would turn a half-created-but-present agent into a row with no container, still holding its name — strictly worse than the leak. (Caught in review of my own first cut, which removed it.)

Option (b) deliberately not implemented

Extending the Docker-as-truth sweep to any trinity.platform=agent container with no ownership row is unsafe on a shared daemon — exactly the multi-stack setup the issue's own "Related" section describes. A sibling stack's live agents have no rows in this install's DB, so that sweep would delete another install's running agents.

It needs a per-install container label first — which by construction only helps containers created after it ships, i.e. it would not heal the existing leaks that motivate (b). Worth its own issue with that prerequisite stated, rather than a destructive sweep shipped inside this fix.

Two defects the full suite caught that the targeted tests could not

Worth naming, since both were invisible to the 20 focused tests:

1. isinstance(exc, docker.errors.APIError) raises TypeError wherever a sibling test stubs the docker module — propagating out of a function whose whole contract is never raises and replacing the original creation error. A clear "failed to persist per-agent GitHub PAT" became an unrelated TypeError. The 409 check is duck-typed on .response.status_code now.
2. bug(lifecycle): stale transport circuit-breaker Redis key survives agent delete/recreate — a fresh healthy agent fast-fails as "unhealthy" #1560's guard fired, correctly. It bans clear_agent_runtime_state from crud.py outright, because the full sweep drops the slot ZSET and would strip an in-flight async execution (refactor: fire-and-forget dispatch — a hung turn holds zero backend resource #1083) off a live container. This reclaim is the first place in that file where the container is provably gone. Rather than delete the guard I narrowed it to enforce its reason: the sweep may appear only inside the reclaim; every other path stays on clear_agent_breakers.

Verification

tests/unit/test_ent313_failed_creation_container_reclaim.py   20 passed
tests/unit/test_1484_create_agent_characterization.py         40 passed
full unit suite: 6263 passed, 1 failed
  └─ the failure is test_1069_voip_call_path_param, which fails identically on clean dev

test_1484's case 15 asserted container_mock.remove.assert_not_called() with the comment "left for the cleanup watchdog (PRESERVED)" — a characterization test pinning the defect itself. Inverted, with the reason written at the assertion.

AC coverage

• No orphan container after a post-containers.run failure — removed inline
• Volumes reclaimable — the container no longer pins them, so bug: Docker volumes are never deleted — volume_remove has zero callers; agent purge leaks workspace/public/shared volumes forever #1581's strike counter advances
• Redis keyspace cleared via agent_runtime_state
• No phantom in the Docker-as-truth listing
• A live agent mid-recreate is never reaped — the provenance floor + 409 + ownership gates
• Regression test covering "create fails after container start"

Related to trinity-enterprise#313

🤖 Generated with Claude Code

[vybe]

Validated via /validate-pr. Fixes abilityai/trinity-enterprise#313 (reference made explicit — cross-tracker, so status-in-dev needs setting by hand on ent#313; GitHub auto-close is same-repo only).

No secrets, no schema change, no new config var, no new top-level backend module. Architecture doc updated alongside.

Three things that make this the right fix rather than the obvious one:

• The issue's option (a) is correctly rejected as incomplete. "The orchestrator holds the handle" doesn't hold for a 60s Docker read timeout — the daemon created the container and the client never got the handle. Splitting into handle-in-hand vs re-derive-by-name, with the name path behind three fail-closed gates (not-a-409, no ownership row, trinity.created floor), is the shape that actually covers the reported failure.
• Option (b) deliberately not implemented, for a real reason — a Docker-as-truth sweep over trinity.platform=agent with no ownership row deletes a sibling stack's running agents on a shared daemon, and would need a per-install label that by construction can't heal the existing leaks motivating it. Deferring that with the prerequisite stated beats shipping a destructive sweep inside a leak fix.
• The ownership gate applying to the handle-in-hand shape too. _register_agent writes the row before the last step, so a _materialize_agent_files failure arrives holding the handle of a container the DB already considers created — removing it would trade a leak for a row with no container still holding its name, which is strictly worse.

Also good: #1560's guard was narrowed to enforce its reason rather than deleted, and the isinstance(exc, docker.errors.APIError) → duck-typed .response.status_code fix removes a TypeError that was replacing the original creation error — both defects the 20 focused tests couldn't see and the full suite did.

Merged dev and resolved the tests/registry.json append conflict (union of entries preserved, JSON re-parsed, touched-file set unchanged).