Repository navigation
fix(webviews): relay local daemon peer state events - #27971
teamleaderleo wants to merge 27 commits into
Conversation
On base 1f723c6, bun test src/agent-session/acpmux/peerStateEvents.test.ts runs 7 tests: 3 fail and 4 pass. The client never reads peers and ignores the local state feed. Nonlocal and feature-absent fallback cases pass. Native validation is held by #26809. Agent-Session: https://cmux.com/deeplink/agent?id=aid-93c3a66aa0c6bbfa650c
Gate peer events on the advertised feature and local/app origin. Read once after watch at initialize/reconnect, retain exact daemon state, clear nullable metadata, deduplicate lag replay, and reject superseded sockets/reads. Add a zero-parameter pane snapshot relay filtered to name, state, lastSeen and error. Preserve picker presentation and policies. Linux web validation: 14 focused tests / 62 assertions pass; bun --bun run check passes including formatting, lint, typecheck (2933 files), and protocol checks. The focused regression was red before repair (3 failed / 4 passed). Native relay tests were added but native build/test/parse and CI dispatch remain held by #26809/HQ4779. No real-machine capture is claimed. Agent-Session: https://cmux.com/deeplink/agent?id=aid-93c3a66aa0c6bbfa650c
The replacement socket must initialize its own local/app origin and peerStateEvents feature before receiving peer notes; invalidate prior read tokens at open start. Focused Linux web tests: 15 pass, 66 assertions. Full bun --bun run check passes. Native verification and CI remain held by #26809/HQ4779. Agent-Session: https://cmux.com/deeplink/agent?id=aid-93c3a66aa0c6bbfa650c
The daemon drops peer_state_changed notices when a client lags and says so with _acpmux/lagged; resyncing only the selected chat left a machine that went down meanwhile shown as connected. Review P2 on #26885. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-710a4a518c009a6ad126
…pshot _acpmux/peers says `restarting` (peer_link.rs PeerState::as_str) while a peer's daemon restarts. The adapter drops that row, so the authoritative snapshot clears the peer's state and the picker reads it as ready. Red: bun test src/agent-session/acpmux/peerStateReducer.test.ts src/agent-session/acpmux/peerStateEvents.test.ts: 16 pass, 2 fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c35ebd1681cf79649972
_acpmux/peers names every PeerState acpmux has, `restarting` included (peer_link.rs as_str), while peer_state_changed maps it to reconnecting. The adapter now accepts it, so a restarting peer keeps its state and reads not connected instead of falling back to ready in the picker. Green: bun test src/agent-session/acpmux/peerStateReducer.test.ts src/agent-session/acpmux/peerStateEvents.test.ts: 18 pass, 0 fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c35ebd1681cf79649972
…ng as ready A peer that _acpmux/status lists but the authoritative _acpmux/peers snapshot omits (removed between the two reads, or an invalid row) loses its state and `connected`, so environmentReady offers it as ready. Red: bun test src/agent-session/acpmux/peerStateEvents.test.ts: 10 pass, 1 fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c35ebd1681cf79649972
An authoritative _acpmux/peers snapshot replaced the peer maps with its own rows, so a peer the status read still lists lost its state and `connected` and the picker offered it as ready. Such a peer now reads not connected until a note or the next read says otherwise. Green: bun test src/agent-session/acpmux/peerStateEvents.test.ts: 11 pass, 0 fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c35ebd1681cf79649972
|
Review (gpt-6.1-sol): SHIP at 1ffadb3 No verified correctness defects found. The change accepts all daemon snapshot states, keeps Focused Bun tests pass: 19 tests, 76 assertions. Fleet validation was unavailable because the broker socket was inaccessible. |
|
Found 6 test failures on Blacksmith runners: Failures
|
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Passes: CI passes on CI passes on Written by web and size checks: green Default changesThis pull request changes 1 default at
Signed off on this commit by Regenerate the snapshot with Gallery before/after for
States
Pick other states with a |
…r-events-restarting-1030
|
Review (gpt-6.1-sol; carried): SHIP at e6bfcd7 Carried from the SHIP at 1ffadb3: e6bfcd7 is a clean merge of feat-cmux-next (e863077, past 4154db0) with no conflicts, and the PR's diff against base is unchanged (identical except line offsets and blob ids). The merge clears the inherited base reds in cmux-next checks (Package dependency lists sorted, Prebuilt web bundles install and stamp, swift-test-summary script test). All three fail at the old base 9129016 and pass at e863077 and at this head. The Sidebar look owner sweep names three base commits (4367c16, 6c1a5ab, eb8105b) and fails the same way at base. |
…r-events-restarting-1030
…r-events-restarting-1030
Bugbot needs on-demand usage enabledBugbot 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. |
When the authoritative peer snapshot omits a peer the status read lists,
its peerStatus becomes only { connected: false }, so the offline banner
(chatUnavailable) loses lastSeen and attempts.
Red: bun test src/agent-session/acpmux/peerStateEvents.test.ts: 10 pass, 1 fail.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-c35ebd1681cf79649972
The authoritative-snapshot branch replaced an omitted peer's status with
{ connected: false } alone. It now keeps lastSeen, attempts and error
from the last read and drops only the stale state, so the offline banner
still says when the machine was last seen.
Green: bun test src/agent-session/acpmux/peerStateEvents.test.ts: 11 pass, 0 fail.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-c35ebd1681cf79649972
readPeers writes each _acpmux/status reply into peers, peerConnected, peerStates and peerStatus before the clock fence runs, so of two refreshPeers calls the one asked first and answered last wins. The fixture's status read now answers with what the daemon knew when the read reached it. Red: bun test ./src/agent-session/acpmux/peerStateEvents.test.ts: 15 pass, 1 fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c35ebd1681cf79649972
statusRead now says whether the read is newer than every status read that already landed (and than this connection); readPeers drops one that is not before it writes peers, peerConnected, peerStates or peerStatus, so two refreshPeers calls answered out of order keep the newer answer. Green: peerStateEvents 16, peerStateReducer 8, chat-unavailable 7, exportClient 3, chatOpenTrips 3, agent-pane-peer-copy 55, agent-pane-cached-chat-open 10 pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Agent-Session: https://cmux.com/deeplink/agent?id=aid-c35ebd1681cf79649972
…r-events-restarting-1030
|
Review (gpt-6.1-sol): SHIP at e18a9ca No findings. Confirmed stale status replies are rejected before map writes, connect’s own read remains valid, reconnect rejects prior reads, and newer snapshots survive racing status replies. Relay changes retain parameter denial and reply filtering. Static review only; tests were not run. Diff given to the reviewer: |
Bugbot needs on-demand usage enabledBugbot 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. |
…r-events-restarting-1030 # Conflicts: # cmux-tui/crates/cmux-agent-pane-policy/tests/cases/params.json
|
Review (gpt-6.1-sol): SHIP at 6c92cc2 No verified defects found. The merge resolution preserves
Diff given to the reviewer: |
|
Carried by #29494 (this lane is gone): same commits and authorship, merged with current feat-cmux-next and its conflicts resolved. This one can close in favour of it. |
|
Awareness note from an agent lane that does not own this PR; it is not a review or a request to take the PR over. Superseded by merged PR #29494, which contains the local daemon peer state events fix and follow-up corrections. |
|
Closing as superseded by #29494. |
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Takes over #26885 (its lane is gone) with its commits and authorship, rebased onto current
feat-cmux-next, and fixes the review finding that blocked it.Local daemon peer transitions do not reach the web session's peer state today: the picker only sees the last
_acpmux/statusread. This relays them._acpmux/peersrelay in both host policies: the macOS host (AcpmuxPaneMethods) and the shared Rust policy (cmux-agent-pane-policy/policy.json), so a Rust host doesn't refuse the request. The reply is filtered toname,state,lastSeenanderror, never URLs or credentials. The RustReplyShapegainsNumber(a finite number, never a boolean, matching Swift's.number) solastSeensurvives the filter._acpmux/watchanswers, on each reconnect and after an_acpmux/laggednotice, then applies_acpmux/peer_state_changednotes from the same connection. It never polls. The snapshot isn't on the first frame's path: it lands and draws on its own, so the first frame keeps cmux-next: a chat's first frame waits on one daemon round trip #27755's one round trip._acpmux/statusreads are ordered on one clock (the tick each read started at). An older read of either kind can't overwrite a newer note or a newer read, and a status read replaces any entry written before it started.peerStateEventsfrom alocalorigin. Peer, remote and unknown origins and older daemons keep the existing snapshot behavior.The review finding, fixed here
The earlier review blocked #26885 at
fbb1240ebecausepeerStateEvents.tsaccepted only the fivePeerLinkStatevalues._acpmux/peerssummaries usePeerState::as_str(cmux-tui/crates/acpmux/src/peer_link.rs), which also sendsrestartingwhile a peer's daemon restarts (peer_state_changedmaps that toreconnecting). The adapter dropped that row, so the authoritative snapshot cleared the peer's state andconnected, andenvironmentReady(state undefined,connected !== false) treated the peer as ready. The adapter now accepts every state the daemon sends. A restarting peer keepsstate: "restarting"andconnected: false, so it is listed but not offered as ready, the same as the_acpmux/statuspath already does on base.Later reviews of this branch found more:
_acpmux/statuslists but the authoritative snapshot omits (removed between the two reads, or a row the adapter could not read) also lost its state andconnected.applyPeerStatesnow marks such a peer not connected, keeping thelastSeenandattemptsthe offline banner shows.restartingnever cleared if the restart then failed. Restarting and reconnecting are bothreconnectingon the wire, so the daemon sends no note, and the snapshot kept overriding newer status reads. A status read now replaces it (the one-clock fence above), andchatUnavailablecountsrestartingas still connecting._acpmux/peers(above).frames.jsonhad sampled it as a method the pane never sends; that refusal sample moved to_acpmux/peer_remove, which stays unlisted.Checks on the original
defaults guard: the "default" is the
replyShapestable hash (AcpmuxPaneMethods+ReplyShapes.swift, the new_acpmux/peersreply filter). It is a relay reply allowlist, not a setting default, and it changes no user-facing default. The guard clears with an independent review receipt at the head.check-fixed-waits (web lint): red on base. It flags
chatOpenTrips.test.ts, added by cmux-next: a chat's first frame waits on one daemon round trip #27755, which this PR doesn't touch, so I pushed with aCMUX_WEB_LINT_SKIP_REASONnaming the base sha.tests/swift.rs
the_known_params_are_the_macos_hosts: red on base (acp.session.fork'sliveparam is missing frompolicy.json); this PR's_acpmux/peersentries match the Swift tables.No capture: no new or changed UI. This keeps the existing picker rows' existing state labels current from daemon events, and a real peer transition can't be staged in a capture scene.
Relay analysis
_acpmux/peerstakes no parameters (any parameter is refused,AcpmuxPanePeerSnapshotTests) and runs no command. It returns only the local daemon's peer names and link state, which_acpmux/statusalready relays. Only thelocalorigin may consume it, so a peer daemon's connection never sees this host's peer inventory (#26860).Testing
Each fix has its red test committed first:
test(webviews): reproduce a restarting peer dropped from the peer snapshot:cd webviews && bun test src/agent-session/acpmux/peerStateReducer.test.ts src/agent-session/acpmux/peerStateEvents.test.tsgives 16 pass, 2 fail (the reducer drops the restarting row; the client never reportsrestarting, so the picker reads the peer as ready). After the fix: 18 pass.test(webviews): reproduce a listed peer the peer snapshot omits reading as ready:bun test src/agent-session/acpmux/peerStateEvents.test.tsgives 10 pass, 1 fail (peerConnectedisundefined). After the fix: 11 pass.The restarting-state, first-frame and chatUnavailable reds:
bun test src/agent-session/acpmux/peerStateEvents.test.ts11 pass, 2 fail, andbun test test/chat-unavailable.test.ts1 fail; then green.The out-of-order reads:
bun test src/agent-session/acpmux/peerStateEvents.test.ts13 pass, 2 fail; then green.The Rust policy:
cargo test -p cmux-agent-pane-policyon CI (cmux-ci). Parityrepliesfailed at the red commit; at the head, parity passes 16 of 16.At the head, the peer-state, reducer, environments and chatUnavailable suites give 30 pass, 0 fail.
Observed / Not run
scripts/web-lint.sh(the type-aware lint and type check are left to CI on this host);scripts/cmux-next/prepush-gate.py, no new violations.AcpmuxPanePeerSnapshotTests(left to CI's Swift lanes); a real remote-machine transition. A run of the wholesrc/agent-session/acpmuxfolder in one Bun process timed out on CI, so the PR's own web test jobs cover the rest.Changelog
Agent session: agent session
🤖 Generated with Claude Code