Repository navigation
fix(cross-model): reach every peer through one pinned acpx transport - #1843
Merged
Merged
Conversation
Adds a scripted ACP stub agent and an opt-in contract suite (`bun run test:acpx-contract`, CI job on ubuntu) that runs the real pinned acpx and pins what the cross-model workers will depend on: exit codes, the prompt result found by request id, permission denial after end_turn (exit 5), SIGTERM -> cancelled, pre-prompt model rejection, and .acpxrc.json agent overrides. Canned real acpx streams and a PATH-shadowing npx stub serve later worker tests, which never run real npx. Includes the acpx transport plan this work executes.
Every ce-pov peer route now reaches its agent through one pinned acpx (0.19.4) invocation and one ACP stream parser, replacing the per-CLI argv, envelope parsers, and the Claude-only receipt. - Success is the prompt's own end_turn result (exit 5 after a denied permission still publishes; a cancelled turn does not). - Failures before the prompt is sent report "transport unavailable (pre-egress, shared|route)": Node older than 22.13, npx or npm fetch failure, a missing agent CLI, an acpx config file that replaces the route's agent launch, or a rejected model. - Served models come from the adapter's _meta report, so codex and the native grok route now record one too. - Each route runs in its read-only mode where the adapter has one; Claude launches through a --safe-mode wrapper so the reviewed repository's project settings and hooks do not apply. The codex adapter does not enforce read-only, which the reference and guide now state. - OpenCode runs in plan mode: denying bash outright makes its free tier reject the session over ACP. The acpx transport block is parity-tested for every migrated worker and pinned to the version the contract suite tests.
Cross-model peers now run through acpx, which needs Node 22.13 or newer and npx. The health report gains a `node` row for that capability, and ce-setup offers, with approval, to warm the npm cache for the pinned acpx and the codex and claude adapters it launches (`check-health --warm-acpx`), so the first peer run does not wait on a download. Warming installs each package into npx's cache with `npx --package=<spec> -- node --version`; `npm cache add` only caches the top-level tarball, not the install npx reuses. The pin lines are parity-tested against the workers' pin.
The doc-review worker now reaches every route through the pinned acpx with --deny-all from an empty per-peer workspace, sharing ce-pov's acpx transport block (parity-tested) and receipt logic. Native per-CLI argv, envelope parsers, and the hard-only grok path are removed. - The trust boundary is restated from live probes: claude, grok-cli, and opencode refuse reads outside the empty workspace; codex and the Cursor routes can still read anywhere, and codex can write because its adapter does not enforce read-only mode. - The one-time overload retry now fires only on the ACP form a provider overload takes (claude-agent-acp's errorKind "overloaded"); other adapters retry overloads themselves. - Pre-egress transport failures report the lens as not run, with no replacement route when the failure is shared by every route. - A workspace that cannot be created now skips the peer instead of falling back to the run dir. ce-pov's adapter comment is corrected: Claude's --allowed-tools is an auto-approve list, not a tool restriction.
The adversarial-review worker now reaches every route through the pinned acpx from the repository root, sharing the acpx transport block, receipt kernel, override validation, and run loop with ce-doc-review and ce-pov (all parity-tested). Native per-CLI argv, envelope parsing, and the codex `git diff` prompt variant are removed. - Large diffs: every route gets the private path of the full staged diff and reads it selectively. OpenCode's config allows external-directory reads for that, and grok-cli runs with --no-fs on large diffs, because acpx's file access refuses paths outside --cwd. Grok's own file tools also write without asking, so it does so only when the diff must be read from disk. - --max-turns reaches only Claude over ACP, so only the claude route carries a turn limit. - adversarial-codex-usage.json is now built from codex's ACP usage report; the unread raw event log, which would hold the outbound prompt, is no longer kept. - The recovery reference treats a pre-egress transport failure as "not run": a shared failure goes straight to the in-process reviewer, a route failure still allows the one replacement. - The trust boundary is restated per route from live probes, including that the codex adapter can write.
…acpx ce-work's write-capable worker now reaches every route through the pinned acpx with --approve-all in the prepared workspace, carrying the same parity-tested transport block as the review workers. Native argv, output parsers, the codex -o result file, and the Cursor --list-models probe are removed. - Cursor routes request ACP preset ids (grok-cursor at Cursor's high/fast preset, composer at fast); the controller accepts the bracketed form and compares served models on the id before "[". - A served model matching the requested family is recorded "asserted" (the adapter's own report), never "verified"; a mismatch still blocks. - Live probes show no route confines the worker to its workspace over ACP, so every route records restriction_posture "cooperative" and a run requiring enforced confinement finds every route unavailable. The reference records what still limits each route. - The env -i allowlist adds npm's cache, registry, and user-config locations, never token values. - Redaction covers JSON-escaped forms of each value and runs on the ACP stream (which echoes the prompt and file contents) before any log, reply text, or failure reason is taken from it. - Success is the prompt's own end_turn; a prompt never sent reports "transport unavailable (pre-egress, shared|route)". - OpenCode takes no effort setting over ACP.
acpx is pre-1.0 and has broken compatibility in patch releases, so the pin is exact. A weekly (and manually dispatchable) workflow resolves the newest acpx, runs the contract suite against it in a job with read-only permissions and no persisted token, and only then, in jobs that never execute acpx: - on green, rewrites every pin copy and ce-setup's adapter specs with scripts/bump-acpx-pin.ts (specs read as text from the packed acpx agent registry), opens or updates one bump PR, and dispatches ci.yml on that branch, since a PR opened with GITHUB_TOKEN triggers no workflow; - on red, opens or updates one tracking issue with the failing scenarios. The bump script discovers pins by pattern, refuses copies that disagree or are malformed, and is a no-op on a second run.
Git Bash's ps has no -o, so peer_alive reported a live peer as dead at once: the idle and hard-cap guards never fired and the worker blocked until the peer exited on its own. It now falls back to `ps -p` when `ps -o` is unsupported. OpenCode launches through acpx's raw --agent command, which acpx rejects on native Windows, so the shared transport block reports that route as a route-scoped pre-egress failure there instead of failing after launch. The windows-native CI job gains a Git Bash smoke that runs the ce-pov worker through peer-job-runner.py with a stub npx: a published result with its receipt, an idle reap that leaves no stub process, and opencode reported unavailable.
The guides now state that OpenCode peers are unavailable on native Windows, and the cross-model eval packs describe the stub npx harness that replays canned acpx streams instead of fake per-CLI binaries.
…iew nits npx resolves a package from its working directory's node_modules and .npmrc before the cache or registry, so a repository with its own acpx or npm config could decide which acpx ran. Every worker now starts npx from its private scratch directory and passes the repository to the agent only through --cwd; ce-setup's cache warm runs from an empty temporary directory too, so it fills the same cache entries the workers read. Also from the branch review: - auto-model routes (Cursor default, OpenCode) record an adapter-reported model without logging a false "model mismatch"; - the config template and its example copy list OpenCode among the routes that take no effort setting; - ce-doc-review drops the out_missing_or_invalid helper whose only caller the migration removed; - the pin-bump script's error paths (disagreeing copies, duplicate or missing pins, an unreadable agent registry) are now tested.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 66ccbb885e
ℹ️ 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}
npm launches acpx through `sh -c`. bash execs the command, but Ubuntu's dash does not, so a TERM sent to the npx leader stopped at the shell: on Linux, ce-work's timeout and raw-output-cap stops left acpx and the write-capable agent running. The route now runs in its own process group and every stop signals the group, as the review workers already do. The acpx contract suite hit the same shell boundary (the SIGTERM case hung on ubuntu CI) because it signalled npx. It now signals the acpx process itself, which is the contract the workers depend on.
- bump-acpx-pin accepts only plain `[@scope/]name@range` adapter specs. The specs come from the untrusted acpx package and are written into a bash assignment check-health expands, so a spec carrying `$(...)`, backticks, quotes, a backslash, or a glob is now refused. - The canary's bump-PR body carries the required Security and Agent Disclosure sections, which --body-file would otherwise skip. - ce-work's route argv and effort-override tests run one emit per case instead of looping many subprocesses inside one test.
…talled --emit-adapter printed `--agent acp` when opencode was not on PATH, which failed the route tests on CI runners. The launch falls back to the bare `opencode` name; a live run still refuses the route in preflight when the CLI is missing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6f60da4ca
ℹ️ 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: 5cb0ea199f
ℹ️ 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".
- ce-work parses the ACP stream from the private raw copy. A redaction value that also appears in protocol text (such as `end_turn`) used to corrupt the outcome; logs, failure evidence, and the published receipt are still redacted. - ce-setup's cache-warm offer says npm runs any install scripts the packages declare, as a first peer run would, instead of claiming nothing from the packages starts. - ce-setup offers the warm only when the bundled health script reported the node row, since the warm runs that script; the inline fallback route no longer offers an action it cannot perform.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c5290c628
ℹ️ 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".
- The worker route tests stub node at a fixed compliant version instead of using the machine's. The workers refuse Node older than 22.13, so on an older Node every expected-success route test failed before the stub npx ran. - The ce-pov, ce-doc-review, and ce-code-review guides list jq among the peer prerequisites, as the ce-work guide already did.
- The three review workers accept cursor-grok-* model overrides on the grok-cursor route again, as main and ce-work do. Widening grok-4.7-* to grok-* had dropped that spelling, so a configured override skipped a valid Grok peer. - The acpx-contract CI job runs npm-hosted acpx with read-only contents and no persisted checkout credential, matching the canary's contract job.
michael-wojcik
added a commit
to michael-wojcik/compound-engineering-dsh
that referenced
this pull request
Oct 9, 2026
… opt-in gate Upstream moved one commit, EveryInc#1843: a large cross-model transport rework that replaces per-provider peer invocation with one pinned acpx, and touches four files that carry this port's bindings. The rebase was clean - 31 commits replayed with no conflicts - because the bindings sit at the head of those files and upstream edited elsewhere. Mirror unchanged at +140/-0, zero upstream lines modified or removed repo-wide. The binding claims were checked against the rework rather than assumed: every token they rest on still exists in the tree (peer.outcome in 4 files, job id in 4, reap in 8, detached in 11), so the transport swap changed the route beneath the contract without changing the contract the binding names. The candidate list moved 23 to 24, and check 7 caught it before the doc did again. The new file is ce-work/references/cross-model-execution.md, whose added sentence about how every route *reaches its agent* matches the launcher pattern. It governs external CLI workers and never instructs a subagent dispatch, so it is a candidate rather than a binding site. The opt-in transport gate now has a result: ACPX_CONTRACT=1 passes 9/9 and acpx-transport-parity 2/2. On a cold npx cache the first test fails at its 60-second bound while the other eight pass, which reads as a broken transport and is only a cold fetch. Then the full suite was re-run the way the captured learning prescribes: 4511 pass, 10 skip, 8 fail, and zero cross-model transaction failures. The family the gitignore diagnosis named is gone, and the total falls from 20 to 8. Each residual family was checked for fork responsibility and none has any.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Cross-model peers in
ce-pov,ce-doc-review,ce-code-review, andce-worknow reach every model through one pinned acpx transport instead of a hand-built adapter per CLI. A CLI that changes its flags or output format no longer breaks peers until someone patches four scripts. Peers that used to fail now run, and each route's real trust boundary is stated from live probes.This lands the whole plan (
docs/plans/2026-09-28-1150-refactor-acpx-cross-model-transport-plan.md) in one PR. The plan called for one PR per worker; every worker carries the same parity-tested transport block, and a stack would have left the tree half-migrated between merges.What changes for users
npx. Each worker checks it before contacting any provider. A missing prerequisite reportstransport unavailable (pre-egress, shared|route): <reason>, nothing is sent, and the skill reports that peer as not run.ce-setupgains anodehealth row and offers, with approval, to warm the npm cache so the first peer run doesn't wait on a download.ce-code-reviewused to send grok-cli and OpenCode a truncated diff; every route now reads the full staged diff from private temp.--safe-mode, so the reviewed repo's settings and hooks don't applyDesign decisions
session/promptresult (end_turn), not the exit code.bypassPermissionsthat default, a "read-only" peer could write; every Claude route now setsmode=default(review) or a deliberate mode (ce-work).node_modulesand.npmrcfirst. The agent still gets the repo through--cwd.bashorreadin OpenCode's config makes its free tier reject the session over ACP.Session-settled decisions carried from planning: acpx required with native adapters deleted (user-directed, over a ce-pov-only trial); native Windows tested, not special-cased (user-directed); read-only posture best effort, not a gate (user-directed); hostile checkout out of scope (user-directed, over npm-config guards); grok-cursor at Cursor's high/fast preset (user-directed); exact pin with a weekly canary (user-directed, over floating
latest).Validation
bun run test: 4493 pass, 0 fail, plus typecheck,release:validate, and skill guards.bun run test:acpx-contract, new CI job): runs the real acpx 0.19.4 against a stub agent and pins the exit codes, prompt-id matching, cancellation, model rejection, and config-override behavior the parser depends on.ce-code-reviewlarge diffs (316 KB) were read in full on grok-cli and OpenCode. A Claude peer in a repo with project hooks did not fire them. Realce-workunits on codex and grok-cursor completed and were committed by the host.ce-pov,ce-doc-review, andce-code-review: no regressions. One run per cell.windows-nativeCI runs for the first time on this PR.Known residual risks:
--safe-modewrapper probably can't launch on native Windows; that route then reports itself unavailable.Related: #1797
Related: #1776
New concepts
Agent Client Protocol (ACP) through acpx. ACP is a JSON-RPC protocol between a client and a coding agent: the client opens a session, sends a prompt, and receives streamed
session/updatemessages, permission requests, and a finalstopReason. acpx is a headless ACP client with built-in launchers for Codex, Claude, Grok, Cursor, OpenCode, and others. It prints the raw stream with--format json.Before this PR, each CLI had its own argv builder and output parser, and each broke on that CLI's releases. With ACP, the worker reads one message shape for every route. It finds the result whose
idmatches its ownsession/promptrequest and reads the reply fromagent_message_chunkupdates.In this PR,
acpx_outcometreats exit 5 afterend_turnas success (a permission was denied but the turn finished) and exit 0 withcancelledas failure.ACP is the wrong tool when you need a CLI-only feature its adapter doesn't expose. Codex's
-s read-onlysandbox is one such feature, and this PR gives it up.Security Disclosure
npx -y acpx@0.19.4. That command fetches acpx, which launches the codex and claude adapters through npx within acpx's own version ranges. The acpx pin is exact; the adapters float within those ranges (accepted in planning).codex exec -s read-onlyenforced it. grok-cli on largece-code-reviewdiffs can also write. Noce-workroute confines its worker to the workspace, so every route recordscooperativeposture. All of this is documented in each skill's trust-boundary section; read-only was settled as best effort.bypassPermissionsfrom user settings.ce-workenvironment. Theenv -iallowlist gains npm's cache, registry, and user-config locations, never token values. Redaction now covers JSON-escaped forms of each secret and runs on the ACP stream, which echoes the prompt and file contents, before any log or failure evidence is written.acpx@latesthas read-onlycontentsand no persisted token; PR, issue, and dispatch writes happen in jobs that never execute acpx.Agent Disclosure
Claude Code · claude-opus-5-5