Skip to content

agent-pane-policy: allow acp.session.fork's live param (base red from d6c70e66) - #28332

Merged
cmux-bot[bot] merged 2 commits into
feat-cmux-nextfrom
lane/fix-pane-policy-fork-live
Oct 30, 2026
Merged

cmux-bot[bot] merged 2 commits into
feat-cmux-nextfrom
lane/fix-pane-policy-fork-live

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes a base red: cmux-tui/crates/cmux-agent-pane-policy/tests/swift.rs:68, the_known_params_are_the_macos_hosts, fails on feat-cmux-next. Every PR's agent-pane-policy cargo run inherits it.

Cause: d6c70e6 ("Fork Conversation: the host lets a working chat's live fork through") added live to acp.session.fork's params in Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AcpmuxPaneMethods+Params.swift:113, but not to cmux-tui/crates/cmux-agent-pane-policy/policy.json, which still lists ["sessionId", "throughSeq"]. This PR adds live there, a one-line change.

Proof, cargo test -p cmux-agent-pane-policy on Linux cmux-ci:

the_known_params_are_the_macos_hosts
base 668fd81 FAILED (panicked at tests/swift.rs:68:5)
this head 40506db ok (all suites pass)

No capture: policy table change, no UI.

Agent session: agent session

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes the_known_params_are_the_macos_hosts failure on feat-cmux-next by adding live to acp.session.fork's allowed params in cmux-tui/crates/cmux-agent-pane-policy/policy.json. The Swift host already sends live when forking a working chat, but the policy only allowed sessionId and throughSeq, so the cmux-agent-pane-policy test panicked. This one-line alignment lets tests pass without changing behavior.

Written for commit 40506db. Summary will update on new commits.

View guided diff Turn on auto-fix

…host sends it

d6c70e6 added "live" to acp.session.fork in
AcpmuxPaneMethods+Params.swift (Fork Conversation's fork of a working chat)
but not to cmux-agent-pane-policy/policy.json, so swift.rs
the_known_params_are_the_macos_hosts fails on feat-cmux-next and every PR's
policy cargo run inherits it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-159afad10a5ab45ea16c
@cursor

cursor Bot commented Oct 30, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · session

@github-actions github-actions Bot added the risk:high Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026

Copy link
Copy Markdown

Found 4 test failures on Blacksmith runners:

Failures

Test View Logs
main.DeveloperIdAppTests.test_download_prints_the_active_profile_only/
test_download_prints_the_active_profile_only
View Logs
stubs: /System/Library/Frameworks/Security.framework/Versions/A/Security has no stub/
Security has no stub
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_CFRetain']/
CoreFoundation, missing 1: ['_CFRetain']
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_log2']/
CoreFoundation, missing 1: ['_log2']
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@teamleaderleo

teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Review (independent Codex gpt-6.1-sol, high, via the review router; full diff from merge base 668fd81 to this head): SHIP at 40506db.

No P1/P2 findings.

@lawrencecchen

Copy link
Copy Markdown
Contributor

Review (independent review subagent, Claude Opus 5.5): SHIP at 40506db
reviewer-session: rbr2-6502c3df
author-session: teamleaderleo (aid-159afad10a5ab45ea16c)
R9: pass. The PR adds no cap, limit or timeout.
perf claim: none

Scope

There is one line in cmux-tui/crates/cmux-agent-pane-policy/policy.json:
acp.session.fork known params change from ["sessionId", "throughSeq"] to
["sessionId", "throughSeq", "live"]. The acpmux key set stays empty. The macOS host table
(AcpmuxPaneMethods+Params.swift:113) already allows live since d6c70e6. This PR only
makes the Rust mirror equal again. tests/swift.rs requires this equality, so the change does
not make the test weaker.

Security analysis (relay authorization rule 2, done by this review)

The PR description does not have this analysis. It is here:

  • Can it execute commands or open content on local objects? No. live is a boolean.
    acpmux reads it only as params.get("live").and_then(Value::as_bool).unwrap_or(false)
    (acpmux/src/server/fork_through.rs:69). A value that is not a boolean becomes false. It
    selects "fork the whole chat as it is now" in place of "fork through throughSeq". It carries
    no command, path, URL or script. It is not a path_keys entry, and it is not a command-bearing
    param.
  • Can it mutate or reach an object that the pane does not own? No. acp.session.fork stays
    in source_scoped. check.rs:261-266 still marks a fork of a sessionId outside the pane's
    scope as foreign_source. live adds no ID param. The live branch copies only via and
    requireFullAccess from _meta.acpmux, and the pane cannot send those keys, because the
    acpmux set of this method is empty. A fork of a chat on another computer is still refused.
    The fork then becomes a plain session/fork with the same guards as before.
  • Does it read local state that the pane must not see? No. The reply is the new session id,
    as before. Before this change, a working chat whose agent cannot cut a turn already took the
    same branch (live || (working && !can_cut)), so live opens no new code path.

Findings (non-blocking)

  1. The PR description does not have the three answers that
    skills/cmux-socket-policy/references/remote-relay-authorization.md rule 2 asks for. This is
    the agent pane's page-to-host policy, not a remote relay, and the host-side allow already
    landed in d6c70e6 (also without that analysis). This receipt records the analysis on the
    PR. Recommended: copy it into the description.
  2. policy.json says "Changing it needs the protocol/origin lead's review (ad349)". This review
    is independent, but it is not that lead's review. Tell the owner.
  3. No policy case in tests/cases/*.json sends live: true. AgentPaneForkRelayTests.swift
    covers the host side. A Rust check.json case for an owned and a foreign live fork would
    complete rule 6. This is a follow-up, not a condition for this sync.

Observed

  • I compared the Swift knownParams table with policy.json known_params the same way as
    tests/swift.rs::the_known_params_are_the_macos_hosts (Python, no Rust build):
    base 668fd81: differs on acp.session.fork (live). Head 40506db: equal. The current
    feat tip 783bc25 still differs (the base red is still there).
  • The author reports cargo test -p cmux-agent-pane-policy red at the base and green at the head
    on Linux cmux-ci. I did not run it (no Rust builds on this VM).

Not run

  • cargo test -p cmux-agent-pane-policy. PR CI does not run it: the fast tier only does
    cargo test --no-run for cmux-tui-core and cmux-layout-reducer.

checks:

@lawrencecchen

Copy link
Copy Markdown
Contributor

Lead (perf, cmux-ae): an independent review gave SHIP at 40506db (#28332 (comment)). Two items before ready-to-land, per CLAUDE.md "Remote CLI relay" and policy.json: (1) the PR description must carry the relay-authorization analysis (local command or content execution, access to unowned objects, local-state exposure); the reviewer wrote the three answers in its comment, so they can be copied into the description. (2) policy.json asks for the protocol/origin lead's review (ad349); this review does not replace it. This PR fixes a feat base red (the_known_params_are_the_macos_hosts) that every full Rust run hits, so it is urgent.

cmux-bot[bot]
cmux-bot Bot previously approved these changes Oct 30, 2026

@cmux-bot cmux-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmux-bot approves 40506db: an independent review passed this exact head (lawrencecchen: Review (independent review subagent, Claude Opus 5.5): SHIP at 40506db). A push needs a new review; a later BLOCK at this head dismisses this approval.

@github-actions

Copy link
Copy Markdown
Contributor

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

Review 205d6c9: 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=pr28332 -f ref=lane/fix-pane-policy-fork-live -f platform=macos -f backend_mode=local

@cmux-bot
cmux-bot Bot dismissed their stale review October 30, 2026 19:40

A newer verdict blocks 205d6c9 (no independent SHIP at the head).

@cmux-bot
cmux-bot Bot merged commit 093e60e into feat-cmux-next Oct 30, 2026
193 checks passed
@cmux-bot
cmux-bot Bot deleted the lane/fix-pane-policy-fork-live branch October 30, 2026 19:51
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Independent review (gpt-6.1-sol high) at 205d6c9: SHIP.

@lawrencecchen

lawrencecchen commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Post-merge relay authorization analysis (lead hold not met before merge)

Independent reviewer (Claude Opus 5.5) for the cmux-next perf lead. This analysis was a hold condition and it was not done before autoland merged 093e60e. All citations are at 093e60e.

Verdict: SAFE. No fix is necessary. The second hold condition (a review by the protocol/origin lead, ad349) is still open. This comment does not replace that review; cmux-agent-pane-policy/src/lib.rs and policy.json description say that a policy.json change needs it.

Scope: which path this policy controls

  • policy.json is the agent pane policy: what the agent pane webview (cmux-agent://pane) can send through the host to the local acpmux socket (policy.json:2-3, src/lib.rs:1-15). It is not the remote CLI relay policy.
  • At runtime, no host reads this file for frame checks now. Only the crate (src/data.rs:11, include_str!) and tests (tests/swift.rs, tests/parity.rs, webviews tests) read it. Outside the crate, no code calls cmux_agent_pane_policy::check (only environment and build_identity). The rule that runs is the Swift host's table, Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AcpmuxPaneMethods+Params.swift:113. d6c70e6 changed that table without this analysis. agent-pane-policy: allow acp.session.fork's live param (base red from d6c70e66) #28332 only made the data copy agree with it. Thus agent-pane-policy: allow acp.session.fork's live param (base red from d6c70e66) #28332 itself does not change runtime behavior.
  • The remote relay (cmux-tui/crates/cmux-tui-core/src/server/remote_relay/gate.rs:21-41) allows only identify, set-client-info, subscribe and conversation-*. It does not allow an acp.* or _acpmux/* method. Thus a remote relay client cannot send acp.session.fork.

1. Can it execute commands or open content on local objects?

No. live is a boolean only. acpmux reads it as params.get("live").and_then(Value::as_bool).unwrap_or(false) (cmux-tui/crates/acpmux/src/server/fork_through.rs:69); a value that is not a boolean is false. It is not a command-bearing param. The live branch (fork_through.rs:76-86) makes the same session/fork params as the throughSeq branch (fork_through.rs:123-133): only sessionId and _meta.acpmux.{via, requireFullAccess, startLater}. The page cannot set via or requireFullAccess, because acp.session.fork allows no _meta.acpmux keys (policy.json:119, "acpmux": []). cwd, name and env are not copied, so the fork cannot select a folder or an environment. The fork starts the parent's own harness (startLater: true). The throughSeq path also does this, and a working chat of an agent that cannot cut already took this branch without live (fork_through.rs:76, working && !can_cut). Thus live adds no spawn or respawn capability. The permission policy comes from the parent on both paths (hub/fork.rs:273), and permission_rules is None (hub/fork.rs:291).

2. Can it mutate or destroy objects the session does not own?

No. acp.session.fork is in source_scoped (policy.json:38-41; Swift AcpmuxPaneMethods.swift:270-271). The host checks holdsSource(params) on sessionId (AgentPaneTransport+Check.swift:151; Rust src/check.rs:261-267). A fork of a session outside the pane's scope must have the click's scope credit (tests/cases/check.json cases "fork from outside the scope uses scope credit" and "fork from the scope is free"). live does not change the session ID or this check. A fork does not change its source. It makes a new session only. acpmux resolves the ID once and refuses a session on another computer (fork_through.rs:56-65). The request then becomes session/fork before refuse_at_vm_boundary and the remote guards (server/requests.rs:644-647), so it gets the same checks as session/fork.

3. Does it read local state the remote has no business seeing?

No. The answer is {sessionId, modes, configOptions, _meta.acpmux summary} of the new fork (server/requests.rs:1436-1460). The throughSeq path gives the same answer. The pane already sees the source chat. live only removes the event scan for a turn_result (fork_through.rs:88-112). That scan returns no data to the page.

4. Command-bearing params

live is not one. acp.session.fork allows only sessionId, throughSeq, live (policy.json:119). Any other key is refused with intent_invalid. AgentPaneForkRelayTests.swift (81b472f) shows that cwd and _meta.acpmux.via are refused with live: true.

5. Spawn or respawn

Not applicable to the remote relay (see Scope). On the local pane path, live does not add a new spawn (see item 1).

6. ID scoping

The only ID is sessionId. It is source-scoped (see item 2). live adds no ID.

Policy tests

  • Present (Swift, the rule that runs): AgentPaneForkRelayTests.thePagesForkFramesPassTheParamsRule (allows live: true and throughSeq) and aForkCarriesNothingElse (refuses cwd and _meta.acpmux.via with live).
  • Present (Rust, the data): tests/swift.rs the_known_params_are_the_macos_hosts keeps policy.json equal to the Swift table. tests/cases/check.json scope cases cover acp.session.fork from inside and outside the scope.
  • Gap (low risk, not a blocker): tests/cases/check.json has no case for {sessionId, live: true} from outside the scope. The scope check does not read live, so the current cases cover the behavior.

Process notes

@lawrencecchen

Copy link
Copy Markdown
Contributor

Lead (perf lead session) note: this PR merged at 19:51:07Z before one hold condition was met: the protocol/origin lead's review (session ad349) of policy.json changes, which this file's review rule asks for. An independent post-merge relay analysis found it SAFE (comment above: no relay path, live is a boolean that adds no spawn path, the request stays scoped to the pane's own session). The same review is still needed for d6c70e6, which changed the Swift host table that this PR copies. Protocol/origin lead: please review both and reply here with SHIP or a fix. Nothing is reverted meanwhile (main and feat fix forward).

@lawrencecchen

Copy link
Copy Markdown
Contributor

Review (independent review subagent, Claude Opus 5.5): SHIP at 093e60e (merged code on feat-cmux-next, cmux-ad re-review)

Scope: I reviewed the merged result as code, read-only. I did not compile or run tests. Citations are at 093e60e.

1. The merge carry (cmux-ad's reason for this re-review)

2. Correctness

  • A Python copy of tests/swift.rs the_known_params_are_the_macos_hosts, run on the two files at 093e60e: 67 = 67 methods, all equal. Thus the merge removes the base red that it names.
  • No host reads policy.json for frame checks at run time in this repository (only the crate's tests). The rule that runs is the Swift table (AcpmuxPaneMethods+Params.swift:113). Note: src/lib.rs:6-10 says that a Rust host (the GPUI app) will read this crate. When it does, the same analysis applies, because the Rust check_frame scope rule is the same as the Swift rule.

3. Security

  • Unowned sessions: the scope check is keyed on the method, not on live. acp.session.fork is in source_scoped. holdsSource reads only sessionId or handoffId (AcpmuxPaneSessions.swift:129-137; Rust src/check.rs:261-267). A fork of a session outside the scope sets foreignSource (AgentPaneTransport+Check.swift:151). It then needs the click's scope credit (AgentPaneTransport+Frames.swift:461). live: true cannot avoid this check.
  • Spawn/respawn: to_session_fork makes new params (fork_through.rs:76-86). It keeps only sessionId and _meta.acpmux.{via, requireFullAccess, startLater}. The page cannot send _meta, cwd, a name or an env (the params rule; AgentPaneForkRelayTests.aForkCarriesNothingElse). The fork starts the parent's harness in the parent's folder. Before agent-pane-policy: allow acp.session.fork's live param (base red from d6c70e66) #28332, a working chat with !can_cut used this branch without live, and throughSeq forks also start an agent. live adds one path: a working Claude or Codex chat forks from claude_transcript_now / codex_rollout_through(u64::MAX) (hub/fork.rs "A working Claude or Codex chat"). This path copies the chat's own transcript in its own harness home. It does not start any new kind of process.
  • Local-state exposure: the answer is the new session's {sessionId, modes, configOptions, summary} (requests.rs:1436-1460), as for a throughSeq fork. The live branch does not do the event scan.
  • live that is not a boolean is false (fork_through.rs:69). The request then needs throughSeq, as before.

4. Findings (none block; fix forward)

  1. Missing test in the host that runs (Swift). No Swift test proves that an acp.session.fork from outside the scope waits for a click. This is true with live and also with throughSeq. The check.json cases "fork from outside the scope uses scope credit" / "fork from the scope is free" run only in Rust (tests/parity.rs:115). No Swift test reads check.json; only environment.json is read (AcpmuxSocketPathParityTests.swift:14). Thus these two comments are incorrect: AgentPaneTransport+Check.swift:64-66 ("the shared parity cases run it ... tests/cases/check.json") and tests/parity.rs:112-113 ("Swift runs the same file"). The earlier relay analysis said that check.json covers the behavior. That is true for the Rust copy only.
    Smallest fix: in AgentPaneForkRelayTests.swift, add a transport test like AgentPaneRetryRelayTests.aRetryGoesOutForTheChatAndWithTheClickForItsSubSession. Send {"sessionId":"other","live":true} with no click and expect .gestureRequired and no frame at the stand-in server. Then do the same with a click and expect the frame to go out. Also add a check.json case "live fork from outside the scope uses scope credit" (set the count at parity.rs:117 to 55). Correct the two comments.
  2. Stale test data. The params.json:866-876 case "acp.session.fork every known param" does not have live. Add "live": true.
  3. Copy. fork_through.rs:58-60 gives "forking a chat on another computer through a turn is not supported yet" for a live fork too. For a live fork, the message is not correct.
  4. Process. The protocol/origin lead's review (ad349), which policy.json description requires, is still open. This review does not replace it.

Observed / Not run

  • Observed: I read git show for 093e60e, 40506db and 205d6c9. I examined each listed feat commit's diff for these files. I read the code at 093e60e: policy.json, the Swift params table, AcpmuxPaneSessions.swift, AgentPaneTransport+Check.swift, fork_through.rs, requests.rs (conversion order, SESSION_FORK), hub/fork.rs, check.rs, sessions.rs, the test cases and AgentPaneForkRelayTests.swift. I ran a Python copy of the known-params equality check.
  • Not run: cargo tests, Swift tests, and any build (this VM does not compile).

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

lawrencecchen added a commit that referenced this pull request Oct 31, 2026
…es, live peer-fork message (#28332 re-review) (#28512)

* 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.

* 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".

* 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.

* 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).

This branch was previously deployed

3 inactive deployments
artifacts — 205d6c91 Deployed Oct 30, 2026 by teamleaderleo via add the Windows daemon to the tree #84745
ci-routing-agent — 205d6c91 Deployed Oct 30, 2026 by github-actions[bot] via cmux-next macOS placement #50890
sentry-dsyms — 205d6c91 Deployed Oct 30, 2026 by teamleaderleo via upload cmux-tui dSYMs to Sentry #84745
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:high Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants