agent-pane: Swift test of the fork scope rule, live in the policy cases, live peer-fork message (#28332 re-review)

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)

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])
+    }
 }

cmux-tui/crates/acpmux/src/server/fork_through.rs

@@ -50,23 +50,33 @@ pub(super) fn through(params: &Value) -> Option<u64> {
     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<Value, RpcError> {
     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;
@@ -134,3 +144,58 @@ async fn to_session_fork(hub: &Hub, params: &Value) -> Result<Value, RpcError> {
     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 (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<Hub> {
+        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);
+        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
+    }
+
+    /// 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();
+        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();
+        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"
+        );
+    }
+}

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\"}}",

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,

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<BTreeSet<String>> = s["mode_fields"]
@@ -125,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())
@@ -145,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<String> = c["text"]
                 .as_str()
                 .and_then(|text| serde_json::from_str::<Value>(text).ok())
@@ -215,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() {