Skip to content

Commit 67035e9

Browse files
tmchowtmchow
authored
fix(cross-model): reach every peer through one pinned acpx transport (EveryInc#1843)
1 parent cef001f commit 67035e9

64 files changed

Lines changed: 5919 additions & 5626 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.compound-engineering/config.example.yaml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@
125125
# routes through a non-default `model_provider`).
126126
# Effort levels: claude low|medium|high|xhigh|max, codex low|medium|high|
127127
# xhigh plus max and ultra on the models that list them, grok low|medium|high|
128-
# xhigh; cursor/composer routes take none. A value the peer cannot honor skips
129-
# the pass with a stated reason -- never substituted.
128+
# xhigh; cursor/composer and OpenCode routes take none. A value the peer
129+
# cannot honor skips the pass with a stated reason -- never substituted.
130130

131131
# cross_model_review_mode: off # auto | off (default: auto)
132132
# cross_model_peer: codex # codex | claude | grok | cursor | composer | opencode
@@ -156,8 +156,8 @@
156156

157157
# Reasoning effort for the external worker, per harness. Each value is one of
158158
# that harness's own effort levels. An unset harness keeps its own default:
159-
# Codex and Claude at high, native Grok at xhigh, OpenCode with no variant.
160-
# Cursor routes take no effort. A map in config.local.yaml replaces this one.
159+
# Codex and Claude at high, native Grok at xhigh. OpenCode and Cursor routes
160+
# take no effort. A map in config.local.yaml replaces this one.
161161

162162
# work_engine_effort:
163163
# codex: xhigh

‎.github/workflows/acpx-canary.yml‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
name: acpx canary
2+
3+
# Tests the newest acpx against the contract suite and proposes a pin bump.
4+
# Only `contract` executes acpx, so it gets no write permissions and no persisted
5+
# token; the jobs that write to the repo never run acpx.
6+
7+
on:
8+
schedule:
9+
- cron: "17 9 * * 1"
10+
workflow_dispatch:
11+
12+
permissions: {}
13+
14+
concurrency:
15+
group: acpx-canary
16+
cancel-in-progress: false
17+
18+
env:
19+
BUMP_BRANCH: ci/acpx-bump
20+
ISSUE_TITLE: "acpx canary: newest acpx fails the contract suite"
21+
22+
jobs:
23+
resolve:
24+
runs-on: ubuntu-latest
25+
timeout-minutes: 5
26+
permissions:
27+
contents: read
28+
outputs:
29+
latest: ${{ steps.versions.outputs.latest }}
30+
pin: ${{ steps.versions.outputs.pin }}
31+
32+
steps:
33+
- uses: actions/checkout@v6
34+
with:
35+
persist-credentials: false
36+
37+
# A concrete version, never `latest`: the suite asserts `acpx --version` equals it.
38+
- name: Resolve newest acpx and the current pin
39+
id: versions
40+
run: |
41+
latest=$(npm view acpx version)
42+
pin=$(sed -nE 's/^export const ACPX_PIN = "([0-9]+\.[0-9]+\.[0-9]+)"$/\1/p' tests/helpers/acpx-pin.ts)
43+
for v in "$latest" "$pin"; do
44+
[[ "$v" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]] || { echo "::error::unexpected version string: '$v'"; exit 1; }
45+
done
46+
echo "latest=$latest" >> "$GITHUB_OUTPUT"
47+
echo "pin=$pin" >> "$GITHUB_OUTPUT"
48+
if [ "$latest" = "$pin" ]; then
49+
echo "acpx $pin is already the newest release; nothing to do." >> "$GITHUB_STEP_SUMMARY"
50+
fi
51+
52+
contract:
53+
needs: resolve
54+
if: needs.resolve.outputs.latest != needs.resolve.outputs.pin
55+
runs-on: ubuntu-latest
56+
timeout-minutes: 10
57+
permissions:
58+
contents: read
59+
60+
steps:
61+
- uses: actions/checkout@v6
62+
with:
63+
persist-credentials: false
64+
65+
# acpx requires Node >= 22.13.
66+
- name: Setup Node
67+
uses: actions/setup-node@v6
68+
with:
69+
node-version: "22"
70+
71+
- name: Setup Bun
72+
uses: oven-sh/setup-bun@v2
73+
with:
74+
bun-version: latest
75+
76+
- name: Install dependencies
77+
run: bun install
78+
79+
- name: acpx contract suite against acpx ${{ needs.resolve.outputs.latest }}
80+
shell: bash
81+
env:
82+
ACPX_VERSION: ${{ needs.resolve.outputs.latest }}
83+
run: bun run test:acpx-contract 2>&1 | tee "$RUNNER_TEMP/contract.log"
84+
85+
- name: Keep the contract log
86+
if: always()
87+
uses: actions/upload-artifact@v4
88+
with:
89+
name: acpx-contract-log
90+
path: ${{ runner.temp }}/contract.log
91+
if-no-files-found: ignore
92+
93+
propose:
94+
needs: [resolve, contract]
95+
runs-on: ubuntu-latest
96+
timeout-minutes: 10
97+
permissions:
98+
contents: write
99+
pull-requests: write
100+
actions: write
101+
102+
steps:
103+
- uses: actions/checkout@v6
104+
105+
- name: Setup Bun
106+
uses: oven-sh/setup-bun@v2
107+
with:
108+
bun-version: latest
109+
110+
# The registry is read as text by the bump script; nothing from the package runs.
111+
- name: Fetch the acpx package without running it
112+
env:
113+
VERSION: ${{ needs.resolve.outputs.latest }}
114+
run: |
115+
mkdir -p "$RUNNER_TEMP/acpx"
116+
npm pack "acpx@$VERSION" --ignore-scripts --pack-destination "$RUNNER_TEMP/acpx"
117+
tar -xzf "$RUNNER_TEMP/acpx/acpx-$VERSION.tgz" -C "$RUNNER_TEMP/acpx"
118+
119+
- name: Bump the pin
120+
env:
121+
VERSION: ${{ needs.resolve.outputs.latest }}
122+
run: |
123+
bun scripts/bump-acpx-pin.ts --version "$VERSION" --from-package "$RUNNER_TEMP/acpx/package" > "$RUNNER_TEMP/bump.md"
124+
cat "$RUNNER_TEMP/bump.md"
125+
126+
- name: Open or update the bump PR
127+
env:
128+
GH_TOKEN: ${{ github.token }}
129+
VERSION: ${{ needs.resolve.outputs.latest }}
130+
run: |
131+
title="fix(cross-model): bump acpx to $VERSION"
132+
git config user.name "github-actions[bot]"
133+
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
134+
git switch -C "$BUMP_BRANCH"
135+
git diff --quiet && { echo "::error::the bump script changed no files"; exit 1; }
136+
git commit -am "$title"
137+
git push --force origin "$BUMP_BRANCH"
138+
{
139+
echo "Weekly acpx canary: acpx $VERSION passed the contract suite (run $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID)."
140+
echo
141+
cat "$RUNNER_TEMP/bump.md"
142+
echo
143+
echo "Before merging, run a local live smoke for each adapter whose spec changed above: the contract suite drives a stub agent, not the real adapters."
144+
echo
145+
echo "CI was dispatched on \`$BUMP_BRANCH\` because a PR opened with GITHUB_TOKEN does not trigger workflows."
146+
echo
147+
# --body-file bypasses the PR template, so its required sections are written here.
148+
echo "## Security Disclosure"
149+
echo
150+
echo "Changes which acpx version, and which codex and claude adapter version ranges, the cross-model peers fetch from npm and run on users' machines. The new version passed the contract suite against a stub agent only; review its release notes before merging."
151+
echo
152+
echo "## Agent Disclosure"
153+
echo "- **Model:** No agent used. Generated by the acpx canary workflow."
154+
} > "$RUNNER_TEMP/body.md"
155+
pr=$(gh pr list --head "$BUMP_BRANCH" --state open --json number --jq '.[0].number')
156+
if [ -n "$pr" ]; then
157+
gh pr edit "$pr" --title "$title" --body-file "$RUNNER_TEMP/body.md"
158+
else
159+
gh pr create --head "$BUMP_BRANCH" --title "$title" --body-file "$RUNNER_TEMP/body.md"
160+
fi
161+
162+
- name: Run CI on the bump branch
163+
env:
164+
GH_TOKEN: ${{ github.token }}
165+
run: gh workflow run ci.yml --ref "$BUMP_BRANCH"
166+
167+
report:
168+
needs: [resolve, contract]
169+
if: failure() && needs.contract.result == 'failure'
170+
runs-on: ubuntu-latest
171+
timeout-minutes: 5
172+
permissions:
173+
issues: write
174+
175+
steps:
176+
- name: Fetch the contract log
177+
uses: actions/download-artifact@v4
178+
continue-on-error: true
179+
with:
180+
name: acpx-contract-log
181+
path: ${{ runner.temp }}/log
182+
183+
# The log came from a job that ran untrusted code; it is only quoted, never executed.
184+
- name: Open or update the tracking issue
185+
env:
186+
GH_TOKEN: ${{ github.token }}
187+
GH_REPO: ${{ github.repository }}
188+
VERSION: ${{ needs.resolve.outputs.latest }}
189+
PIN: ${{ needs.resolve.outputs.pin }}
190+
run: |
191+
failures=$(grep -E '^\(fail\)' "$RUNNER_TEMP/log/contract.log" 2>/dev/null || true)
192+
{
193+
echo "acpx $VERSION failed the contract suite (pin stays at $PIN). Run: $GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID"
194+
echo
195+
echo "Failing scenarios:"
196+
echo
197+
echo '```'
198+
echo "${failures:-none recorded; see the run log}"
199+
echo '```'
200+
} > "$RUNNER_TEMP/body.md"
201+
issue=$(gh issue list --state open --search "in:title \"$ISSUE_TITLE\"" --json number,title \
202+
--jq "map(select(.title == \"$ISSUE_TITLE\")) | .[0].number // empty")
203+
if [ -n "$issue" ]; then
204+
gh issue comment "$issue" --body-file "$RUNNER_TEMP/body.md"
205+
else
206+
gh issue create --title "$ISSUE_TITLE" --body-file "$RUNNER_TEMP/body.md"
207+
fi

‎.github/workflows/ci.yml‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,36 @@ jobs:
136136
if: ${{ !cancelled() }}
137137
run: bun run typecheck
138138

139+
# Runs the pinned acpx from the npm registry against the stub ACP agent. Kept out
140+
# of `bun run test` because it needs network access to fetch acpx.
141+
acpx-contract:
142+
runs-on: ubuntu-latest
143+
timeout-minutes: 10
144+
permissions:
145+
contents: read
146+
147+
steps:
148+
- uses: actions/checkout@v6
149+
with:
150+
persist-credentials: false
151+
152+
# acpx requires Node >= 22.13.
153+
- name: Setup Node
154+
uses: actions/setup-node@v6
155+
with:
156+
node-version: "22"
157+
158+
- name: Setup Bun
159+
uses: oven-sh/setup-bun@v2
160+
with:
161+
bun-version: latest
162+
163+
- name: Install dependencies
164+
run: bun install
165+
166+
- name: acpx contract suite
167+
run: bun run test:acpx-contract
168+
139169
# Focused Windows gate for the skill scripts that reach for POSIX process and
140170
# file primitives: peer-job-runner (#1243) and ce-babysit-pr's pr-snapshot
141171
# (#1280). Not a full-suite matrix — POSIX lifecycle tests (fork/killpg/ps)
@@ -265,6 +295,20 @@ jobs:
265295
- name: Scratch-root preamble executes under Git Bash
266296
run: bun test tests/scratch-root-preamble-executes.test.ts
267297

298+
# acpx needs Node >= 22.13; the runner image's default Node may be older.
299+
- name: Setup Node for the acpx worker smoke
300+
uses: actions/setup-node@v6
301+
with:
302+
node-version: "22"
303+
304+
# Runs a cross-model worker through peer-job-runner.py under Git Bash with a
305+
# stub npx: a published result, an idle reap with no surviving process (Git
306+
# Bash's ps has no -o), and opencode reported unavailable on native Windows.
307+
- name: acpx worker smoke under Git Bash
308+
shell: bash
309+
timeout-minutes: 4
310+
run: bash tests/windows/acpx-worker-smoke.sh
311+
268312
# The cross-model workers' skip-evidence helper once stalled for minutes on macOS
269313
# /bin/bash 3.2 (#1785). Ubuntu's bash 5 cannot reproduce that, so this job runs the
270314
# guard under the real 3.2 and fails if the image stops shipping it.

‎docs/guides/ce-code-review.md‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,20 @@ That last point is what makes review strictness compound. Notice a mistake worth
124124

125125
## Cross-model adversarial pass
126126

127-
When adversarial is selected and the working tree is the reviewed head (current branch, or a PR whose local tree already matches the PR head), the adversarial lens runs through one model provider different from the host, in a separate read-only process. A started peer replaces the in-process `adversarial` persona; they never both receive the same brief. The in-process persona runs if the peer cannot start, or if the started peer never reviewed the diff, for example because of a session quota, an auth-context failure, or a CLI that rejects an option its installed version lacks. In that case the next announced different-family peer is tried when one is eligible, otherwise the local persona covers the lens. An exact provider-overload 529 gets one same-route retry; repeated overload, another stubborn transient rate limit, or max-turn exhaustion falls back locally without an unbounded retry loop. Remote PR or branch diffs stay on the in-process persona, because that reviewer can inspect the fetched refs.
127+
When adversarial is selected and the working tree is the reviewed head (current branch, or a PR whose local tree already matches the PR head), the adversarial lens runs through one model provider different from the host, in a separate read-only process. A started peer replaces the in-process `adversarial` persona; they never both receive the same brief. The in-process persona runs if the peer cannot start, or if the started peer never reviewed the diff, for example because of a session quota, an auth-context failure, or a CLI that rejects an option its installed version lacks. In that case the next announced different-family peer is tried when one is eligible, otherwise the local persona covers the lens. A provider overload (HTTP 529) that the peer's adapter reports gets one same-route retry (today only the Claude adapter reports one); repeated overload, another stubborn transient rate limit, or max-turn exhaustion falls back locally without an unbounded retry loop. Remote PR or branch diffs stay on the in-process persona, because that reviewer can inspect the fetched refs.
128128

129129
Agreement between the peer and another in-process reviewer is a strong promotion signal in synthesis.
130130

131131
`cross_model_review_mode: off` in CE config keeps this pass from running at all. No peer is resolved and nothing leaves the host; the in-process reviewers cover the lens and Coverage says the pass was disabled by checkout config. A direct request in conversation for a peer overrides it for one run. The peer target is auto-chosen and overridable, in priority order: conversation, `cross_model_peer:` in CE config, active project instructions, then `codex → claude → grok → composer`. `Cursor` means `cursor-agent` using its configured default/Auto model. `Composer` means a Composer model through Cursor. `Grok` binds the native grok CLI when it is installed; Grok through Cursor is a different route, used when asked or when the grok CLI is missing and Cursor is allowed. Cursor Auto does not count as independent agreement unless its serving family is verified different from the host. `cross_model_model:` and `cross_model_effort:` in CE config pin that target's model (e.g. `fable` for claude or `gpt-6.1-sol` for codex, or a namespace-qualified codex id such as `openai.gpt-6.1-sol` when that CLI routes through a non-default `model_provider`) and reasoning effort; a value the peer cannot honor skips the pass with a stated reason rather than substituting. See the [configuration reference](./configuration.md).
132132

133133
The prerequisite is a peer agent CLI. The pass drives a read-only agent CLI (`codex`, `claude`, `grok`, `cursor-agent`, or `opencode`) so the peer can inspect the tree itself; a bare `OPENAI_API_KEY`, Anthropic key, or Gemini key does not enable it. Peers are discovered on `PATH`, plus the CLI bundled inside the Codex desktop app (`ChatGPT.app/Contents/Resources/codex` since the July 2026 app merger, or `Codex.app/…` on older installs; the app does not link it onto `PATH`). Gemini has no standalone peer target; it participates only through Cursor when `cursor-agent` attests a Gemini serving family. With no peer CLI installed the skill runs the in-process adversarial reviewer and reports "cross-model pass: not run"; the skip reason names what to install.
134134

135+
### Requirements and trust boundary
136+
137+
Peers run through [acpx](https://github.com/openclaw/acpx), which talks to each agent CLI over the Agent Client Protocol. It needs Node 22.13 or newer with `npx`, `jq`, and the peer's own CLI. On native Windows, OpenCode is reported unavailable, because acpx cannot launch it there. The first run fetches a pinned acpx version from npm; later runs reuse the npm cache. When a prerequisite is missing, that peer is reported as not run and nothing is sent.
138+
139+
Each peer runs from the repository root so it can read surrounding code. Its read requests are approved, every other permission request is denied, and each agent runs in its read-only mode where it has one. This is best effort rather than a sandbox. Codex, Claude, and the Cursor routes can read any file your account can read. Codex can also run shell commands and write files, because its adapter does not enforce read-only mode. The native Grok CLI can write files on a large diff, when it reads the diff with its own tools. The native Grok CLI can still search and fetch from the web. Claude peers start in safe mode and OpenCode peers ignore the repository's OpenCode config, so the repository's project settings and hooks do not apply to them. On a diff too large to inline, every peer gets the path of the full staged diff instead of the diff itself. Served models are recorded from what each adapter reports. Codex, Claude, and the native Grok CLI report one; Cursor and OpenCode routes stay unverified.
140+
135141
This shares the provider/route kernel with `ce-doc-review` but keeps a narrower scope: adversarial-only, diff/work-tree delivery, not doc-review's judgment trio or whole-doc sweep.
136142

137143
## Severity and autofix class are orthogonal

0 commit comments

Comments
 (0)