[mchwang]

Closes #189

What changes

Docker test objects without this run's runner label (io.codeboost.runner), or with another runner's, are now removed however their test ends: pass, fail, timeout, a create that lands late, or a client killed between create and start.

• test/fixtures/docker-sweep.ts
  • removeLabelledObjects(label, value, when, docker = REAL_DOCKER) removes the containers, networks and volumes with label=value, and first every container that holds one of those networks or volumes, labelled or not.
    • Holders of a network are confirmed by network name, since Docker's network filter also matches the start of another network's ID.
    • Everything is removed by full ID.
    • It reports each failed call, naming the whole command, and each listed line it cannot read or that is not the shape it expects (a network with its full ID, a container ID, a volume name, a holder's full ID and its networks), with its stage. Such a line is skipped, so none is passed on as an option. It says nothing of a holder or network gone since the listing.
    • It reads the stdout of a call that exited by itself whatever its status (an inspect naming a gone container prints the others and exits 1), but not of a killed call, whose output may be cut short. Main read no output of a failed call.
    • It never throws. Eleven calls of 30 s each give a worst case of 5.5 minutes. Main's removeRunnerObjects made six calls (3 minutes), with no holder pass, ran the docker and environment on PATH at the time of the call, and named only a call's first two arguments; the hook budgets below rise for that.
  • removeRunnerObjects(owner, when?) is that sweep for one runner owner. The five Docker test files that called it on main still call it in afterAll; agent-planning and agent-question keep their own sweeps.
  • removeByName(kind, name, when, docker?) removes only an object of exactly that name, by its full ID, since Docker reads a missing name as an ID prefix. It stays quiet on Docker 27.4's own missing answers, including a network gone between its inspect and its removal. It reports any other failure, naming the exact command, and any answer it cannot read, including an ID that is not a full ID. It refuses a name that could read as an option. Every value either function passes on as an argument has a checked shape, so none can read as an option.
  • REAL_DOCKER is the docker found when the module loads, resolved to an absolute path. It is the default for both removals, so a shim left on PATH by a timed-out test is not called. requireRealDocker() refuses a shim when none was found.
  • Every call there runs with DOCKER_HOST and DOCKER_CONTEXT as they were at load. agent-supervisor points DOCKER_HOST at a missing socket until its handle settles.
  • objectExists(kind, ref) is for the tests' checks that an object is gone. It refuses a ref that could read as an option. Only Docker's own missing answer counts as gone, and any other failure (a killed call, a daemon that cannot be reached) throws, naming the command. Every gone check in a test this branch changes now requires Docker's own missing answer (objectExists, a helper's error matched against that answer, or an empty listing by allocation); on main some counted any failed inspect as gone. Checks in tests it does not change keep main's form.
  • CLIENT_LIMIT (60 s, then SIGKILL) is for the tests' synchronous Docker calls, which no Vitest timer can stop.
  • TEST_RUN_LABEL and TEST_FILE_LABEL are labels only tests write.
• test/agent-container.test.ts
  • withoutRunnerLabel refuses a prefix that is not a create, since it puts its labels after create. It labels what it makes with a per-call test-run ID and this file's run ID. sweepWhenFinished removes the test-run ID in onTestFinished (6-minute budget), and afterAll sweeps the file's run ID.
  • Each object made under another runner's owner, under a random one, or with no labels is covered one of two ways.
    • Swept by allocation or owner: async tests register sweepWhenFinished at their start, and synchronous ones sweep in their finally.
    • Tracked by name before it is made. This covers named containers, networks and volumes, and the containers that mount a label-less volume.
  • afterAll removes the tracked names with removeByName, containers first. It then sweeps by file label and by owner, and reports every step's failure at the end. Its limit rises from 2 to 13 minutes, for the two sweeps.
  • The racer, foreign and unlabelled shims now act only on their own test's calls (swap already did). Every shim in the five files takes the real docker from requireRealDocker() rather than looking it up at test time.
  • Creates that may land late are made inside their try, so the finally removes what they made: the rogue volume (tracked by name before it is made), its container and the foreign keeper.
  • The seccomp startup probe's container is now named and tracked, and its call takes CLIENT_LIMIT. The rogue-volume test's finally removes the container before the volume it mounts.
  • "never removes a task keeper replaced by another runner" sweeps its allocation in its finally, so its budget rises from 60 s to 7 minutes.
  • "refuses task storage that belongs to another runner" releases its storage in its own finally, and a release failure after a pass fails the test there. On main it was pushed to taskFilesystems, so such a failure failed afterAll.
  • "rejects task volumes without their runner label" also checks the labels the wrapper puts on.
  • "never removes a network found by name after a killed create when its runner label differs" lists the network with CLIENT_LIMIT (SIGKILL; main's helper used SIGTERM), and a failed listing names its command. Its finally removal is replaced by an allocation sweep in onTestFinished.
  • "keeps a failed allocation whose cleanup did not settle live" checks, after a pass and after its sweep, that nothing of the allocation is left (networks included), in a 4-minute onTestFinished hook. Main's finally listed only containers and volumes.
  • A holder-sweep test checks against the daemon the Docker behaviours the sweep relies on, the sweep itself, and removeByName. Its budget is 8 minutes: the sweep's worst case and ordinary calls.
  • An it.fails test and its follow-up check that a keeper left by a test that stopped early is gone.
• test/agent-network.test.ts
  • The shims that make foreign objects record each name, one per line, before they make the object, in a records directory outside the shims. afterAll reads it again.
    • The stand-in shim acts only on an exec into a codeboost-proxy-* container. On main it acted on every exec.
    • The three tests that read a record fail with "the shim recorded no foreign …" (or a name check) when it is empty. On main a missing record failed them with ENOENT.
    • "removes a network that lands … after its create client was killed" and "removes a proxy that was created even though its start failed" check the recorded name or ID before their gone check, so a shim that recorded something else is reported as itself; a leaked object is then left to afterAll's runner sweep. The proxy's removal takes CLIENT_LIMIT (none on main).
    • Their check that the foreign object survived moved into the finally and uses objectExists: only Docker's missing answer counts as gone, and any other failure is reported, naming the command, after the body's own checks.
  • Unnamed helper run --rm containers carry this file's run ID, and afterAll sweeps it. A helper whose client is killed between create and start is never removed by --rm, and is no holder once its network is gone.
  • The peer, the stand-in proxy and the partial-removal blocker are tracked by name, and the blocker is made inside its try. The afterAll hook's limit rises from 4 to 13 minutes.
  • The attached helper runs (curl, DNS, peer) are killed with SIGKILL at their limit: an attached docker run passes SIGTERM on to the container rather than exiting. A killed helper's container then runs on to its own short timer (15 s at most).
• test/agent-supervisor.test.ts: the two foreign-name creates and the ack-script run are made inside their try, with CLIENT_LIMIT. The two foreign-name containers are removed by name when their test finishes (90 s hooks, for two calls of 30 s) and tracked by name for afterAll, as is the ack-script container. The afterAll limit rises from 3 to 7 minutes.
• test/agent-gate.test.ts, test/runner-workspace.test.ts:
  • Their afterAll hooks, which already ran removeRunnerObjects, get budgets for the sweep's worst case: 4 to 6 minutes and 4 to 7 minutes.
  • runner-workspace's Sweep leftover path identity probe containers in startup recovery #181 test now sweeps its two owners in onTestFinished (6 minutes each), not in a finally, so the sweeps run on a timeout too.
  • runner-workspace's docker helper takes CLIENT_LIMIT.
  • runner-workspace's check that the probe's volume is gone now wants Docker's "no such volume" answer; on main any failed inspect passed it.
• Failures kept from main: a cleanup that could fail a test on main still fails it, with Docker's own error, after the test's own checks pass. This uses a passed flag, or a check registered to run after the sweep. A failure in the test body stays the one reported.
• test/docker-sweep.test.ts (15 tests, fake dockers that say beside them what they answer as Docker 27.4, and what they leave out or invent):
  • unreadable output;
  • a killed call;
  • removal by name;
  • the loaded daemon address, for every kind of call;
  • a holder, or a network, gone since the listing;
  • a holder line that is not an ID and networks;
  • listed lines that are not a container ID or a volume name, and a holder search line that is not a container ID;
  • every call's time limit and SIGKILL;
  • a docker that cannot start;
  • objectExists: gone only on Docker's missing answer, a throw naming the command on any other failure, at the loaded daemon;
  • the load-time docker as the default.
• Workflows:
  • The Live probes job limit rises from 75 to 100 minutes. Its comment now counts all five probes, agent-planning's from Check the planning agent at startup and before planning requests (#164) #193 included (44 minutes), and the teardown limits, now 26 minutes: 90 minutes plus setup. test/vitest-config.test.ts pins the new value.
  • The Agent isolation comment says its 45 minutes cover ordinary runs, not every hook at its limit.

Accepted limits

• In agent-container's recovery test, a cleanup AggregateError replaces the body's failure when both fail. This was already so on main.
• Test budgets cover the sweep's worst case plus ordinary Docker call times, not every call at its own timeout.
• Objects in synchronous test bodies are removed by their finally, which always runs.
• An object that lands after its finish-time sweep has run, from a timed-out body or a killed process's Docker client, is not covered.
• withoutRunnerLabel's wrapper, if a timed-out test leaves it on PATH, also unlabels a later test's matching create, which then fails. What it makes carries the file's run ID, and afterAll removes it.
• Shims on main that make no foreign or label-less objects still act on every caller's calls. They leak nothing.
• Docker calls this branch did not add, move or rewrite keep main's time limits.

Review record

All rounds were independent-agent reviews: a fresh general-purpose subagent with no author context, reading the full origin/main...HEAD diff each time, never only the fixes. Each round's fixes are one commit whose message starts "Review round N" and lists what changed.

Rounds	Findings	What changed
1–23	majors and minors: objects left by a test that stopped early, sweeps that missed holders or matched by ID prefix, a sweep run through a shim left on PATH	test-run and file labels, finish-time sweeps, holder pass by network name, removal by full ID, REAL_DOCKER, removeByName
24–41	minors: objects that land after the finally, records that a late client overwrites, never-started containers that are no holder once their network is gone, DOCKER_HOST left changed	tracking by name before the create, per-file records outside the shims, file label on helper runs, load-time daemon address, scoped shims
42–56	a major (an assertion read a spy after mockRestore cleared it) and minors: cleanups that lost main's failure, fakes not matching Docker 27.4, quiet answers that hid real failures	passed flags and after-sweep checks, fakes copied from the daemon, exact missing answers, workflow limits (Live probes 75→100 min)
57–74	majors (time limits lost on moved calls) and minors: unlimited synchronous calls, gone checks that passed on a killed inspect, values passed on that could read as options, untested guards	CLIENT_LIMIT, objectExists, shape checks on every value passed on, a test for each guard and each time limit
75	none: CLEAN	—

Declined, with reasons (each repeated to later rounds so they were not re-reported):

• withoutRunnerLabel's guard refusing a prefix that is not a create has no test: it is a precondition of a test-only helper inside a Docker suite file, and every caller passes a create.
• The reports name the command as docker <args> even when a stand-in path is given: that argument is only for stand-ins that pass the call through or imitate it, so the command named is the one that ran.
• Checks and Docker calls in tests this branch does not change keep main's form and limits.
• The shims on main that make no foreign object (listed under accepted limits).

Merge of main at 79deeec (#193, #196): no conflicts; the review rounds after it read the merged diff.

Clean independent-agent pass authorizing the Copilot request: round 75, base 79deeec, head f6bf790.

Validated head f6bf790 (base 79deeec): CI test and real-docker passed on it.

Copilot review on head f6bf790 (review 5602245061): approval recommended, no open issues, no inline comments.

Review-lesson audit

• Cleanup that missed an object however its test ended, and shims that acted on other tests' calls: covered by "A test's setup must leave the state the production path would" and by the claim rule (every claim of what cleanup covers was traced through timeouts and late creates).
• Fakes that did not answer as Docker 27.4 does, or did not say what they leave out or invent: covered by the existing rule on fakes.
• Checks that could not fail (a spy read after mockRestore, a guard with no test, a gone check that passed on a killed call): covered by "The Design system and plan format #1 Rule of E2E Tests" and "give it a test that fails if the claim is false".
• Time limits lost or untested, and workflow sums that no longer held: covered by "Treat every behavioural claim … as something to verify" (for example "bounded by N seconds").
• Cleanups that lost main's failure or its message: covered by the same claim rule and by the Design system and plan format #1 rule.
• PR body statements that drifted from the diff: covered by "Treat every behavioural claim, in code comments, the PR body or docs".
• Wording and layout nits: one-off; no rule.

No new rule is needed, so no AGENTS.md or CLAUDE.md change is in this branch.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

No blocking issues were identified in the cleanup logic, regression coverage, or workflow changes.

No open issues 🎉

What changed in this PR

Addresses #189 by strengthening Docker test cleanup for objects missing this run’s runner label, including failure and timeout paths.

Changes:

• Adds shared label-based and exact-name cleanup with bounded Docker calls.
• Tracks test-created resources and adds cleanup regressions.
• Adjusts teardown budgets and the live-probe job timeout.

File	Description
test/​vitest-config.test.ts	Pins the updated live-probe timeout.
test/​runner-workspace.test.ts	Adds finish-time sweeps and stricter removal checks.
test/​fixtures/​docker-sweep.ts	Adds holder cleanup, exact-name removal, and bounded calls.
test/​docker-sweep.test.ts	Tests cleanup safeguards and Docker failure handling.
test/​agent-supervisor.test.ts	Tracks foreign containers and bounds creation calls.
test/​agent-network.test.ts	Records foreign objects and labels helper containers for cleanup.
test/​agent-gate.test.ts	Increases the sweep’s teardown budget.
test/​agent-container.test.ts	Adds resource tracking, finish-time sweeps, and leak regressions.
.github/​workflows/​live-probes.yml	Raises the job timeout to 100 minutes.
.github/​workflows/​agent-isolation.yml	Clarifies existing timeout coverage in comments.

_{Review effort: Balanced}