[iscekic]

Summary

Three wrapper tests failed in about 1 of 10 CI runs:

• native upload episode projection > keeps archive and diagnostic episodes independent, ... got a second stderr line.
• adopting a repository snapshot against a real repository > ... never saw session.ready.
• local worktree recovery > restores additions, ... had git init killed by SIGPIPE.

They share one cause: a file descriptor closed twice. Bun 1.3.14 closes the parent end of an extra stdio pipe (fd 3 or above) two times. The net.Socket that wraps it closes it first. When the subprocess is collected, Bun closes the same number again. By then another descriptor uses that number, for example the stdout pipe of a later tar or git. Bun 1.4.0 fixed this (oven-sh/bun#33828).

spawnGated used an fd 3 pipe as its start gate. setup-cgroup.test.ts runs it. A GC a few files later then closed a pipe in whichever test was running. In every failed CI run I checked, setup-cgroup.test.ts ran 1 to 5 files before the failing file. Bun's file order changes with each checkout, so the failing test changed too.

The fix moves the gate to the child's stdin. Bun closes the stdin pipe once.

• Owned children (the Kilo server and tools) keep that pipe open as their stdin, as before.
• Setup commands get stdin closed after start, so they read EOF, as they did with ignore. A setup spawn with any other stdin now throws, because the gate takes stdin.

The production sandbox image runs Bun 1.3.12, which has the same bug. There, too, each gated spawn could later close an unrelated descriptor. This change fixes that as well.

A new test in setup-cgroup.test.ts checks that descriptors opened after a gated setup command stay open through GC. It failed 10 of 10 runs before the fix and passes 10 of 10 after. No existing test or assertion changed.

Verification

All runs were on gcloud VMs (8 vCPU, Bun 1.3.14), with 4 to 8 loops in parallel.

• Before the fix, the CI command bun test src --path-ignore-patterns='**/control-plane/process.test.ts' failed 5 of 75 runs. Every failure was the upload episode test, 3 files after setup-cgroup.test.ts, with tar spawn error in the log. bun test src failed 1 of 20 runs, the same way. The CI command with --randomize failed 4 of 20 runs. Those failures were the snapshot adoption tests and setup-cgroup.test.ts itself.
• After the fix, the CI command passed 60 of 60 runs, the CI command with --randomize passed 20 of 20, and bun test src passed 15 of 15.
• A descriptor probe opened 64 descriptors after 5 gated spawns, then ran GC. With the old gate, GC closed 1 of them in every run, on both the placed path and the failed-placement path. With the stdin gate it closed 0. The result was the same on Bun 1.3.14 and on the sandbox image's Bun 1.3.12.
• workload-isolation.real.test.ts ran in a private-cgroup Docker container (4 GiB, Kilo 7.8.1). The owned-tool test passed. The two Kilo tests fail with 401 Unauthorized on both the base and this branch, so that failure was already there. The gated Kilo server still started and answered.
• tsgo --noEmit, oxlint, oxfmt --check, and check:duplication pass.

Visual Changes

N/A

Reviewer Notes

• Moving to Bun 1.4 would also fix this. That is a much larger change: the sandbox base image (Bun 1.3.12), the CI pin, and @types/bun would all have to move.
• The parent writes only start\n to the gate. So the gate shell cannot take any stdin data that belongs to the command.

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change since the previous review is limited to the spawnInCgroup doc comment in setup-cgroup.ts, which now explicitly documents that only a placement failure falls back to a plain spawn while any non-ignore stdio[0] throws; this resolves the prior advisory finding with no new issues.

Files Reviewed (1 file)

• services/cloud-agent-next/wrapper/src/control/setup-cgroup.ts

Notes

• Prior finding (doc comment contradicted the new stdin guard) is resolved: the comment at lines 81-84 now states "a placement failure falls back to a plain spawn" and that any other stdio[0] throws, and the author confirmed the throw is intentional because the gate now owns stdin and a silent unmanaged fallback would drop containment without a signal.
• The comment-only change matches the behavior at setup-cgroup.ts:95 (if (stdin !== 'ignore') throw new Error('A setup command takes no stdin');) and the try/catch fallback that follows.

Previous Review Summary (commit 72f3be0)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 72f3be0)

Status: 1 Issue Found | Recommendation: Merge (advisory note only)

Executive Summary

The fd-double-close fix (gate on the child's stdin instead of an extra fd 3 pipe) is correct and leak-free; the only finding is a minor contract/behavior note on the new setup-stdin guard.

Overview

Severity	Count
CRITICAL	0
WARNING	0
SUGGESTION	1

Review Notes

The change replaces the fd 3 start gate with the child's stdin. I verified the key invariants:

• GATE_SCRIPT now reads the gate line from stdin, matching stdio: 'pipe' / ['pipe', ...] at both call sites; no fd 3 is created, so Bun cannot close a reused descriptor number when collecting the subprocess.
• releaseGate switching from end('start\n') to write('start\n') is required: for owned children the gate is the child's stdin now, and the previous stdio[0] pipe was likewise never written or ended, so the child still sees an open, empty stdin. Both paths close the pipe exactly once (releaseChildStreams destroys stdin; the setup path calls gate.end() explicitly), so no double close or leak is introduced.
• The setup try/catch still ends the gate, destroys stdout/stderr, kills the gated child, and falls back to a plain spawn; the placement-failure test at setup-cgroup.test.ts:66 still exercises that path.

No memory leaks found: the removed child.gate reference is fully superseded by child.process.stdin, whose stream is already destroyed in releaseChildStreams, and the new test cleans up its sentinel descriptors in afterEach.

Findings

services/cloud-agent-next/wrapper/src/control/setup-cgroup.ts:94 — the new if (stdin !== 'ignore') throw runs before the try, so it bypasses the documented "any failure falls back to a plain spawn" contract. Not reachable by current callers (setup never passes stdinFd), but it converts a graceful degradation into a rejection if one ever does. Advisory only.

Files Reviewed (4 files)

• services/cloud-agent-next/wrapper/src/control/gated-spawn.ts
• services/cloud-agent-next/wrapper/src/control/owned-processes.ts
• services/cloud-agent-next/wrapper/src/control/setup-cgroup.ts - 1 issue
• services/cloud-agent-next/wrapper/src/control/setup-cgroup.test.ts

---

_{Reviewed by deepseek-v4.1-flash · Input: 35.1K · Output: 2.6K · Cached: 176.4K}

_{Review guidance: REVIEW.md from base branch ci/speed-20-blocking-checks}