feat(v1): run Codex and Claude through ACP - #2257
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 245129a234
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. This PR introduces a new feature changing how Codex and Claude Code agents are run (via ACP adapters). Additionally, there are two unresolved high-severity review comments identifying security concerns in the new Node.js installation logic - specifically around potential arbitrary code execution from shared paths and indefinite hangs from orphaned lock files. You can customize Macroscope's approvability policy. Learn more. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9136e1d803
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b95444dfa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b95444d to
98eb1a5
Compare
| command -v curl >/dev/null 2>&1 \ | ||
| || { apt-get update -qq && apt-get install -y -qq curl ca-certificates >/dev/null; } | ||
| case "$(uname -s)" in Linux) node_os=linux ;; Darwin) node_os=darwin ;; *) echo "unsupported os: $(uname -s)" >&2; exit 1 ;; esac | ||
| if [ ! -x "$node/bin/node" ] || [ "$("$node/bin/node" --version 2>/dev/null)" != "v$VF_NODE_VERSION" ]; then |
There was a problem hiding this comment.
🟠 High harnesses/node.py:28
The installer executes a pre-existing /var/tmp/vf-node/bin/node binary before verifying it owns or created the NODE_DIR directory. On a shared host, another user can pre-create /var/tmp/vf-node/bin/node with an executable payload, and the next ensure_node call runs that payload with the verifier worker's privileges — both in the node_ok version check and the if [ ! -x "$node/bin/node" ] guard. Consider creating $node under a user-private path or securely initializing the directory with verified ownership before executing any binary from it.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/harnesses/node.py around line 28:
The installer executes a pre-existing `/var/tmp/vf-node/bin/node` binary before verifying it owns or created the `NODE_DIR` directory. On a shared host, another user can pre-create `/var/tmp/vf-node/bin/node` with an executable payload, and the next `ensure_node` call runs that payload with the verifier worker's privileges — both in the `node_ok` version check and the `if [ ! -x "$node/bin/node" ]` guard. Consider creating `$node` under a user-private path or securely initializing the directory with verified ownership before executing any binary from it.
| async def ensure_node(runtime: Runtime) -> None: | ||
| """Install the shared Node runtime used by ACP adapter harnesses.""" | ||
| lock = f"{NODE_DIR}.install.lock" | ||
| guarded = ( |
There was a problem hiding this comment.
🟠 High harnesses/node.py:43
If a Node install process is killed before its EXIT trap runs and its PID is later reused by an unrelated long-lived process, every subsequent ensure_node call hangs indefinitely on the orphaned /var/tmp/vf-node.install.lock. The stale-lock recovery in the spin loop checks only kill -0 "$owner", so any live process with that PID keeps the loop waiting forever. Consider replacing the PID-based heuristic with flock or adding bounded retry/timeout logic so an orphaned lock is eventually reclaimed.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @verifiers/v1/harnesses/node.py around line 43:
If a Node install process is killed before its `EXIT` trap runs and its PID is later reused by an unrelated long-lived process, every subsequent `ensure_node` call hangs indefinitely on the orphaned `/var/tmp/vf-node.install.lock`. The stale-lock recovery in the spin loop checks only `kill -0 "$owner"`, so any live process with that PID keeps the loop waiting forever. Consider replacing the PID-based heuristic with `flock` or adding bounded retry/timeout logic so an orphaned lock is eventually reclaimed.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 98eb1a5. Configure here.
2 participants
Overview
Run the Codex and Claude Code harnesses through maintained ACP adapters so they use the same saved-session continuation path as the existing ACP harnesses.
Details
exec resume --lastflow with the Codex App Server ACP adapterThe remaining harnesses keep their current transports: Hermes, Kimi, Pi, Pool, and OpenClaw already use ACP, while the in-house/stateless harnesses and Terminus 2 do not have equivalent maintained resumable ACP adapters.
Note
Medium Risk
Large harness transport swap for two heavily used agents adds runtime dependency on npm/Node and changes resume semantics away from Codex's native
exec resume --lastflow.Overview
Codex and Claude Code no longer invoke their own CLIs directly; both install pinned npm ACP packages and run through the shared
ACPharness path (persistentsessionwhere the runtime supports live processes,session_pathfor one-shot resume on launch).The ACP stack gains optional
session_meta, threaded through config JSON intonew_session/resume_session/load_session, and treated as part of persistent session identity so mid-rollout config changes fail fast. Claude passes MCP strictness, disallowed tools, and appended system prompt via that metadata; Codex keeps interception routing, MCP timeouts, feature flags, and namespace hashing via env/CODEX_CONFIGinstead of CLI-cflags.A new
ensure_nodehelper centralizes Node 22.19+ install under/var/tmp/vf-node; Pi drops its duplicate Node bootstrap and uses it. Claude skills discovery moves to.claude/skills; ClaudeSUPPORTS_RESUMEflips on.E2e
ACP_RESUME_PLACEMENTSadds docker rows for codex and claude-code; test helpers rename_pair/_mto publicpair/mark.Reviewed by Cursor Bugbot for commit 4aa588a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Run Codex and Claude Code harnesses through the ACP adapter
@agentclientprotocol/codex-acpand the Claude Agent SDK) instead of direct binary or app invocation.ensure_node) so harnesses install a consistent Node.js runtime; pi/harness.py is updated to use it too.session_metasupport throughout the ACP layer (acp/init.py, runner.py) so harnesses can pass per-session options (disabled tools, system prompt, MCP config) to the agent process.codexandclaude-codein the docker runtime.Macroscope summarized 4aa588a.