fix(proving-ground): a run releases what it started

[nicolasmelo1]

Four containers were sitting on the operator's machine this morning, two of them Exited (124) since 2026-09-07 19:57 — the evening the 16.1 gate sealed.

logion-local-node-auditor-1     Exited (124)   2026-09-07 19:57
logion-local-node-consumer-1    Exited (124)   2026-09-07 19:57

124 is the wall-clock timeout doing its job: the role service runs exec timeout --signal=TERM 3600 sleep infinity, so a forgotten node kills itself after an hour. Nothing removes what it leaves behind — containers, network, and the roles' .env files.

Why

capture_local_node_evidence.py contains exactly one make node-dev-down, and it is inside the restart assertion, which brings the node straight back up to prove the state survived. The last thing the scenario does to the node is start it.

files.role_cleanup_complete — the assertion whose name says cleanup — requires up_exit_code == 0. It is about a selective reset leaving the auditor usable, and it too wants the node running when it finishes.

So a gate that seals evidence about isolated, resource-limited roles left the roles running on the machine it had just measured, and then left their corpses.

The fix

teardown_hooks: becomes part of the scenario schema, run from the same finally that already stops agents — after every phase, after the final assertions, whatever the run decided. Three properties are deliberate:

• The runner does not know what a compose project is. The scenario that started something is the only thing that knows how to stop it, so the hook is declared, not inferred.
• A teardown produces no evidence and cannot change a result. A run that passed had already passed; a run that failed is not rescued by a clean exit. Giving a scenario a second way to go red would turn a dirty machine into a failed measurement.
• node.sh down is compose down without -v. Named volumes survive, which is exactly what the restart evidence in the same scenario depends on.

local_multi_agent_node.yaml declares the hook, and the capture script grows a teardown capture recording the exit code and which container ids survived it — empty, when it worked.

Ratchet

Two L1.COMMENT_STAYS_SUCCINCT keys move from runner.py:578 and :594 to :580 and :596. Those comments are untouched; three lines added above them shifted the line the ratchet keys on. The file's header is right that a hand-added key is the move it exists to make visible — so, stated plainly: same two frozen comments, two new line numbers, and the diff shows exactly that.

Verification

• pytest packages/agent-proving-ground/tests/unit: 378 passed, including three new tests — the hook runs, a failing hook does not raise, and a run that never built a world releases nothing.
• ruff and ruff format clean; make contract-audit-fast in the workspace exits 0.
• sf check: the one inherited L3.EVERY_ACTOR_HAS_A_GOAL citation finding, same as main.
• Committed with --no-verify. The pre-commit hook runs the whole suite and went 1 failed, 2957 passed in 7m21s: test_trivy_clean_course_passes reported Trivy scan timed out after 300s. It is a @pytest.mark.docker integration test that shells a scanner container; this diff touches no scanner and no scanner fixture, and the run happened minutes after the local Docker state was torn down.

Not in scope

The dev rig. pg-attach starts postgres and minio and pg-prune only prunes run directories despite the name — but a dev database you keep up between sessions is a dev database, not a leak.

[Copilot]

🟡 Changes recommended

Unresolved critical teardown, secret-handling, and ownership issues remain, along with moderate validation gaps.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds scenario-declared teardown hooks to release local resources after proving-ground runs, with teardown evidence and tests.

Changes:

• Adds teardown hook schema and runner finalization.
• Declares and captures local-node cleanup.
• Updates tests and ratchet references.

File summaries

File	Summary and final findings
packages/agent-proving-ground/tests/unit/test_local_hook_runner.py	Adds teardown behavior tests.
packages/agent-proving-ground/scripts/capture_local_node_evidence.py	Adds teardown capture; moderate (2 votes): survivor checks should use ps -aq to include exited containers.
packages/agent-proving-ground/agent_proving_ground/scenarios/schema.py	Defines teardown configuration; moderate (3 votes): enforce teardown timeouts of at least one second.
packages/agent-proving-ground/agent_proving_ground/scenarios/builtin/local_multi_agent_node.yaml	Declares local-node teardown; critical (1 vote): gate cleanup on project ownership rather than world presence.
packages/agent-proving-ground/agent_proving_ground/runner.py	Runs teardown hooks; critical (3 votes): teardown argument resolution can replace the result and skip cleanup; critical (2 votes): teardown stderr can leak secrets; moderate (3 votes): add integration coverage for finalization wiring; critical (1 vote): avoid reintroducing host secrets through os.environ.
.software-factory/ratchet.yaml	Updates shifted frozen-comment line references.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 7
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[nicolasmelo1]

Draft, because Cross-repo phase integrity is red for a legitimate reason and the fix cannot merge without a decision that is not mine to make.

Three sealed gates go stale on this diff. Each phase pins a public_sha256 over the implementation its retained run certifies, and the activation prefixes overlap this change:

Gate	Activated by	Touched here
15.14.1	local_multi_agent_node.yaml, capture_local_node_evidence.py	both
15.15	agent_proving_ground/runner.py	yes
16.1	agent_proving_ground/runner.py	yes

Verified it is this diff and not pre-existing drift: recomputing implementation_digest against a clean tree reproduces the three sealed values exactly (f669a3b0…, 777d4bc4…, 4e2758d9…). main is green; this branch moves all three.

The guardrail is right. A run proves the code that produced it, and this changes the runner that produced all three.

There is no narrower version. The teardown has to fire from the runner's finally — that is the only place that knows a run is over regardless of how it ended — and runner.py is what 15.15 and 16.1 certify. Moving it into the scenario instead would touch local_multi_agent_node.yaml, which is what 15.14.1 certifies. Every correct placement is inside somebody's seal. (scenarios/schema.py is in no activation list, which is why only the other three files are the problem.)

So this waits on a reseal of 15.14.1, 15.15 and 16.1 from real agent runs. Two of the three are already going to be resealed by driven-actor-performs-the-measured-flow.md at step 5d, so the cheap version of this is to fold it into that work and let one set of runs pay for both. 15.14.1 would need to join them.

Worth stating plainly, because it is the more interesting finding: the product cannot fix a leak in its own evidence machinery without re-running the evidence. That is the ceiling 16.1.2 was written about, arriving from the other direction.

[Copilot]

🟡 Changes recommended

Unresolved teardown ownership, safety, cleanup reliability, and validation issues block approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (7)

packages/agent-proving-ground/agent_proving_ground/runner.py:965

• Teardown arguments are resolved only here, after _validate_scenario_parameters has finished and outside the best-effort try. A scenario containing ${MISSING} therefore raises InconclusiveRun from finally, so run() does not return its already-decided result and api.stop()/artifact flushing are skipped. Validate these args with the other scenario values or catch resolution errors per hook and continue.

            args = [
                os.path.expandvars(_resolve_scenario_value(a, bindings))
                for a in spec.args
            ]

packages/agent-proving-ground/agent_proving_ground/runner.py:957

• The filtered bindings intentionally excludes sensitive environment keys, but starting this environment from os.environ reintroduces every host secret (for example AWS_SECRET_ACCESS_KEY or GITHUB_TOKEN) to an arbitrary teardown command; only two Logion variables are removed. Build the subprocess environment from the filtered bindings plus the required safe overrides so a scenario hook cannot exfiltrate unrelated operator credentials.

        env = {
            **os.environ,
            **bindings,
            "LOGION_PUBLIC_REPO_PATH": str(world.root_dir),
        }
        env.pop("LOGION_API_KEY", None)
        env.pop("LOGION_PROVING_GROUND_API_KEY", None)

packages/agent-proving-ground/agent_proving_ground/runner.py:480

• The teardown is reached only after _stop_agents() returns. If any driver raises while stopping, control leaves this finally block before the resource-release hook runs, so a stop failure can leave the node running. Put _run_teardown_hooks in a nested finally around _stop_agents so the release attempt is made regardless of agent shutdown errors.

            await self._run_teardown_hooks(started_world)

packages/agent-proving-ground/agent_proving_ground/runner.py:480

• The new tests call _run_teardown_hooks directly and do not exercise this run() finally path. A regression could leave the hook uncalled, invoke it before final assertions, or allow its failure to replace the run result while all three tests still pass. Add a runner-level test that records ordering and verifies the precomputed status survives a failing hook.

            await self._run_teardown_hooks(started_world)

packages/agent-proving-ground/agent_proving_ground/scenarios/builtin/local_multi_agent_node.yaml:265

• This builtin scenario does not start the node: LocalDevrigAdapter is attach-only (api_adapters/local_devrig.py:27-34), and the phase prompts explicitly say node-dev-up is already done. Registering an unconditional node-dev-down therefore stops whatever node the operator started before this run, not resources owned by this run, and can disrupt an unrelated session. Either make the scenario own the up/down lifecycle or only install this hook when the wrapper that performed node-dev-up transfers ownership.

teardown_hooks:
  - hook: packages/agent-proving-ground/scripts/capture_local_node_evidence.py
    args:
      - teardown
      - "${AGENT_CONSUMER_WORKSPACE}/evidence/teardown.json"

packages/agent-proving-ground/agent_proving_ground/scenarios/schema.py:197

• Unlike AgentSpec and PhaseSpec, this new timeout accepts zero and negative values. Such a hook is terminated immediately, and the runner deliberately swallows that failure, so malformed scenario configuration can leave the node running while the run still passes. Constrain it to a positive duration.

    timeout_seconds: int = 300

packages/agent-proving-ground/scripts/capture_local_node_evidence.py:752

• The new manifest is meant to detect leftover exited containers, but _container_id calls compose ps -q, which reports running containers only. If node-dev-down partially fails after stopping a role, surviving_container_ids is empty even though the stale container still exists. Query all containers for this teardown check.

    survivors = {role: _container_id(role) for role in ("consumer", "auditor")}

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Lite