[lawrencecchen]

Summary

Fixes forward the findings of the independent re-review of merged #28332 (comment). None of them is a security defect; finding 1 is a test gap on a security path.

1. Swift test of the fork scope rule. No Swift test proved that an acp.session.fork of a chat outside the pane's scope waits for the click's scope credit. check.json runs only in the Rust crate. AgentPaneForkRelayTests.aForkFromOutsideTheScopeGoesOutOnlyWithTheClick drives the real AgentPaneTransport against the stand-in acpmux server: {"sessionId":"other","live":true} and {"sessionId":"other","throughSeq":3} get .gestureRequired with no click and nothing reaches acpmux; the pane's own chat forks with no click; after a click one fork goes out with its params unchanged; the next fork outside the scope needs its own click.
   • New check.json case "live fork from outside the scope uses scope credit" (count 54 -> 55 in tests/parity.rs).
   • The two comments that said Swift runs check.json (AgentPaneTransport+Check.swift, tests/parity.rs) now say that only the crate runs it and that the Swift rule has its own Swift tests.
2. params.json: the case "acp.session.fork every known param" now has "live": true.
3. acpmux message: a live fork of a chat on another computer was refused with "forking a chat on another computer through a turn is not supported yet". It names no turn, so it now gets "forking a chat on another computer is not supported yet". The throughSeq message does not change.
   • Localization: not localized, on purpose. It is acpmux daemon error text, which is English like every other acpmux RpcError. The pane never sends a fork of a peer's chat (webviews/.../conversation/messageMenu.ts forkServed refuses summary.peer; Fork Conversation then shows the localized notice.forkFailed), so only a raw protocol client reads this text. No string a person sees in the app changes.

The new Swift test and the two JSON cases pin existing behavior: they are coverage tests, not a red-first bug fix. They are expected to pass on the base too. Only the acpmux message has a red-first test. Commit ead6efbd071 in the history is NOT valid red evidence (its fixture lacked the peer, so it failed for the wrong reason). The red result is scratch 3cb9ea55783 (= ead6efbd071 + the fixture fix d4bfd23c647, without the message fix dc4d15e9e1a; never merge), job b27f3045d1a265a584d16074: it fails on the live assert only (left "forking a chat on another computer through a turn is not supported yet", right "forking a chat on another computer is not supported yet"). The green result is the head d4bfd23c647, job 25ae0ce9de68429cb575916b: PASS. Focused command: scripts/ci/cmux-tui-rust-check.sh test a_peer_chats_fork_is_refused_with_what_it_asked_for. The first review (FIX, #28512 (comment)) found that this test's fixture did not list the peer, so resolve_remote found nothing and the test failed for the wrong reason at both commits. d4bfd23c647 lists the peer, asserts that the fixture resolves, and corrects three more stale "Swift runs" comments in parity.rs.

Review request. The agent-pane policy file asks for the protocol/origin lead's review. The protocol/origin lead (session ad349) is asked to review this PR together with #28332 and d6c70e6. acpmux owner PL-acpmux (session cmux-4f): this PR changes one refusal message in server/fork_through.rs and adds its unit test.

Testing

All runs are on the fleet (cmux-ci, pool perf-lead); nothing was compiled or tested on the VM.

Fleet results (pool perf-lead; Rust steps went to Blacksmith overflow). Every attempt counted; no gate-notice: FLAKY in any passing log.

question	ref	job	result
compile-check (CmuxNext + test targets, app, cmux-tui; recipe 2)	head d4bfd23	bdeb92e3767a6bba4fbcf996	passed (169 s / 48 s / 36 s)
compile-check, same	dc4d15e	62e9b8d30408c6b8efd2bb74	passed
Swift AgentPaneForkRelayTests (3 tests, new one included)	dc4d15e (Swift unchanged since)	1022b994772f65d5e135ea90	passed, 1 attempt
Rust cmux-tui-rust-check.sh test full_check_order (55 cases)	dc4d15e (later commit changes only comments in parity.rs)	da2f676b55a0956e1f228e8b	PASS, 1 attempt
Rust test params_rule	dc4d15e	d7de6bad330a24d4e4ccbdb0	PASS, 1 attempt
Rust test a_peer_chats_fork_is_refused_with_what_it_asked_for, red	scratch 3cb9ea5 = ead6efb + the fixture fix, without the message fix (branch scratch/fork-relay-rereview-28332-red-never-merge, never merge)	b27f3045d1a265a584d16074	FAIL as expected, 2 attempts, both on the live assert: left "forking a chat on another computer through a turn is not supported yet", right "forking a chat on another computer is not supported yet"
same test, green	head d4bfd23	25ae0ce9de68429cb575916b	PASS, 1 attempt
Rust clippy	head d4bfd23	3f74be9526e58e7267ec49d4	pass
Superseded (fixture defect, not a result of the change)	dc4d15e / ead6efb	0b2ce261bfa5e4154390f170, 35eb3b762b8d5a88b1fc9e98	both FAIL, 2 attempts each, "no session matches "hb/peer-chat-1""

The Swift test and the two JSON cases passed at once, as expected: coverage of existing behavior.

Observed / Not run

• Observed: independent review SHIP at d4bfd23 (agent-pane: Swift test of the fork scope rule, live in the policy cases, live peer-fork message (#28332 re-review) #28512 (comment)). python3 scripts/cmux-next/prepush-gate.py passed on the head (no new violations; owner note for acpmux). The fleet results above.
• Not run: a tagged app build and a live fork in the app (the change is tests, two comments and one daemon error text that the pane never shows); the full cmux-tui test suite (only the named filters ran).

Changelog

none

Proof

No capture: no native UI change (tests, comments and one acpmux protocol error text).

[github-actions]

cmux status _{(one comment, edited in place; each line is updated by the job that owns it)}

Review d4bfd23: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr28512 -f ref=fix/fork-relay-rereview-28332 -f platform=macos -f backend_mode=local

Shipped in:

• cmux NEXT nightly 1.0.0-nightly.4115488208801 (dd66817742)
• cmux NEXT nightly 1.0.0-nightly.4115755828101 (7c0adaf184)

[lawrencecchen]

Review (independent review subagent, Claude Opus 5.5): FIX at dc4d15e
reviewer-session: 89d72a52-707b-46d0-bcac-eb11f0264909
author-session: fork-relay-rereview author (perf-lead subagent 7cace044-9478-4472-a3fc-7a0db6847bd9)
R9: pass
perf claim: none

Scope: git diff 5e5312c4b8e0..dc4d15e9e1ab, read-only in the worktree. I did not compile or run tests.

Findings

1. Blocking. The new Rust unit test fails at the head, and the red commit fails for a different reason.
   fork_through.rs tests::hub_with_a_peer_chat adds a row to hub.remote_sessions, but it does not add the peer hb to hub.peers. Hub::resolve_remote (hub/peers.rs:986) finds the row by its key. Then it returns None at let peer = self.peer(&peer_name)?; (hub/peers.rs:1029), because Hub::new starts with an empty peers map (hub/mod.rs:736) and the config {} has no peers. Thus to_session_fork returns the e from hub.resolve (no session matches "hb/peer-chat-1"). It does not return remote_refusal(live).

   • At dc4d15e the first assert_eq! (live) fails: the message is no session matches ..., not forking a chat on another computer is not supported yet.
   • At ead6efb the test fails on the same line, but not because of the message text that the fix changes. Thus the red-first evidence does not prove the fix.
   • key_route_tests.rs uses the same fixture, but its tests do not go through resolve_remote. hub/own_host.rs:144-148 add_peer exists for this reason: "A peer that is listed but never runs, so resolve_remote can route to it".
   • Smallest fix: in hub_with_a_peer_chat, add the peer as own_host.rs does:
     let (tx, _rx) = tokio::sync::mpsc::channel(8); hub.peers.lock().unwrap().insert("hb".into(), crate::peer::Peer::new("hb", "ws://127.0.0.1:9", None, None, tx));
     (Hub::peers is pub(super) in hub, so crate::server can read it; Peer::new is pub.) With the peer present, Hub::resolve returns not_found early for hb/... (own_host.rs:18-23), and resolve_remote returns Some. Then run the test again on the fleet at the red commit (the live assertion must fail on the message) and at the head (it must pass).
   • The fleet jobs for this test (0b2ce261bfa5e4154390f170 at the head, 35eb3b762b8d5a88b1fc9e98 at the red commit) were still running when I wrote this. I expect both to fail on the first assert_eq!, with left: "no session matches \"hb/peer-chat-1\"".

2. Not blocking. The Swift test is correct. I examined each step against the code:

   • The fork is in sourceScoped and not in sessionScoped or gestureRules. Thus sessionRefusal and the grant rule do not refuse it. A fork of other sets foreignSource (AgentPaneTransport+Check.swift:153). The frame is not free and goes to decide. With no click, gestures.consumeScope() is false and the result is .gestureRequired (AgentPaneTransport+Frames.swift:461-465). This is true for live and for throughSeq. The refusal returns before encodeAndSend, so sent().isEmpty is a valid check.
   • The fork of s is free and goes out from analyzeOne. The stand-in server records each frame before it replies, so sent() after the "id":3 reply is complete. The relay sends a fresh serialization with only the page's params (no added _meta for this method), so the NSDictionary equality is correct (true and 3 bridge to equal NSNumbers).
   • record(click: true) sets the grant and the scope credit. Fork 4 spends the scope credit; the grant stays, but the fork does not need it. Thus fork 5 gets .gestureRequired, and fork 6 goes out after a second click.
   • The fork reply {"sessionId":"f1"} adds only f1 to the scope (AcpmuxPaneSessions.observe). noteSent adds a session only for an attach. Thus other stays outside the scope, and the reuse of other for forks 5 and 6 is correct.
   • Isolation: @MainActor on one test of a non-isolated suite is valid; the other tests of the suite are synchronous and pure. The fleet compile check (62e9b8d30408c6b8efd2bb74, CmuxNext test targets) passed. Each test makes its own server and transport. The gesture lifetime is 30 s on the real clock, and the test has a 1 min limit. I found no ordering race.
   • Note: the test is a coverage test of existing behavior (as the PR body says). It passes at the base too.

3. Not blocking. check.json, parity.rs and params.json are correct. check.json parses and has 55 entries with 55 unique names. The new case has the same shape as "fork from outside the scope uses scope credit" (same state, foreign_source: true, free: false, frame unchanged with live). parity.rs expects 55. params.json parses (119 cases). "acp.session.fork every known param" now has "live": true with breaks: false, which agrees with the Swift table (AcpmuxPaneMethods+Params.swift:113).

4. Not blocking. The message change and the "not localized" decision are correct. live is now read before hub.resolve; it is the same expression as before, so the local path does not change. The pane does not send a fork of a peer's chat: forkServed (messageMenu.ts:44-48) refuses summary.peer, and Fork Conversation (App.tsx forkRefusal) and the turn footer (canFork) use it. If a fork fails in another way, direct.ts fork() shows the daemon text inside the localized notice.forkFailedReason, as it does for all other acpmux RpcError text, which is English. The old text was English too. Thus this PR does not add a new English string that a person sees in the app. The cmux CLI can show the text, as it shows every other acpmux error.

5. Not blocking. The two corrected comments are now true, but the same file has more stale comments of the same kind. AgentPaneTransport+Check.swift:64-68 and parity.rs:112-114 are now correct (no Swift file reads check.json; I searched all *.swift). These comments, which the PR did not change, still say that Swift reads the shared cases:

   • parity.rs:219-220: "tests/cases/sources.json; Swift runs it against AcpmuxPaneSessions". No Swift file reads sources.json.
   • parity.rs:129 ("crate only: Swift does not read it yet") and parity.rs:147-151 ("Mirror the Swift parity harness ... in both hosts") tell of a Swift harness for check.json that does not exist.
     Fix them in the same fix push, or in a follow-up.

Observed / Not run

• Observed: I read the full diff of the 3 commits, the PR body and the re-review comment. I read AgentPaneTransport+Check.swift (checkOne, sessionRefusal), AgentPaneTransport+Frames.swift (decide, noteSent), AcpmuxPaneMethods.swift (sessionScoped, gestureRules, sourceScoped), AcpmuxPaneSessions.swift, AgentPaneUserGestures.swift, AcpmuxStandInServer.swift, PageInbox and AgentPaneRetryRelayTests.swift. In Rust I read server/mod.rs imports and session_key, fork_through.rs, key_route_tests.rs, hub/own_host.rs (resolve, add_peer), hub/peers.rs (peer, resolve_remote), hub/mod.rs (peers, remote_sessions, resolve_here), rpc.rs (RpcError), parity.rs. In webviews I read messageMenu.ts (forkServed, forkRefusal), direct.ts fork() and the App.tsx Fork Conversation path. I parsed check.json and params.json with Python. I read the fleet status: compile check 62e9b8d30408c6b8efd2bb74 done, clippy e9e044528a19633bb4b688a0 done with exit 0.
• Not run: cargo, swift and any build (this VM does not compile). I did not wait for the fleet Rust test jobs (0b2ce261bfa5e4154390f170, 35eb3b762b8d5a88b1fc9e98, da2f676b55a0956e1f228e8b, d7de6bad330a24d4e4ccbdb0) or the Swift suite job (1022b994772f65d5e135ea90); they were still running. Finding 1 comes from reading the code, not from a test result. I did not run prepush-gate.py.

[lawrencecchen]

Review (independent review subagent, Claude Opus 5.5): SHIP at d4bfd23
reviewer-session: be2ce14b-3250-4a75-867e-6fea03339fbb
author-session: fork-relay-rereview author (perf-lead subagent 7cace044-9478-4472-a3fc-7a0db6847bd9)
R9: pass
perf claim: none

Scope: the whole PR (git diff 5e5312c4b8e0..d4bfd23c6479), and the delta over my FIX review at dc4d15e (git diff dc4d15e9e1ab..d4bfd23c6479, one commit d4bfd23). I read the code read-only. I did not compile or run tests. My review at dc4d15e is #28512 (comment).

Findings

1. Closed (was blocking). The Rust unit test now tests the message.

   • hub_with_a_peer_chat now adds peer hb to hub.peers with the same call as hub/own_host.rs add_peer. Hub::peers is pub(super) in hub, so crate::server can read it. Peer::new is pub. The fixture runs inside the #[tokio::test], so a runtime is present.
   • With the peer present, Hub::resolve("hb/peer-chat-1") returns not_found early (own_host.rs:18-23). resolve_remote finds the row by its key and then finds the peer (peers.rs:1029). Thus to_session_fork returns remote_refusal(live). The new assert!(hub.resolve_remote(...).is_some()) makes a fixture error fail with a clear message.
   • Red: scratch 3cb9ea5 is ead6efb plus this fixture fix. git diff 3cb9ea55783f d4bfd23c6479 is only the message fix in fork_through.rs. Fleet job b27f3045d1a265a584d16074 (hq run 41123137562) failed at fork_through.rs:181: left "forking a chat on another computer through a turn is not supported yet", right "forking a chat on another computer is not supported yet". I read that log. The failure is on the message assertion only.
   • Green: job 25ae0ce9de68429cb575916b (hq run 41123137518) at d4bfd23: PASS. I read that log.

2. Closed (was not blocking). The stale parity.rs comments are correct now. The three comments (the old lines 129, 147-151 and 219) no longer say that a Swift harness reads check.json or sources.json. No *.swift file reads them (I searched at dc4d15e; the delta adds no Swift).

3. Not blocking. The red commit in the PR history is not the red evidence. ead6efb has the old fixture. It fails for the missing peer, not for the message. The valid red result is the scratch commit 3cb9ea5 on scratch/fork-relay-rereview-28332-red-never-merge. The PR body must name that scratch commit, its job and its failure as the red result, and the head job as the green result. A squash merge removes the history problem. Edit the PR body before you add ready-to-land. This does not change the code verdict.

4. No change from my last review. The Swift test aForkFromOutsideTheScopeGoesOutOnlyWithTheClick, the check.json case and count 55, params.json "live": true, and the "not localized" decision are correct, for the reasons in my last review. The delta does not touch them. Fleet job 1022b994772f65d5e135ea90 (AgentPaneForkRelayTests at dc4d15e) passed. The Swift files did not change after that commit.

Observed / Not run

• Observed: I read git diff dc4d15e9e1ab..d4bfd23c6479 and git diff 3cb9ea55783f d4bfd23c6479. I examined the fixture against hub/own_host.rs (resolve, add_peer), hub/peers.rs (peer, resolve_remote) and hub/mod.rs (peers). I read the fleet job states with cmux-ci status: compile check bdeb92e3767a6bba4fbcf996 done at d4bfd23; test 25ae0ce9de68429cb575916b done with exit 0 at d4bfd23; test b27f3045d1a265a584d16074 failed with exit 1 at 3cb9ea5; clippy 3f74be9526e58e7267ec49d4 done with exit 0 at d4bfd23; Swift suite 1022b994772f65d5e135ea90, full_check_order da2f676b55a0956e1f228e8b and params_rule d7de6bad330a24d4e4ccbdb0 done with exit 0 at dc4d15e. I read the logs of hq runs 41123137562 (the red failure text above) and 41123137518 (PASS).
• Not run: cargo, swift and any build (this VM does not compile). full_check_order and params_rule did not run at d4bfd23; the delta changes only comments in parity.rs. I did not run prepush-gate.py.

[lawrencecchen]

Check classification at head d4bfd23 (gh pr checks 28512: every check pass or skipping, except the two below).

• cmux-next swift test (job https://github.com/manaflow-ai/cmux/actions/runs/41123059589/job/124294913491): base finding.
  • This PR's own suite ran first in the changed-suite pass and passed: ✔ Suite AgentPaneForkRelayTests passed after 0.022 seconds (log line at 21:39:56Z).
  • The broad pass then had 24 red tests in unrelated suites (MainMenuPlacementTests, NativeTooltipGuardTests, SSHRecoveryShotsTests, SidebarHoverCardDwellTests, UpdateStagedYieldsToNewerTests, ...) and about 226 sidebar layout tests that never finished (swift test exit 143). This PR changes no Swift source outside one doc comment and one test file.
  • The same reds and the same stuck tests are on unrelated PRs at the same time: lane/orphan-composer-27544-carry (job 124306976066: NativeTooltipGuardTests, SSHRecoveryShotsTests, 228 stuck), lane/session-open-spare (job 124288542007: 24 red incl. MainMenuPlacementTests compactMenuCountsMatchAudit, exit 143), fix/context-menu-instant-dismiss (job 124286853001: 28 red incl. NativeTooltipGuardTests, SSHRecoveryShotsTests, 239 stuck). feat-cmux-next pushes skip this job, so there is no base-tip run of it.
• next-autoland / verdict ("its own red: cmux-next / cmux-next swift test"): wrapper of the check above.

No red of this PR's own. Fleet proof of the changed suites is in the PR body (Swift AgentPaneForkRelayTests job 1022b994772f65d5e135ea90 passed; Rust red/green jobs b27f3045d1a265a584d16074 / 25ae0ce9de68429cb575916b).