From c54fe29c8070df49e7c3743c2d747f8e6d65ef41 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:08:28 +0000 Subject: [PATCH 1/4] test(agent-pane): a fork of a chat outside the pane's scope needs the click, live or through a turn Re-review of #28332 (finding 1, 2): no Swift test proved the host's scope rule for acp.session.fork; check.json runs in Rust only. Adds a transport test in AgentPaneForkRelayTests (live and throughSeq, with and without the click), the matching live case in check.json (55 cases), live in params.json's every-known-param case, and corrects the two comments that said Swift runs check.json. Coverage of existing behavior, not a red-first bug fix. --- .../AgentPaneTransport+Check.swift | 6 +- .../AgentPaneForkRelayTests.swift | 58 +++++++++++++++++++ .../tests/cases/check.json | 41 +++++++++++++ .../tests/cases/params.json | 3 +- .../cmux-agent-pane-policy/tests/parity.rs | 7 ++- 5 files changed, 109 insertions(+), 6 deletions(-) diff --git a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneTransport+Check.swift b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneTransport+Check.swift index 344848abfca2..742b503a84db 100644 --- a/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneTransport+Check.swift +++ b/Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneTransport+Check.swift @@ -62,8 +62,10 @@ extension AgentPaneTransport { } /// What the rules that need no main-actor state make of one frame: refused (a ticket in it, if - /// any, is to be spent), or the checked frame and its facts. No socket and no request ids, so the - /// shared parity cases run it (cmux-agent-pane-policy `check_frame`, tests/cases/check.json). + /// any, is to be spent), or the checked frame and its facts. No socket and no request ids. The + /// Rust copy of these rules (cmux-agent-pane-policy `check_frame`) runs the cases in its + /// tests/cases/check.json; no Swift test reads that file, so a rule here needs its own Swift + /// test (the transport tests, for example `AgentPaneForkRelayTests`). nonisolated enum Checked: Sendable { case refuse(AcpmuxPaneMethods.Decision, spend: String?) case frame(Facts, FrameBox) diff --git a/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneForkRelayTests.swift b/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneForkRelayTests.swift index 2efee78420e7..dd657dadad32 100644 --- a/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneForkRelayTests.swift +++ b/Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/AgentPaneForkRelayTests.swift @@ -29,4 +29,62 @@ struct AgentPaneForkRelayTests { func aForkCarriesNothingElse(_ params: String) { #expect(AcpmuxPaneMethods.breaksParamsRule(Self.object(params), modeFields: []), "\(params)") } + + static func frame(id: Int, params: String) -> String { + #"{"jsonrpc":"2.0","id":\#(id),"method":"acp.session.fork","params":\#(params)}"# + } + + /// A fork of a chat outside the pane's scope waits for the click's scope credit, as an attach the + /// person makes (``AcpmuxPaneMethods/sourceScoped``, `AgentPaneTransport+Frames.swift` + /// `consumeScope`). `live` names no other source and needs the click too; one click brings one + /// fork. The pane's own chat forks with no click. The Rust copy of this rule is + /// cmux-agent-pane-policy tests/cases/check.json ("live fork from outside the scope uses scope + /// credit"); this test runs the rule in the host that serves the pane. + @MainActor + @Test(.timeLimit(.minutes(1))) func aForkFromOutsideTheScopeGoesOutOnlyWithTheClick() async throws { + let server = AcpmuxStandInServer() + try await server.start() + defer { server.stop() } + server.answer("acp.session.fork", with: #"{"sessionId":"f1"}"#) + let inbox = AgentPaneRelayedAllowTests.PageInbox() + let transport = AgentPaneTransport() + transport.deliver = { event, done in inbox.deliver(event.frames); done() } + let id = try await transport.open(AcpmuxConnection(url: server.url, dashboardToken: "t", localAppToken: nil)) + #expect(await transport.send(connection: id, frames: [#"{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}"#]) == nil) + transport.sessions.add("s") + let sent = { server.peers.first?.frames.filter { $0.contains("acp.session.fork") } ?? [] } + func params(_ frame: String) throws -> NSDictionary { + let object = try JSONSerialization.jsonObject(with: Data(frame.utf8)) as? [String: Any] + return try #require(object?["params"] as? [String: Any]) as NSDictionary + } + + // No click: a chat outside the scope, with `live` and through a turn. + #expect(await transport.send(connection: id, frames: [Self.frame(id: 1, params: #"{"sessionId":"other","live":true}"#)]) == .gestureRequired, + "a live fork of a chat outside the pane's scope, with no click") + #expect(await transport.send(connection: id, frames: [Self.frame(id: 2, params: #"{"sessionId":"other","throughSeq":3}"#)]) == .gestureRequired, + "a fork through a turn of a chat outside the pane's scope, with no click") + #expect(sent().isEmpty, "a refused fork reached acpmux: \(sent())") + + // The pane's own chat forks with no click. + #expect(await transport.send(connection: id, frames: [Self.frame(id: 3, params: #"{"sessionId":"s","live":true}"#)]) == nil, + "the relay refused a live fork of the pane's own chat") + _ = await inbox.frame(containing: #""id":3"#) + #expect(try sent().map(params) == [["sessionId": "s", "live": true] as NSDictionary]) + + // A click brings one fork of a chat outside the scope in. + transport.gestures.record(click: true) + #expect(await transport.send(connection: id, frames: [Self.frame(id: 4, params: #"{"sessionId":"other","live":true}"#)]) == nil, + "the relay refused a live fork the person clicked for") + _ = await inbox.frame(containing: #""id":4"#) + #expect(try sent().dropFirst().map(params) == [["sessionId": "other", "live": true] as NSDictionary]) + + // The click's credit is spent: the next fork outside the scope needs its own click. + #expect(await transport.send(connection: id, frames: [Self.frame(id: 5, params: #"{"sessionId":"other","throughSeq":3}"#)]) == .gestureRequired) + #expect(sent().count == 2, "\(sent())") + transport.gestures.record(click: true) + #expect(await transport.send(connection: id, frames: [Self.frame(id: 6, params: #"{"sessionId":"other","throughSeq":3}"#)]) == nil, + "the relay refused a fork through a turn the person clicked for") + _ = await inbox.frame(containing: #""id":6"#) + #expect(try sent().dropFirst(2).map(params) == [["sessionId": "other", "throughSeq": 3] as NSDictionary]) + } } diff --git a/cmux-tui/crates/cmux-agent-pane-policy/tests/cases/check.json b/cmux-tui/crates/cmux-agent-pane-policy/tests/cases/check.json index 6c0be8866fc1..f0d1f208c021 100644 --- a/cmux-tui/crates/cmux-agent-pane-policy/tests/cases/check.json +++ b/cmux-tui/crates/cmux-agent-pane-policy/tests/cases/check.json @@ -514,6 +514,47 @@ } } }, + { + "name": "live fork from outside the scope uses scope credit", + "text": "{\"jsonrpc\":\"2.0\",\"id\":13,\"method\":\"acp.session.fork\",\"params\":{\"sessionId\":\"s9\",\"live\":true}}", + "state": { + "first": false, + "token": null, + "mode_fields": null, + "sessions": [ + "s1" + ], + "denies": [] + }, + "expect": { + "frame": { + "jsonrpc": "2.0", + "id": 13, + "method": "acp.session.fork", + "params": { + "sessionId": "s9", + "live": true + } + }, + "facts": { + "is_first": false, + "method": "acp.session.fork", + "page_id": "13", + "ticket": null, + "other_meta": false, + "pick": null, + "session_id": null, + "needs_gesture": false, + "needs_path_check": false, + "setting": null, + "attach_session": null, + "foreign_source": true, + "handoff_id": null, + "harness_enable": false, + "free": false + } + } + }, { "name": "fork from the scope is free", "text": "{\"jsonrpc\":\"2.0\",\"id\":12,\"method\":\"acp.session.fork\",\"params\":{\"sessionId\":\"s1\"}}", diff --git a/cmux-tui/crates/cmux-agent-pane-policy/tests/cases/params.json b/cmux-tui/crates/cmux-agent-pane-policy/tests/cases/params.json index 34fb4b622f19..eb9319552109 100644 --- a/cmux-tui/crates/cmux-agent-pane-policy/tests/cases/params.json +++ b/cmux-tui/crates/cmux-agent-pane-policy/tests/cases/params.json @@ -870,7 +870,8 @@ "method": "acp.session.fork", "params": { "sessionId": 1, - "throughSeq": 1 + "throughSeq": 1, + "live": true } }, "mode_fields": null, diff --git a/cmux-tui/crates/cmux-agent-pane-policy/tests/parity.rs b/cmux-tui/crates/cmux-agent-pane-policy/tests/parity.rs index 40f0d9618ba4..e34d8ed42b13 100644 --- a/cmux-tui/crates/cmux-agent-pane-policy/tests/parity.rs +++ b/cmux-tui/crates/cmux-agent-pane-policy/tests/parity.rs @@ -109,12 +109,13 @@ fn facts_json(facts: &cmux_agent_pane_policy::Facts) -> Value { }) } -/// The full check in Swift's order (tests/cases/check.json; Swift runs the -/// same file against `AgentPaneTransport.checkOne`). +/// The full check in Swift's order (tests/cases/check.json). Only this crate +/// runs the file: no Swift test reads it, so `AgentPaneTransport.checkOne` has +/// its own Swift tests (for example `AgentPaneForkRelayTests`). #[test] fn full_check_order() { let all = cases("check.json"); - assert_eq!(all.as_array().unwrap().len(), 54, "check.json: the full order's 54 cases"); + assert_eq!(all.as_array().unwrap().len(), 55, "check.json: the full order's 55 cases"); for c in all.as_array().unwrap() { let s = &c["state"]; let modes: Option> = s["mode_fields"] From ead6efbd0712b3fb666df42e9a4b1395202c3af9 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:08:35 +0000 Subject: [PATCH 2/4] test(acpmux): a live fork of a peer's chat is refused without "through a turn" Re-review of #28332 (finding 3). Fails on the base: the live fork gets "forking a chat on another computer through a turn is not supported yet". --- .../crates/acpmux/src/server/fork_through.rs | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/cmux-tui/crates/acpmux/src/server/fork_through.rs b/cmux-tui/crates/acpmux/src/server/fork_through.rs index a5c72d58b21f..3218e1f7d1d1 100644 --- a/cmux-tui/crates/acpmux/src/server/fork_through.rs +++ b/cmux-tui/crates/acpmux/src/server/fork_through.rs @@ -134,3 +134,50 @@ async fn to_session_fork(hub: &Hub, params: &Value) -> Result { out.set("_meta", json!({"acpmux": mux})); Ok(out) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{Config, StoreMode}; + use crate::session_identity::SessionReceipt; + + /// A hub with no chat of its own, and peer hb listing a working chat. + fn hub_with_a_peer_chat() -> Arc { + let mut config: Config = serde_json::from_value(json!({})).unwrap(); + config.store.mode = StoreMode::Memory; + let store = + crate::store::open(&config.store, std::path::Path::new("/nonexistent")).unwrap(); + let hub = Hub::new(config, store); + let row = crate::hub::RemoteSession { + peer: "hb".into(), + summary: json!({"sessionId": "peer-chat-1", "name": "hb/chat", "status": "running"}), + receipt: SessionReceipt { + host_id: "hb".into(), + mux_session_id: "peer-chat-1".into(), + agent_session_id: None, + generation: 1, + }, + }; + hub.remote_sessions.lock().unwrap().insert("hb/peer-chat-1".into(), row); + hub + } + + /// Review of #28332: a `live` fork (Fork Conversation of a working chat) names no turn, so + /// its refusal for a peer's chat does not say "through a turn"; a fork through a turn does. + #[tokio::test] + async fn a_peer_chats_fork_is_refused_with_what_it_asked_for() { + let hub = hub_with_a_peer_chat(); + let live = to_session_fork(&hub, &json!({"sessionId": "hb/peer-chat-1", "live": true})) + .await + .unwrap_err(); + assert_eq!(live.message, "forking a chat on another computer is not supported yet"); + let through = + to_session_fork(&hub, &json!({"sessionId": "hb/peer-chat-1", "throughSeq": 3})) + .await + .unwrap_err(); + assert_eq!( + through.message, + "forking a chat on another computer through a turn is not supported yet" + ); + } +} From dc4d15e9e1ab91d6896d4729fb526c16fb7406f4 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:08:36 +0000 Subject: [PATCH 3/4] fix(acpmux): a live fork of a peer's chat says what it asked for A live fork (Fork Conversation) names no turn; its refusal for a chat on another computer no longer says "through a turn". Daemon error text in English, as acpmux's other errors: the pane never sends a fork of a peer's chat (messageMenu.ts forkServed), so no person-facing string changes. --- .../crates/acpmux/src/server/fork_through.rs | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/cmux-tui/crates/acpmux/src/server/fork_through.rs b/cmux-tui/crates/acpmux/src/server/fork_through.rs index 3218e1f7d1d1..a7531d1f3f6b 100644 --- a/cmux-tui/crates/acpmux/src/server/fork_through.rs +++ b/cmux-tui/crates/acpmux/src/server/fork_through.rs @@ -50,23 +50,33 @@ pub(super) fn through(params: &Value) -> Option { params.pointer("/_meta/acpmux/throughSeq").and_then(Value::as_u64) } +/// Why a chat on another computer (a peer's) cannot be forked here: a `live` +/// fork names no turn. Daemon error text, in English as acpmux's other +/// errors; the pane never sends a fork of a peer's chat (webviews +/// `messageMenu.ts` `forkServed`), so only a raw client reads it. +fn remote_refusal(live: bool) -> &'static str { + if live { + "forking a chat on another computer is not supported yet" + } else { + "forking a chat on another computer through a turn is not supported yet" + } +} + /// The `session/fork` params for an `acp.session.fork` request, or why the /// fork cannot happen at that turn. async fn to_session_fork(hub: &Hub, params: &Value) -> Result { let key = session_key(params)?; + // `live`: the whole chat as it is now (Fork Conversation), its running turn cut off + // there (`hub/fork.rs`). + let live = params.get("live").and_then(Value::as_bool).unwrap_or(false); let session = hub.resolve(key).map_err(|e| { if hub.resolve_remote(key).is_some() { - RpcError::invalid_params( - "forking a chat on another computer through a turn is not supported yet", - ) + RpcError::invalid_params(remote_refusal(live)) } else { e } })?; let working = matches!(session.meta().status, SessionStatus::Running | SessionStatus::Waiting); - // `live`: the whole chat as it is now (Fork Conversation), its running turn cut off - // there (`hub/fork.rs`). - let live = params.get("live").and_then(Value::as_bool).unwrap_or(false); let can_cut = { let harness = session.meta().harness; let cfg = hub.config.read().await; From d4bfd23c647967a4957387238c1f3d961fce7b83 Mon Sep 17 00:00:00 2001 From: lawrencecchen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:24:55 +0000 Subject: [PATCH 4/4] test(acpmux): the peer-fork message test lists the peer, so resolve_remote finds its chat Review of #28512 (finding 1): the fixture added the peer's row to remote_sessions but not the peer to hub.peers, so resolve_remote returned None and both the red and the green step failed on "no session matches" (jobs 35eb3b76, 0b2ce261). The test now asserts the fixture resolves first. Also corrects the parity.rs comments that named a Swift harness for check.json and sources.json (no Swift test reads them). --- cmux-tui/crates/acpmux/src/server/fork_through.rs | 10 +++++++++- .../crates/cmux-agent-pane-policy/tests/parity.rs | 14 +++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/cmux-tui/crates/acpmux/src/server/fork_through.rs b/cmux-tui/crates/acpmux/src/server/fork_through.rs index a7531d1f3f6b..5d6e3fcfd8de 100644 --- a/cmux-tui/crates/acpmux/src/server/fork_through.rs +++ b/cmux-tui/crates/acpmux/src/server/fork_through.rs @@ -151,7 +151,8 @@ mod tests { use crate::config::{Config, StoreMode}; use crate::session_identity::SessionReceipt; - /// A hub with no chat of its own, and peer hb listing a working chat. + /// A hub with no chat of its own, and peer hb (listed, never run) listing a working chat: + /// `Hub::resolve_remote` needs the peer as well as its row (`hub/own_host.rs` `add_peer`). fn hub_with_a_peer_chat() -> Arc { let mut config: Config = serde_json::from_value(json!({})).unwrap(); config.store.mode = StoreMode::Memory; @@ -169,6 +170,9 @@ mod tests { }, }; hub.remote_sessions.lock().unwrap().insert("hb/peer-chat-1".into(), row); + let (tx, _rx) = tokio::sync::mpsc::channel(8); + let peer = crate::peer::Peer::new("hb", "ws://127.0.0.1:9", None, None, tx); + hub.peers.lock().unwrap().insert("hb".to_owned(), peer); hub } @@ -177,6 +181,10 @@ mod tests { #[tokio::test] async fn a_peer_chats_fork_is_refused_with_what_it_asked_for() { let hub = hub_with_a_peer_chat(); + assert!( + hub.resolve_remote("hb/peer-chat-1").is_some(), + "the fixture lists the peer's chat" + ); let live = to_session_fork(&hub, &json!({"sessionId": "hb/peer-chat-1", "live": true})) .await .unwrap_err(); diff --git a/cmux-tui/crates/cmux-agent-pane-policy/tests/parity.rs b/cmux-tui/crates/cmux-agent-pane-policy/tests/parity.rs index e34d8ed42b13..a8ac70c72044 100644 --- a/cmux-tui/crates/cmux-agent-pane-policy/tests/parity.rs +++ b/cmux-tui/crates/cmux-agent-pane-policy/tests/parity.rs @@ -126,8 +126,8 @@ fn full_check_order() { scope.add(session.as_str().unwrap()); } let options = PermissionOptions::new(); - // A question (state `questions`, crate only: Swift does not read it - // yet) is one pending request with its denies and the question mark. + // A question (state `questions`) is one pending request with its + // denies and the question mark. let questions: BTreeSet<&str> = s["questions"] .as_array() .map(|a| a.iter().map(|q| q.as_str().unwrap()).collect()) @@ -146,9 +146,9 @@ fn full_check_order() { for d in s["denies"].as_array().unwrap().iter().filter(|d| d[0] == *q) { listed.push(serde_json::json!({"optionId": d[1], "kind": "reject_once"})); } - // The shared cases keep the question body out of `state`. Mirror the Swift - // parity harness by deriving the pending question's item ids from this frame's - // `answers` object, so answer cases exercise the same key allowlist in both hosts. + // The shared cases keep the question body out of `state`: the pending + // question's item ids come from this frame's `answers` object, so the + // answer cases exercise the key allowlist. let keys: Vec = c["text"] .as_str() .and_then(|text| serde_json::from_str::(text).ok()) @@ -216,8 +216,8 @@ fn debug_output_is_redacted() { assert!(!shown.contains(&token), "{shown}"); } -/// The pane's scope and the source rule (tests/cases/sources.json; Swift runs -/// it against `AcpmuxPaneSessions`). +/// The pane's scope and the source rule (tests/cases/sources.json). Only this +/// crate runs the file; no Swift test reads it. #[test] fn scope_and_sources() { for c in cases("sources.json").as_array().unwrap() {