fix(provider-acp): a missing agent binary exits 125 where /bin/sh is bash

[only21mil]

Problem

The Linux cgroup wrapper in wrapCommandForLinuxCgroup reserves exit 125 for "the target could not start". Where /bin/sh is bash it exits 127 for a missing agent binary, and 126 for a directory or non-executable target, which is the code the wrapper reserves for a cgroup mismatch. The missing-target assertion in AcpSessionRuntime.processTree.test.ts pins 125 and fails on such hosts with expected 127 to be 125.

The wrapper sets trap 'exit 125' 0 and then runs exec "$@". Whether the EXIT trap runs after a failed exec depends on the shell:

/bin/sh	trap 'exit 125' 0; exec /nonexistent-t3-probe exits
GNU bash 5.3	127
bash --posix	127
dash	125
busybox ash	125

CI runs on Ubuntu, whose /bin/sh is dash, so with only the existing assertion CI passes with or without a fix.

#15344 by @Moinax fixes this with one line, but it no longer applies because #17354 moved AcpSessionRuntime.ts from apps/server to packages/provider-acp.

Change

#15344's check and comment, applied at the new path; the comment is #15344's, kept verbatim. Before the exec, the wrapper checks [ -f "$1" ] && [ -x "$1" ] and exits 125 if either test fails. The caller always passes an absolute target, and the check matches the statSync/X_OK test resolveLinuxCgroupTargetCommand already does in Node. For an executable that exists, the line changes nothing on any shell.

For a missing or non-executable target, the 125 now holds whichever shell /bin/sh is. A target that passes the check and still fails execve, such as a script whose shebang interpreter is missing, behaves as before: 126 on bash, 125 on dash.

Two additions go beyond #15344:

• The guard prints t3-acp-cgroup-wrapper: <target>: not an executable file on stderr before exiting. The shell's own exec error no longer appears, and that stderr reaches the user through the AcpProcessExitedError message.
• The process-tree test runs the same missing-target wrapper under bash explicitly when bash is on PATH and expects 125. Without it, CI's dash passes the test with or without the guard.

On dash the exit code does not change. The stderr text does: exec: <target>: not found becomes the line above.

Scope and approval

Submitted under the small, focused fix exception. The wrapper gains one guard line, and the intended 125 is already pinned by a test in this repo that is red today wherever /bin/sh is bash. With the new assertion, CI also fails if the guard is removed.

This carries forward #15344 by @Moinax, credited as co-author on the commit. I'm happy to close this if #15344 is rebased onto the new path instead; the two additions can move there.

Verification

vp test run src/provider/acp/AcpSessionRuntime.processTree.test.ts in apps/server, same machine and checkout, changing /bin/sh (see below for how bash stays bash). "Before" is the old wrapper with the new test.

	bash as /bin/sh	dash as /bin/sh
before	1 failed, 22 passed (existing assertion, expected 127 to be 125)	1 failed, 22 passed (new bash assertion, expected 127 to be 125)
after	23 passed	23 passed

Without the new assertion, the old wrapper passes 23 of 23 with dash as /bin/sh, as on CI.

For the dash runs I bind-mounted a dash binary over /bin/sh inside a private user and mount namespace (unshare --user --map-root-user --mount), so the machine's own /bin/sh was never changed. /bin/sh is a symlink to bash there, so the bind also covers bash itself. I bind-mounted the real bash to a scratch path first and put it first on PATH, and checked $BASH_VERSION inside the namespace.

Shell probe of the new guard line under bash and dash:

target	exit	stderr
missing file	125	t3-acp-cgroup-wrapper: /nonexistent-t3-probe: not an executable file
directory	125	same form
non-executable regular file	125	same form
/bin/true	0	empty

The old tail returned 127, 126 and 126 on bash for the first three, and 125 on dash.

Also ran targeted vp lint on both files, tsc --noEmit for packages/provider-acp and apps/server (no errors), and vp fmt --check.

Not checked:

• Bash versions other than the 5.3 release tested here. fix(server): a missing agent binary no longer reads as a wrapper kill #15344 reports the same result on bash 5.3.20.
• busybox ash and bash --posix with the new guard. The table above is from the old wrapper.
• macOS, where this Linux-only wrapper does not run.

Made with Claude Opus 5.5 in T3 Code (Claude Code harness).

[macroscopeapp]

Approvability

Verdict: Approved at fc50387

Macroscope's review found this PR approvable — This is a narrowly scoped Linux cgroup wrapper bug fix that standardizes missing-target handling across shells and adds focused regression coverage. Existing valid-agent execution remains unchanged, with no schema, default, security, or deployment impact.

Notes:

• Diff unchanged. Approvability was decided on eligibility alone.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 88607b12-bcdf-4806-b576-c4de3b661bc5

---

📥 Commits

Reviewing files that changed from the base of the PR and between 5f7294d and fc50387.

---

📒 Files selected for processing (2)

• apps/server/src/provider/acp/AcpSessionRuntime.processTree.test.ts
• packages/provider-acp/src/server/AcpSessionRuntime.ts

---

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

---

📝 Walkthrough

Walkthrough

The Linux cgroup wrapper now checks whether the target command is a regular executable file before running it. The test checks that a missing target returns status 125 and reports its path.

Changes

Linux cgroup target validation

Layer / File(s)	Summary
Validate target before execution packages/provider-acp/src/server/AcpSessionRuntime.ts, apps/server/src/provider/acp/AcpSessionRuntime.processTree.test.ts	The wrapper reports a missing or non-executable target to stderr and exits with status 125. The test checks this result when Bash is available.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

---

Merge Risk: ⚪ Minimal · up to fc503

The change rejects invalid targets with status 125 while allowing valid targets to proceed. The supplied code and tests establish no actionable merge-blocking risk.

Architecture Summary

Architecture risk: 🔵 Low · up to fc503

The change affects 2 systems.

Changed systems: packages/provider-acp, apps/server

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — packages/provider-acp (library) was modified; 1 changed file maps to changed impact.
• observed — apps/server (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in apps/server/src/provider/acp/AcpSessionRuntime.processTree.test.ts: The missing-target test adds a Bash-specific check, conditional on Bash being available: it expects exit status 125 and stderr to contain the missing executable path.
• observed — Modified behavior in packages/provider-acp/src/server/AcpSessionRuntime.ts: The Linux cgroup wrapper adds an executable-file check before exec. If the target path is not a regular executable file, the wrapper reports the path to stderr and exits 125 rather than attempting execution.

---

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly identifies the provider fix and the shell-dependent exit-status issue for missing agent binaries.
Description check		The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the fix, approval exception, test results, limitations, and agent usage.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

Dismissing prior approval to re-evaluate fc50387