acpmux: a refused Stop is never dropped and answers only its own turn

Merged
TE
7d ago
+258-25

Follow-up to #28160 (merged as f87c3731d29), fixing the two P2s from its review.

Takes back #28160: on purpose. This is #28160's own follow-up from the same lane. It rewrites #28160's _acpmux/cancel_refused entry in docs/acpmux-schema.json: it adds turnId and the no-drop note, and corrects the message note, which still said the message names the machine. It also rebuilds notice() in cancel_refused.rs as a Map. Nothing #28160 did is undone. #28160 is for Leo's asks:

i literally can't stop you i clicked the stop button and it didnt do anything so this is still busted didnt we say we were going to fix this in acpmux?

this is so awkward the stop button doesnt really work and resume doesnt really appear!?

#28160 made a refused Stop heard (_acpmux/cancel_refused). Two gaps were left that could still leave the pane showing "Stopping" with no reason:

What was wrong

  1. The refusal was lossy. _acpmux/cancel_refused went through Conn::send as an ordinary notification. When the client reads slowly, its outbound queue fills, and enqueue_outbound drops notifications past the replies' reserve. The refusal was dropped, and the pane kept showing Stopping.
  2. The pane matched a refusal by sessionId alone. A late refusal of an earlier Stop could end the Stopping state of a later Stop of the same chat. A refusal that arrived when no Stop was waiting still added a "Not stopped" notice.

The change

Never dropped (server/requests/cancel_refused.rs, Waiting):

  • Each connection keeps a small set of refusals that wait for room in its queue.
  • One task per connection sends them in turn with out.send(..).await, so it waits for room instead of dropping. It ends when the set is empty or the connection's reader is gone.
  • Refusals wait by chat and turn: the newest refusal for a chat's turn replaces the one waiting, and refusals of two turns of one chat both wait. Two Stops of one chat can be answered out of order (a forward may take its 10 s), and the pane takes only the refusal for the turn it stopped. If one key per chat were kept, the earlier turn's late refusal would replace the later turn's.
  • At most 16 refusals wait, so a client that sends Stops and never reads holds at most 16 lines. Past that, a refusal is logged at warn and left out.
  • Refusals are masked and size-bounded exactly as Conn::send does.
  • They are not "control" lines, so they never touch pending_controls or the ordering of replies.

Answers only its own turn:

  • Daemon: the refusal carries turnId.
    • For a chat here, it is the chat's running turn (Session::turn()).
    • For a Stop forwarded to a peer, it is the turn that peer's row showed when the Stop went out, captured before the 10 s forward wait.
  • Pane (direct.ts stopRefused): the refusal counts only while a Stop is waiting (stoppingAfterSeq). When both the refusal's turnId and the stopped turn are known, they must be the same turn.

Docs: turnId and the no-drop rule are added to _acpmux/cancel_refused in docs/acpmux-schema.json. The message field note no longer says it names the machine; it has not since #28160's review.

Risk

risk:normal. This PR changes no auth or ownership rule: who may stop a chat is unchanged from #28160. It only changes how the refusal is delivered and how the pane matches it.

Per connection, it adds:

  • one bounded map, capped at 16 chats;
  • at most one task, which ends when the map is empty or the connection closes.

Tests

cmux-tui/crates/acpmux/tests/cancel_owner.rs:

  • a_refusal_waits_for_a_slow_reader_and_is_not_dropped: a Web client whose queue holds 4 lines sends 5 refused Stops before reading, and it hears all 5.
  • guests_other_members_and_forged_owner_claims_cannot_stop_it_and_hear_why: the refusal's turnId is the chat's running turn.
  • a_peers_refusal_of_a_forwarded_stop_reaches_the_sender: the forwarded refusal's turnId is B's turn, as A's row showed it.
  • Unit tests in cancel_refused.rs: the newest refusal for a chat's turn replaces the waiting one, refusals for two turns of one chat both wait (refusals_for_two_turns_of_a_chat_both_wait), and the cap holds.

webviews/src/agent-session/acpmux/stopping.test.ts, "only the refusal of the Stop still waiting counts":

  • A refusal with no Stop waiting adds no notice.
  • A refusal naming an earlier turn leaves Stopping.
  • A refusal naming the stopped turn ends it with one notice.

Red / green

Red: the test commits 11ea868791a (Rust) and e007766a460 (webview), on the code before the fix.

  • cargo test -p acpmux --test cancel_owner: 3 of 7 fail (run).
    • No turnId on the refusal, local or forwarded.
    • The slow reader hears 1 of 5 refusals.
  • bun test src/agent-session/acpmux/stopping.test.ts: "only the refusal of the Stop still waiting counts" fails (run). A refusal with no Stop waiting adds a notice.

A third red came from the review: refusals_for_two_turns_of_a_chat_both_wait at 35463887d13 (run). The earlier turn's refusal replaced the later one's (["t1"] instead of ["t1", "t2"]).

Green at the head, 25fae3f3d7c, merged up to feat-cmux-next with #28160:

  • Rust (run):
    • cargo fmt --check;
    • cancel_owner 7/7, peer_person_stop 4/4, remote_guard 13/13, session_control_peer_cli 3/3, connection_work_limit 3/3, peer_integration 51/51;
    • the lib tests cancel_refused, cancel_owner and protocol (11);
    • clippy -D warnings.
  • Web (run): bun run check and stopping.test.ts 10/10.

Review

  • gpt-6.1-sol at addcae2f888: BLOCK, one P2. Stops of one chat answered out of order could make an earlier turn's late refusal replace the later turn's while it waited. Fixed in 25fae3f3d7c: refusals wait by chat and turn. A red test was committed first.
  • gpt-6.1-sol at 25fae3f3d7c4ec52c7ce736a0912d341c6cea711, diff against merge-base d2cf48d0a66 (three-dot): SHIP, no findings.

Observed / Not run

  • Observed: the runs above.
  • Not run: a live app against a real slow client. The test drives the daemon's real connection with a 4-line queue.

Changelog

  • Fixed: a Stop that cannot be taken always says why, even when the chat is busy sending, and a late answer to an earlier Stop no longer clears a newer one.

No capture: no visible change beyond the notice #28160 added.

Agent session: agent session

šŸ¤– Generated with Claude Code