D: settle deferred output on the container's exit, not the acknowledgement exec

[mchwang]

Fixes the intermittent real-Docker failure of container invocation supervisor > delimits READY after finite newline-free stderr (run 36470233043, commit 3d52c0d).

Diagnosis

The failing assertion was stopReason (line 541), not the stderr check. "trailing-diagnostic" appears in the error only because result.stderr is that assertion's message. The full stderr was trailing-diagnostic\n[codeboost: capture-failure: Deferred output acknowledgement failed.], so stderr capture worked.

After READY, the supervisor writes /run/codeboost-control/collected-<token> with docker exec and used to fail the invocation if that exec failed. The wrapper (probe.sh) polls every 50 ms and exits as soon as the file holds the token. Its exit tears the container down and SIGKILLs the exec if it has not finished yet. On real Docker, an exec that stays alive briefly after writing the file exits with 137. In production only fsync, fstat, close and Node shutdown remain after the write, so the race is rare.

Change

• The exec's status no longer decides the outcome. A failed acknowledgement is retried every 250 ms until one succeeds, the container exits, or a stop or the deadline ends the invocation. No retry starts after the container has closed or a stop has begun.
• The container's exit decides. Deferred output is accepted only if the container exited with the READY status, which the wrapper returns after it sees the acknowledgement. Any other status, or a signal to the attached client, gives capture-failure. The result no longer depends on the order of the exec failure and the container exit.
• ACKNOWLEDGEMENT_SCRIPT is idempotent. It keeps a complete file, and it unlinks and rewrites a partial one. Root has no capabilities in the container, so it cannot reopen its own 0444 file for writing. Symlinks are still refused.
• Diagnostic: a timeout while acknowledgements keep failing says how many attempts failed.

Known limit: a container killed from outside with a status equal to the READY status (for example 137) cannot be told apart. Its output was already collected in full by then, and the status already reports a failure. Closing this completely would need a "collected" frame from probe.sh, which is out of scope here.

Tests

• test/agent-deferred-acknowledgement.test.ts (mocked Docker, 7 tests) covers:

  • the exec failing before and after the container exit;
  • a retry that recovers;
  • a nonzero READY status;
  • a container exit that bypasses the acknowledgement (wrong status or signal), which fails closed;
  • an acknowledgement that never takes effect, which times out.

  Against the previous supervisor, 6 of the 7 fail; the two race cases fail with the exact CI message AssertionError: trailing-diagnostic. The nonzero-status case passes on both, because the old code handled it the same way.

• test/agent-supervisor.test.ts (real Docker) runs the script with --cap-drop=ALL. It checks a fresh write, a repeat, repairs of an empty and a partial file, the file's mode and owner, and that a symlink is refused. The old O_EXCL-only script fails the repeat.

Local results on 035cf59:

• tsc --noEmit is clean.
• The mocked supervisor suites pass (16).
• All 10 real-Docker deferred-output tests pass.

The local real-Docker suite also shows about 10 failures that are unrelated to this change. They come from name conflicts with stale codeboost-agent-* containers left on the local daemon, and test container names are deterministic. The exact-head CI real-docker result is the one to use.

Review rounds

Round	Reviewer	Findings	Outcome
1	independent review agent	1. Retries could never succeed after an attempt that created the file (O_EXCL).	Fixed: idempotent script plus a real-Docker test. New rule? No; covered by the existing "Treat a successful external command as the transition it actually performed" rule.
		2. An external kill with a status equal to READY is accepted.	Documented as a known limit (above); it is also stated in the code comment.
		3. Test gaps.	Real-Docker test for the script added.

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟢 Approval recommended

The lifecycle changes are bounded, fail closed, and covered by targeted race and integration tests.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes deferred-output acknowledgement races by using the container’s exit status as the authoritative outcome.

Changes:

• Adds idempotent acknowledgement creation and retries.
• Validates container exit against the READY status.
• Adds mocked race tests and real-Docker acknowledgement tests.

File	Description
agents/​adapters/​supervisor.ts	Implements acknowledgement retry and exit-based settlement.
test/​agent-deferred-acknowledgement.test.ts	Covers acknowledgement and exit interleavings.
test/​agent-supervisor.test.ts	Verifies acknowledgement-file behavior in Docker.

---

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