acpmux: a peer reads connected only after its handshake, and a request queued on a dropped or stalled link fails as not sent

Merged
TE
8d ago
+408-35

Leo's ask (manaflow-ai/cmuxterm-hq#4770, plan row 2, "make the dot honest at the source"):

"you can just not choose a project ... you just get a little green dot ... as if you were working on your own computer, but now it's remote."

The green dot can only be honest if the daemon's peer state is. This PR fixes three peer-link defects in cmux-tui/crates/acpmux/src/peer.rs.

What changes

  1. Connected only after the handshake (HQ #2894). Before, serve set connected = true as soon as the WebSocket split. Now a new private open flag covers "socket open". connected turns true only after initialize and _acpmux/watch have both been answered and the session list is published, right before PeerNotice::Connected goes out.
    • The handshake's own two requests go out pinned to that link's epoch.
    • Callers' requests and notifications are still refused with peer_unreachable until then (the existing admitting gate stays).
    • run moves the epoch before it clears connected, and the handshake re-checks the epoch after it sets connected. So a link that drops mid-handshake never reads connected.
    • The base already had part of this: admitting (from 419b28c75853, #24105) refused callers' requests during the handshake. But connected, state in summary(), connected_peers(), peer_down_for and dispatch all still read connected on socket open. That is the part fixed here.
  2. Stale queued requests fail as not sent on reconnect (HQ #2892). Base already tagged each queued frame with its link epoch (419b28c75853, #24105), so a stale frame was dropped rather than replayed. But its caller got a bare peer disconnected, which reads as "may have run". Now:
    • serve records each request that actually reached the wire (on_wire).
    • When a link ends, fail_link_requests empties the queue. Every waiting request that never went out (still queued, or blocked waiting for room) fails with the existing peer_unreachable error, which says the request was not sent and is retryable.
    • Requests that did go out fail with peer disconnected, now carrying data.reason = "peer_disconnected", because the peer may have run them.
  3. Queue time counts toward the timeout (HQ #2900). Before, out.send waited with no deadline and the 600 s timer started after it. Now one deadline (ACPMUX_PEER_REQUEST_SECS, default 600 s) starts when the request is made and covers both getting into the queue and the reply.
    • A request that never gets into the queue fails with peer_unreachable, detail: "the request was not sent", plus a new queuedMs field.
    • Prompts and waits (waits_on_turn) still have no reply deadline, but their queue wait is now bounded.
    • Notifications get the same bounded queue wait.
  4. Ready clears the moment the WebSocket drops (#3980, daemon side). This was already true on base: run clears the link state as soon as serve returns. It now also clears open and connected before anything else runs. The agent-chat ready flag named in #3980 is a separate surface, fixed in its own PR (linked below).

State contract (for the UI state feed)

Peer rows in _acpmux/peers, _acpmux/status and peer_add/remove/reconnect (the same rows that acpmux host ls / acpmux peers print). No field is renamed or removed; one field is added.

fieldmeaning
connected (bool)Changed meaning, same name: true only once the handshake finished on the live link (initialize and watch answered, session list published). False from the moment the link drops, and false during the handshake. Green dot = connected.
handshaking (bool, new)The WebSocket is open and the handshake is running.
state (string)connecting (never reached yet) 路 connected (ready and answering) 路 degraded (handshaking, or a ping went unanswered) 路 reconnecting (down after being reached; last-known sessions stay listed) 路 offline (never reached, and attempts failed). Unchanged.
lastSeen, downSince, attempts, errorUnchanged.

Suggested mapping:

  • green: state == "connected";
  • spinner: connecting, reconnecting, or degraded;
  • red: offline, or error set while not connected.

Session rows from peers keep peerState (the same state string) and stale.

Request errors, in RpcError.data:

  • reason: "peer_unreachable", retryable: true, detail: "the request was not sent". The request never left this daemon, so it is safe to retry. It now also covers requests that were queued when the link dropped, and requests whose queue wait passed the deadline (these add queuedMs).
  • reason: "peer_disconnected" (new data; the message is still peer disconnected). The request went out and the link dropped before the reply. The peer may have run it, so do not blindly retry.

docs/acpmux-schema.json documents the _acpmux/peers fields.

Tests: fail before, pass after

New file src/peer_state_tests.rs:

  • a_peer_reads_connected_only_once_its_handshake_finished (#2894)
  • a_request_queued_when_its_link_drops_fails_as_not_sent (#2892)
  • a_drop_fails_what_went_out_as_disconnected_and_what_did_not_as_not_sent (#2892; it uses the new fail_link_requests, so it has no base form)
  • a_request_that_never_gets_into_the_queue_times_out_on_schedule (#2900)
  • the_time_a_request_waits_in_the_queue_counts_toward_its_deadline (#2900)

Before: origin/feat-cmux-next dca81ef07cc1 plus only the test file, through cmux-ci, run https://github.com/manaflow-ai/cmux/actions/runs/40647523404:

running 4 tests
a_request_queued_when_its_link_drops_fails_as_not_sent panicked: RpcError { code: -32603, message: "peer disconnected", data: None }
a_peer_reads_connected_only_once_its_handshake_finished panicked: connected before the handshake finished: {"connected":true,...,"state":"degraded"}
the_time_a_request_waits_in_the_queue_counts_toward_its_deadline panicked: queue time not counted
a_request_that_never_gets_into_the_queue_times_out_on_schedule panicked at peer_state_tests.rs:164 (a request on a stalled link must time out on its own deadline)
test result: FAILED. 0 passed; 4 failed

After: this branch, through cmux-ci, run https://github.com/manaflow-ai/cmux/actions/runs/40647519775:

cargo test -p acpmux --lib -- peer::   test result: ok. 12 passed; 0 failed
cargo clippy -p acpmux --all-targets -- -D warnings   clean

(rustfmt's three layout nits from that run are fixed in the second commit.)

Risk

Medium. Request routing to peers and the reported peer state both change.

  • Callers that read connected now see false for the length of the handshake, which is usually under a second. connected_peers(), peer_down_for, dispatch and the session summary's missing/unreachable therefore treat a handshaking peer as down. That matches the fact that its requests were already refused.
  • The overlap check named teamleaderleo's #26700 (adds one feature flag next to the handshake's flags; no shared logic) and lawrencecchen's #22030 (changes BUILD to build() two lines from the new handshaking field in summary(); textual neighbour only). Neither fixes these issues.

Related: the agent-chat half of #3980 is in the companion PR.

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

Makes a peer's reported state honest: connected is now true only after the handshake finishes, requests queued on a dropped link fail as not sent, and a request's queue wait counts toward its deadline.

Peer state

  • connected flips true only once initialize and _acpmux/watch are answered and the session list is published; a new handshaking field reports socket-open-during-handshake.
  • connected clears the moment the WebSocket drops, before reconnect work starts.
  • _acpmux/peers and _acpmux/status rows add handshaking; connected keeps its name but means "handshake finished on the live link".

Request failures

  • A request still queued when its link drops fails as peer_unreachable (retryable, "the request was not sent") instead of a bare peer disconnected.
  • Requests that reached the wire fail as peer_disconnected, now carrying reason: "peer_disconnected", because the peer may have run them.
  • One deadline (default 600 s) now covers both queue admission and the reply, so a stalled link times out on schedule instead of waiting for queue room indefinitely.