You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Swim Lane D needs production Claude and Codex adapters that cannot outlive their ownership or return unbounded output. This change adds container invocation supervision with immutable 16 MiB stdout, 4 MiB stderr, and 20 MiB combined ceilings; finite deadlines; first-reason cancellation; whole-container TERM/KILL cleanup; and active-attempt ownership until the container and cleanup settle.
Claude output is parsed from its JSON envelope. Codex writes its final response into runner-created bounded tmpfs and uses a private deferred-output handshake so the supervisor can perform a no-follow, nonblocking, regular-file, bounded UTF-8 read before teardown. Invalid, changing, linked, malformed, traversing, or oversized output fails closed.
This PR is stacked on #44 and should be reviewed/landed after it. It implements D4 only and does not connect the adapters to the runner/store lifecycle owned by Lane F.
npx vitest run --no-file-parallelism — 18 files, 295 tests passed
CODEBOOST_RUN_AUTH_PROBES=1 npx vitest run test/agent-supervisor.test.ts with Keychain Claude OAuth and local Codex auth — 34 tests passed, including both production provider probes
Focused supervisor suite — 32 tests passed
Review record
Self-review round 1: fixed deferred Codex capture failures so they trigger immediate whole-container termination and withhold protocol acknowledgement; tightened duplicate protocol-token handling.
Automated round 2: accepted and fixed both inline findings by moving output to a dedicated validated tmpfs mount that the agent cannot replace. Also fixed both summary findings with fatal Claude UTF-8 decoding and immediate capture-failure termination on acknowledgement failure. Regressions cover mount-point replacement and acknowledgement denial.
Automated round 2 late findings: fixed the reader production ceiling, post-close cancellation, nonzero envelope validation, failed-start cleanup ownership, and anchored pathname identity. Exact-head regressions cover the first three; hostile-file plus live Codex probes exercise the pinned reader.
Additional delayed findings: fixed post-close cleanup recovery, stale recovery timers, and unbounded async decoders; also replied to the already-fixed Claude UTF-8, late acknowledgement, and BigInt link-count threads.
Automated round 3: fixed lstat pathname validation, all pre-start cleanup ownership, missing deferred output, and abort-and-await decoder settlement. Also capped configured timeouts at ten minutes to avoid Node timer clamping.
Automated round 4: fixed durable network-only cleanup recovery for both adapters and bounded detachment for an injected decoder that ignores abort.
Automated round 6: reported no inline findings but surfaced two missed summary items; fixed colliding cleanup ownership with a separate recovery registry and delimited READY after newline-free stderr.
Automated round 7: fixed pre-network deadline enforcement for both adapters; the reused-attempt network recovery finding was already fixed by the round-6 recovery registry change.
Automated round 8: confirmed the reused-attempt recovery fix and surfaced profile-staging cleanup ownership; fixed it with retryable construction errors and combined directory/network recovery ownership.
Automated round 9: no inline findings; fixed its summary-only CI serialization concern by running the dedicated Docker workflow with --no-file-parallelism.
Automated round 10: no inline findings; fixed its summary-only timestamp and cancellation concerns with monotonic deadline budgets and a bounded abort race for decoders that ignore cancellation.
Automated round 11: fixed colliding-recovery admission, carried the original monotonic budget into supervision, rechecked deadlines before decoded output publication, and kept cleanup retry timers referenced through settlement.
Automated round 12: no inline findings; fixed its summary-only network cleanup budget concern by sharing the caller deadline during failed construction and handing exhausted cleanup to the retained bounded recovery owner.
Automated round 13: fixed same-profile duplicate rejection without disposal and enforced the monotonic deadline in the close handler before accepting decoded output.
Automated round 14: no inline findings; fixed its summary-only adapter timeout concern by applying the configured timeout to the original pre-network monotonic budget.
Automated round 15: no inline findings; fixed its summary-only concerns by bounding immediate adapter cleanup with the remaining invocation budget and retaining final bounded stderr during close.
Automated round 16: no inline findings; fixed its summary-only settlement concern by keeping decoder timeout and abort-grace timers referenced until ownership is released.
Automated round 17: fixed retained profile identity during cleanup recovery and classified decoder failures after the monotonic deadline as timeout.
Automated round 18: fixed final resolved-container revalidation at the immediate docker start boundary with retained cleanup on refusal.
Automated round 19: fixed profile capability authentication before any cleanup, preventing cloned or forged profile objects from disposing an authentic invocation container.
Automated round 20: no inline findings; fixed its summary-only cancellation concern by preserving the first requested stop reason across retained profile and setup cleanup recovery. The cleanup-budget concern did not identify a failing path; exact-head tests confirm ownership remains active until cleanup settles.
Automated round 21: fixed capability authentication at the exported disposal boundary, so direct callers cannot use cloned profile fields to remove an authentic container or network.
Automated round 22: pending.
Declined findings: none.
Review-lesson audit: deadline enforcement, bounded fail-closed reads, whole-container termination, and settlement ownership are covered by existing repository rules. The path/FIFO/link/UTF-8 details are one-off defenses for this pinned output-file reader and do not add a broader rule.
Follow-up boundary
D5 will provide the full hostile-input and real-Docker boundary gate as a separate PR. Lane F remains responsible for admission, persistence, retries, and runner/store integration.
Reject malformed UTF-8 before parsing Claude output
agents/adapters/claude.ts:9
Buffer.toString('utf8') replaces malformed byte sequences with U+FFFD, so an envelope containing invalid UTF-8 can be parsed and published as a successful Claude response instead of failing closed. Decode with a fatal UTF-8 decoder before calling JSON.parse, matching the Codex adapter's behavior.
Propagate acknowledgement failures and terminate capture
agents/adapters/supervisor.ts:240
The acknowledgement docker exec ... touch is fire-and-forget, and runControl resolves even when the child exits nonzero or emits an error. If acknowledgement creation fails, the wrapper remains in its polling loop until the invocation deadline, so this capture failure is reported as a late timeout and holds the container/network unnecessarily. Make this control operation report failure and immediately preserve the first capture-failure reason while terminating the whole container.
Safely schedule timers for long invocation deadlines
agents/adapters/supervisor.ts:313
This passes an unrestricted deadline delta directly to setTimeout. captureInvocation accepts any safe future deadline and timeoutMs has no upper bound, but Node clamps delays above 2^31-1 ms to about 1 ms, so a valid long-lived invocation can be cancelled immediately instead of at its deadline. Cap and re-arm long delays (or explicitly enforce a compatible maximum) before scheduling this timer.
Large deadlines cause immediate timeout due to setTimeout clamping
agents/adapters/supervisor.ts:315
The invocation deadline is only validated as a safe integer, but Node clamps setTimeout delays above 2^31-1 ms to 1 ms. A valid invocation with a sufficiently distant deadline (or a large timeoutMs) will therefore be stopped immediately instead of at its deadline. Schedule long deadlines in bounded chunks or reject unsupported durations explicitly.
Commit pending work left in the D4 worktree:
- Add isContainerProfileAuthentic and skip disposal when container
validation fails for a profile whose identity was already removed.
- Route a close observed after the deadline through stop('timeout') so a
running decoder is aborted.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A duplicate attempt's profile never created a container, and its
deterministic container name belongs to the active invocation, so
rejection now releases only the duplicate profile's staging and network.
Previously it tried to remove the container by name, which D2 now refuses
for another invocation's container, leaving a cleanup handle that never
settled. The regression reuses one captured invocation, since an attempt
can only be captured once.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The adapter and supervisor suites use Docker and already run one file at
a time in the Agent isolation workflow.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each output event was stored as its own Buffer slice, so a container could
emit the byte ceiling as millions of one-byte writes and exhaust
supervisor memory before the byte limit applied. Output is now copied
into 64 KiB blocks, so retained objects scale with bytes rather than with
write events, and slices no longer pin their original chunks.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Record, per profile, when a killed docker create stops counting as in
flight. Later cleanup (for example the supervisor's dispose after an
early create-path cleanup failure) no longer treats absence inside that
window as proof and releases the profile; it reports "did not settle"
so ownership is retained and retried. Also round the create-path wait
to whole milliseconds, which the deadline helper requires.
- Decode final stdout and stderr with a fatal UTF-8 policy. Invalid
output becomes a capture-failure with that stream withheld, so
replacement characters cannot grow the result past its byte ceilings;
an incomplete trailing character cut at a limit is dropped.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After a killed docker create, the create path waited out the settle
window but still reported "did not settle" when no container appeared,
leaving the profile's staging and network owned. Callers such as
runContainer have no later cleanup, so those resources leaked. Absence
now counts as settled on every path once the window has passed, matching
storage and network cleanup; only inside the window is it reported as
unsettled.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Using { stream: true } leaves an incomplete trailing UTF-8 sequence buffered, so fatal decoding does not reject a malformed output that ends with a lone lead byte; the invocation can therefore be published as successful instead of failing closed. Decode without streaming (or explicitly flush the decoder) for the final captured buffer.
The final output decode used streaming mode on every stream, so output
ending in a lone lead byte was silently trimmed and published as a
success. Only output cut at a capture limit may now end in an incomplete
character; otherwise the decode flushes and fails closed as a
capture-failure.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Follow-up on Copilot's review of 6c757e1 (no inline findings), in a7a6eb9:
Streaming decoder accepts trailing incomplete UTF-8 ("previously missed", supervisor.ts:589): real. Streaming mode was used for every stream, so output ending in a lone lead byte was trimmed and published as a success. Streaming is now used only when capture stopped with output-limit, where a character can legitimately be cut at the byte ceiling. Otherwise the decode flushes, and an incomplete trailing character fails closed as capture-failure. A new truncated-utf8-stderr probe ends stderr with 0xE2: the previous code settled it as a normal run, and it now settles as capture-failure. The existing output-limit tests still pass.
Deadline-bounded cleanup (summary only): cleanup deliberately is not bounded by the invocation deadline. It must still run after a timeout so containers, staging and networks are removed, and each Docker step in it keeps its own bounded budget.
Rejections before container creation (invalid limits, an expired
deadline, a duplicate attempt) own no container, but they cleaned up with
container-level disposal. If the deterministic name was held by another
invocation or Docker inspect failed, that cleanup could never settle, and
the recovery retried it forever while the profile's staging and network
leaked. These paths now dispose only the profile, and recovery retries
that same profile-only cleanup.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem and behavior
Swim Lane D needs production Claude and Codex adapters that cannot outlive their ownership or return unbounded output. This change adds container invocation supervision with immutable 16 MiB stdout, 4 MiB stderr, and 20 MiB combined ceilings; finite deadlines; first-reason cancellation; whole-container TERM/KILL cleanup; and active-attempt ownership until the container and cleanup settle.
Claude output is parsed from its JSON envelope. Codex writes its final response into runner-created bounded tmpfs and uses a private deferred-output handshake so the supervisor can perform a no-follow, nonblocking, regular-file, bounded UTF-8 read before teardown. Invalid, changing, linked, malformed, traversing, or oversized output fails closed.
This PR is stacked on #44 and should be reviewed/landed after it. It implements D4 only and does not connect the adapters to the runner/store lifecycle owned by Lane F.
Validation
Validated head:
a371bc77cd9b809489a50eb5b28ca17c51d87315npm run typechecknpx vitest run --no-file-parallelism— 18 files, 295 tests passedCODEBOOST_RUN_AUTH_PROBES=1 npx vitest run test/agent-supervisor.test.tswith Keychain Claude OAuth and local Codex auth — 34 tests passed, including both production provider probesReview record
--no-file-parallelism.docker startboundary with retained cleanup on refusal.Follow-up boundary
D5 will provide the full hostile-input and real-Docker boundary gate as a separate PR. Lane F remains responsible for admission, persistence, retries, and runner/store integration.