Repository navigation
fix(cmux-tui-core): the reader's end, give-up and the owner's detach end the input port's reconnect wait (R-DAEMON-STATE S1.6c) - #35280
Merged
Conversation
…MON-STATE S1 red) A key line from an attached view goes through the real connection path (the line handler, the connection's dispatcher, the send handler) while another thread holds one shared lock for 300 ms. #31940 took Mux.state off the key; the key still waits for the client registry lock (five takes per line), the link peers lock, the host proxy authors lock, the surface admission lock, the creating-input lock, the client sizing lock and the activity lock. Their other holders are every connect, attach, resize and request of every client. Mux.state and the workspace registry rows are controls (pass on the base). Two keys go in a row, so a lock taken after the PTY write (sizing, activity) shows as a delay of the second key on the connection's dispatcher. Test-only hold hooks (cfg(test)) in creating_input.rs, terminal_sizing.rs, activity.rs and agent_session_attach.rs. No product change. Design: handoff/rewrites/R-DAEMON-STATE/DESIGN.md section 2.6 (stage S1).
…N-STATE S1 step 1 red) The previous commit's census test held nine shared locks in turn; this commit splits it by fix step. Step 1 is the client registry lock (ClientRegistry.state), which a key line takes about nine times (handoff checks, remote-client and remote-device checks, the request's actor, the kept terminal, the reply's incognito filter) while every connect, disconnect, attach and detach of every client holds it. The test turns on the stage switch CMUX_TUI_RDS_INPUT (rds_switch.rs, inert in this commit) and drives two keys through the real connection path while another thread holds the lock for 300 ms. A second test keeps Mux.state and the workspace registry off the key on the new path (#31940). The hold hooks of the other locks leave with this commit; they come back with their fix steps.
…ished facts, never its lock (R-DAEMON-STATE S1 step 1) A key line took the client registry lock (ClientRegistry.state) about nine times: the daemon handoff checks, the remote-client and remote-device checks, the request's actor, the kept terminal of the key and the reply's incognito filter. Every connect, disconnect, attach, detach and capability change of every client holds that lock, so an agent storm held back the person's keys (red at 2472251: 300.4 ms behind a 300 ms hold; STATUS 9131). The registry is now a Published value (published.rs): writers still serialize on its mutex, and a guard that changed the value publishes an immutable ClientFacts projection (handoff state; per connection its transport, remote device flag, base actor and kept input terminals) with arc-swap when it drops, under the mutex, so projections follow the order of the changes. With the stage switch CMUX_TUI_RDS_INPUT=1, the line and key paths read the projection with one atomic load: daemon_handoff_in_progress/committed, transport_of (so is_remote_client), is_remote_device (refuse_line, line_to_recheck, prune_reply), connection_actor and attached_input_surface. A poisoned registry still fails closed (the projection comes in the error, as Mutex::lock gives its guard). With the switch off nothing changes. A projection costs O(connections) under the registry lock per change, bounded by the 64-connection cap; stage S2 makes it O(changed). arc-swap 1.9 is already in the lockfile and the shipped notices (through iroh-dns); cmux-tui-core now names it. Tests: rds_input_lane (a key waits for no client registry lock; the session and workspace registry locks stay off; a respawn under the same id sends the next key to the new shell), published.rs unit tests (a change publishes on drop, a read publishes nothing, a reader never waits for a writer, a poisoned value fails closed). The two remote-relay poison tests poison through the new type.
… stage switch per mux (R-DAEMON-STATE S1 step 1) The hosted run of f1ea1b6 (https://github.com/manaflow-ai/cmux/actions/runs/41655768031) still failed both lane tests: the key waited 300 ms for the client registry and, new, for Mux.state. Two causes: - The key still used the kept terminal of #31940. That cache is validated by process-wide route generations, and cargo test runs the tests of one binary as threads of one process: another test's mux moved the same generation slot (ids start at 1 in every mux), the kept terminal looked stale, and the key fell back to the locked lookup and then to remember_attached_input (registry lock). The stage switch was a process-wide static too. - So the lane must not depend on a cache with process-wide invalidation at all. Now, with CMUX_TUI_RDS_INPUT=1: - The session's three routing tables (surfaces, terminal_catalog, terminal_catalog_by_runtime) are copy-on-write behind an Arc. A state guard that changed one publishes a RouteSnapshot (three Arc clones) with arc-swap when it drops, under the lock, so publications follow the changes; the snapshot it replaced is freed after the lock is released. Mux::surface reads the snapshot with one atomic load. A respawn, close, move, rollback or projection is seen once its guard is released, as a reader that waited for the lock would see it. Nothing is kept per client, so nothing is invalidated. - input_surface on the lane: the published routes give the terminal, the registry's published facts say whether the client types into it (types_into: the person's input QoS mark). attached_input_surface and remember_attached_input stay only on the old path. - The switch is per mux (rds_switch::InputLaneSwitch, read once from the environment when the mux is built; Mux::set_input_lane_for_test in tests), and it also decides whether anything is published: off, the registry builds no projection and the state holds no route snapshot, so a route write never copies a shared table and the off arm costs what the base costs. Cost on the lane: a guard's first route write copies that table once (O(routes)), bounded by one copy per changing guard; State::clone no longer deep-copies the three tables. Stage S2 makes both O(log n).
…an old-path control (R-DAEMON-STATE S1 step 3 red) Every request of every connection reserves its retained bytes from one daemon-wide budget (ServerSurfaceOperationAdmission) under one mutex before it is queued, and releases them under it after it ran. The S1 census measured a key waiting 300.3 ms behind a 300 ms hold of that lock (hosted run 41648095321). The lane test now has a control: the same two keys with stage S1 off must wait for the held lock, so a lane result can never come from a hold that does not reach the key path (a vacuous green). The control is added to the client registry test too. A test-only hook (cfg(test)) holds the admission lock. Red here: with the lane on, the key still reserves through the mutex.
…s on the input lane (R-DAEMON-STATE S1 step 3) Every request of every connection reserves its retained bytes (and every clear-history its worker) from one daemon-wide budget before it is queued and releases after it ran. Under one mutex, a key waited for any other connection's reservation or release (red at 5d0e4a0: 300 ms behind a 300 ms hold). With CMUX_TUI_RDS_INPUT=1 the two counters are atomics with compare-and-swap reservations (fetch_update: add only while the sum stays within the capacity; release saturates at zero), so nothing waits. The limits and the refusals are the same as the mutex arm's: a worker is refused at 16 reserved, bytes when the sum would pass 16 MiB. Each permit releases to the arm it reserved from. With the switch off the mutex arm runs as before; it goes with the switch when the stage passes (S1.8). The arm is fixed when the mux is built (ServerSurfaceOperationAdmission::new(publish_routes)); tests set it with Mux::set_input_lane_for_test before any request. The two scheduler tests that counted reserved workers through the mutex read reserved_workers() (both arms).
…roxy authors lock (R-DAEMON-STATE S1 step 2 red) Two more shared locks on the person's key line (DESIGN.md Appendix A.2 steps 3, 7, 12 and 13): the link peers map (RemoteRelayState.peers: the remote-client check and the request's actor) and the host proxy authors (AgentSessions.proxy_authors: the proxy-link check on the reader thread). The S1 census measured a full 300 ms wait on each (hosted run 41648095321: 300.5 ms and 300.4 ms). The test holds each lock 300 ms on another thread while two keys go through the real line path. It first proves the hold reaches the old path (stage switch off: the key waits), then expects the stage's path not to wait. Red on this commit: the stage does not publish either value yet.
… (R-DAEMON-STATE S1 red) A published value's guard publishes its projection in Drop::drop, which also runs while a panic unwinds, before its MutexGuard field drops and marks the mutex poisoned. A writer that panics in the middle of a change so publishes the half-made value, and in the moment between the publication and the poisoning a reader gets it as a good value: a fail-open window for readers that fail closed on a poisoned value (transport_of, peer_checked). Red on this commit: the test sees the half-made value 9 in the poisoned projection.
…panicked change publishes no routes (R-DAEMON-STATE S1 red) Two defects the #34014 review found on the input lane (both only with CMUX_TUI_RDS_INPUT on): - RouteTable::try_reserve returns early when the map has room, also when a published route snapshot shares the map. The insert after the durable commit then copies the whole table (Arc::make_mut), an allocation that ResourceMutationPlan::apply forbids ("Plan builders reserve all needed capacities before returning"). - The session state guard publishes the routes of a change that a panic cut short: Drop::drop runs during the unwind, before the MutexGuard field poisons the mutex, so the lane routes by half-made tables. Red on this commit: the insert changes the map's pointer, and the routes snapshot is replaced.
…the S1.1 review follow-ups (R-DAEMON-STATE S1 step 2) A person's key line asked two more shared locks (DESIGN.md Appendix A.2 steps 3, 7, 12, 13): the link peers map (the remote-client check and the request's actor) and the host proxy authors (the proxy-link check on the reader thread). Every remote bind, disconnect and revocation sweep holds the first; every proxy bind and every client's disconnect holds the second. - published.rs moves to cmux-tui-util (daemon primitives, no daemon state) so cmux-tui-remote-access can use it. RemoteRelayState.peers becomes a Published copy of the records; peer_checked reads it with no lock when the mux's stage switch is on (Published::publishing), and a poisoned map still fails closed. AgentSessions.proxy_authors publishes the same way; proxy::author reads it. A disconnect of a client without a record publishes nothing. - Review follow-ups of #34014 (review comment 6612546773): - RouteTable::try_reserve copies a map a published snapshot shares, before the durable commit, so the insert after the commit allocates nothing (ResourceMutationPlan::apply). - A change a panic cuts short is never published, by the state guard (routes) or by a PublishGuard: Drop skips the publication when a panic began while the guard held the lock (std's own poisoning test), so no reader sees half-made tables, not even before the mutex is poisoned. - A PublishGuard frees the projection it replaced after the unlock (as the state guard does). - The two poison tests of the relay run on both arms. - ClientFacts and types_into move to server/client_registry/facts.rs (god-file warning). Same switch, CMUX_TUI_RDS_INPUT, default off: with it off nothing is published and every read takes its lock as on the base.
…no session lock behind one (R-DAEMON-STATE S1 step 4 red) Every key to a live terminal first asks whether creating input of its terminal must go first (wait_for_creating_input_flush, DESIGN.md Appendix A.2 step 16). It takes the daemon-wide creating-input lock even when no terminal is being created, and that lock's holders (begin_creating_input, knows_terminal_input_target, finish_creating_input) look up the live terminal under it, which takes the session lock. The S1 census measured a full 300 ms wait on the creating-input lock (hosted run 41648095321: 300.3 ms). Two holders, each with an old-path control: the creating-input lock itself, and the session lock held while a reader waits for it under the creating-input lock (with another terminal being created, so the key cannot skip the lock). Red on this commit: the stage does not change these paths yet.
… is creating; the lock becomes a leaf (R-DAEMON-STATE S1 step 4) On the input lane (CMUX_TUI_RDS_INPUT, per mux, default off): - CreatingInputs keeps the number of creating terminals in an atomic, stored under its lock after every insert and removal (Release). A key reads it (Acquire) and returns at once when it is zero: no queue can be ahead of it, because a terminal's queue is added before the terminal goes live and removed only after every queued byte was written to it. - The creating-input lock's holders look up the live terminal in the published routes instead of under the session lock, so no holder waits for the session lock while it holds the creating-input lock, and a key that takes it never waits behind one. The routes carry the host-id catalog index for this: terminal_catalog_by_host becomes a copy-on-write route table in the RouteSnapshot, and one lookup (route_table::live_terminal) serves the session state and the snapshot. A commit publishes its routes before it releases the session lock, and finish runs after the commit, so begin, knows and finish give the answers they gave under the session lock. Off, every path takes the locks it took on the base; the by-host index is copy-on-write on both arms, as the other three route tables are since step 1.
… control fails on a lost key (#34146 review follow-ups) The #34146 review (comment 6613345630) noted: - Every budget test used the mutex arm, so an atomic arm that admitted everything would have passed. The two mux-wide limit tests now run on both arms, and a new test checks the same limits on both: the 17th worker, the whole byte budget, one byte over, zero bytes, a sum that overflows, and released reservations coming back. - The old-path control passed when a key never arrived (Duration::MAX >= BUDGET); it now fails on a lost key. - ServerSurfaceOperationState and the admission's mutex are private (nothing outside the file reads them), and the doc says which requests reserve (pings, identify, v2 resource lines and reader-thread frame handlers do not).
…(clippy unused_qualifications) The lint (linux) job's clippy -D warnings failed on mux/tests/signaled_mutex.rs:120 (TerminalPublicId is in scope); the #34174 review found it behind the base's acpmux red in the same job.
…lished tables (R-DAEMON-STATE S1 red) The #34171 review noted: after a panic poisons Mux::state, the next guard that recovers the poison may publish the half-made route tables, and the lane's Mux::surface reads the published routes with no poison check, where the old path fails (it unwraps the lock). Red on this commit: with the lane on, a surface read after the poisoning returns from the published routes instead of failing. The relay poison tests now poison published values through a helper in the test file.
…oisoned state; the poison helper is test code (#34171 review) Review comment 6614044759 (FIX at 8f7d405): - published.rs: poison_for_test moved out of the test module with its panic! respelled as panic_any, which hides a hit from the crash ratchet's count. It is test code again: a private helper in the published tests and one in the relay tests, with plain panic!. - Mux::surface on the lane reads the published routes only while the session state is not poisoned; a poisoned state takes the old read and fails as it does (StateMutex::is_poisoned). A guard that recovers the poison may publish the half-made tables of the change that panicked. - Published::load debug-asserts that the value publishes: only a publishing value has a current projection. - The module doc says what load gives on a poisoned value; the caller decides whether to fail closed. The unused_qualifications error at mux/tests/signaled_mutex.rs:120 is fixed in 10f1669.
…ly on an unpoisoned state; its comment states the lane's linearization (#34174 review) Review comment 6613998735 (SHIP, notes): the begin_creating_input comment still said the check takes the state under the creating lock; on the lane it reads the published routes, and its answer is one the old path can give (a commit in progress has not made the id live, as when the old path takes the state before it). The lane's live lookup now takes the old read on a poisoned state, as Mux::surface does since bb0265f.
…ity lock; a key takes the grid back after another view's click (R-DAEMON-STATE S1 step 5 red) After its PTY write a key notes activity in the terminal's sizing (the newest view sets the grid under the default policy): it takes the daemon-wide client sizing lock twice (the note and the size-state publication), then the VM activity stream's lock. Every attach, detach, resize and size report of every terminal holds the sizing lock, and a PTY resize runs under it. The S1 census measured a full 300 ms wait on each (hosted run 41648095321: 300.2 and 300.3 ms). The first test holds each lock 300 ms (with an old-path control each). The second keeps the fix honest: two views of one terminal, the default policy; the other view's focus click takes the grid, and the next key from the typing view must take it back, on both arms. Red on this commit: the first test (the stage does not change these paths yet).
…settled newest; the person's input time is an atomic (R-DAEMON-STATE S1 step 5) On the input lane (CMUX_TUI_RDS_INPUT, per mux, default off): - Each terminal's sizing entry publishes, on the terminal's runtime (shared by every placement), the view a key may skip its sizing note for: the sole newest participant, a client's own view, only while the entry is settled (no grid hold, no handheld with a viewport, and the engine holds the grid or the applied grid and participants are its decision). Noting activity for that view changes no decision under any policy, and the size state carries no activity, so the key compares (client, placement) with one atomic load and takes no sizing lock. - The publication is automatic: Mux::client_sizing is a SizingMutex whose guard republishes, on drop and under the lock, each entry its holder read or wrote (SizingEntries marks them; an apply changes Cells through a shared borrow, so reads count). A removed entry clears its slot; a change a panic cuts short publishes nothing. - Another view's activity makes that view the newest and republishes, so the next key from the first view takes the slow path once and takes the grid back (the second test pins this on both arms). - ActivityStream::note_user_input stores the time in an atomic; it takes the stream lock only while a subscriber exists, once per event the worker sends (a wake_pending flag the worker clears before it reads the time). - TerminalSizingEngine::sole_latest (cmux-terminal-sizing) with a unit test; AppliedSeat::of shares the seat rule between the apply and the settled check; set_workspace_size_policy reads its state before the note (no index). Off, every key takes the sizing and activity locks as on the base; the marks and the guard check one bool.
…ook's cfg(test) (dead code and 2 expect hits in production) dfe81bf put hold_client_sizing_for_test between pin_latest_size_policy_for_test's doc and its #[cfg(test)], so pin_latest_size_policy_for_test compiled into production: clippy -D warnings dead_code, and the panic ratchet's cmux-tui-core expect_used 175 > 173 (hosted lint job 126202464363).
…ts poison check; two test doc comments on their items (#34171 review 2) Review comment 6615927915 (SHIP at 0fb7c43, notes): Mux::surface checked is_poisoned() and then loaded the routes; between the two another thread could poison the state and a third, recovering the poison, publish half-made tables. Poison never clears on the session state, so routes loaded before an unpoisoned check were published before any poisoning: load first, then check. Two doc comments in remote_relay/tests.rs sat above the wrong item.
…content is already here (590cb11); tree unchanged
…_be_written through the module scope (clippy unused_qualifications) lint (linux) at bbc44c2 (runs 41806402684, 41806407494) failed clippy -D warnings on four qualified paths in surface/tests/hosted.rs; both names are in scope through use super::*.
…eader gives up (R-DAEMON-STATE S1.6c red, #35082 review 2) The reader's end, give-up and failed-reconnect paths now call PtyTerminalRuntime::reader_gave_up, which still only notifies stream_progress (today's behavior). The port's writer waits on its route epoch since #35082 review 1, so the wake does not reach it: a_write_waiting_for_a_reconnect_fails_when_the_reader_gives_up waits its full reconnect deadline (5 s) instead of failing at once.
…end the input port's reconnect wait (R-DAEMON-STATE S1.6c, #35082 review 2) Since review 1 the port's writer waits on its own route epoch, so the reader's stream_progress wake no longer reached it: a Written write behind a host the reader gave up on failed only at its reconnect deadline (5 s), not at once as the old path's write did. InputPort::reconnect_ended moves the epoch with no new route; the writer checks the route and the terminal's state again and fails its frame (Failed state, ended reader, or owner detaching). - PtyTerminalRuntime::reader_gave_up (WakeInputOnReaderEnd, wait_or_fail's give-up, a failed reconnect completion) notifies the stream and ends the port's wait. - The four owner_detaching stores in shutdown.rs end the port's wait (only the port lock, under the runtime lock). Test: a_write_waiting_for_a_reconnect_fails_when_the_reader_gives_up (red at b6b0b37).
Contributor
Author
|
Review (claude-reviewer-rds-s16c): SHIP at 1e84ed3 This fixes should-do 1 of my review 2 on #35082. I read the code and the CI logs; I did not build or run anything. Paths are under I checked the claims about the history:
Answers to the three questions1. Is a wake point missing? No.The writer stops on
At every site the store comes before the wake. The port mutex orders the two, so a woken writer always sees its stop condition. In production only the reader thread installs routes ( 2. Can
|
lawrencecchen
added a commit
that referenced
this pull request
Nov 4, 2026
…own detach end a write's reconnect wait (#35280 review, should-do 1) The S1.6c part-1 test called reader_gave_up by hand, so a call site that lost its wake turned no test red. These drive the real call sites: an exhausted TerminalHostReconnectBackoff's wait_or_fail, and Surface::disconnect_for_daemon_shutdown, each while a Written write waits for a reconnect.
lawrencecchen
added a commit
that referenced
this pull request
Nov 4, 2026
…te the lock rule the right way round (#35280 review, should-do 2) The port lock is a leaf: no holder of it waits for another lock, so the runtime lock's holders may take it. The shutdown.rs comments said the reverse.
Base automatically changed from
fix/p0-typing-s1-input-port-order
to
feat-cmux-next
November 4, 2026 11:39
…ping-s1-6c-control-lane
…trol-lane: its content is here through the S1.6b head
lawrencecchen
temporarily deployed
to
ci-routing-agent
November 4, 2026 11:47 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
Contributor
Contributor
Author
|
Review (claude-reviewer-rds-s16c): SHIP at 32f0563 This re-review follows the landing of #35082 (squash 65d098c) and the move of this branch onto feat. The PR's own code is the code I passed at 1e84ed3. I checked the merges myself; I did not build or run anything. The merges
Tests
CI at this head
|
lawrencecchen
temporarily deployed
to
sentry-dsyms
November 4, 2026 11:57 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
temporarily deployed
to
artifacts
November 4, 2026 11:57 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
added
the
ready-to-land
lawrencecchen
temporarily deployed
to
artifacts
November 4, 2026 12:03 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
lawrencecchen
added a commit
that referenced
this pull request
Nov 4, 2026
…-6c2-control-frames: its content is here through the #35280 head
lawrencecchen
added a commit
that referenced
this pull request
Nov 4, 2026
lawrencecchen
added a commit
that referenced
this pull request
Nov 4, 2026
… without waiting; the settle ends by its deadline; a lost taken frame may be written (R-DAEMON-STATE S1.6c part 2) (#35375) * test(cmux-tui-core): the person's key waits for no shared lock (R-DAEMON-STATE S1 red) A key line from an attached view goes through the real connection path (the line handler, the connection's dispatcher, the send handler) while another thread holds one shared lock for 300 ms. #31940 took Mux.state off the key; the key still waits for the client registry lock (five takes per line), the link peers lock, the host proxy authors lock, the surface admission lock, the creating-input lock, the client sizing lock and the activity lock. Their other holders are every connect, attach, resize and request of every client. Mux.state and the workspace registry rows are controls (pass on the base). Two keys go in a row, so a lock taken after the PTY write (sizing, activity) shows as a delay of the second key on the connection's dispatcher. Test-only hold hooks (cfg(test)) in creating_input.rs, terminal_sizing.rs, activity.rs and agent_session_attach.rs. No product change. Design: handoff/rewrites/R-DAEMON-STATE/DESIGN.md section 2.6 (stage S1). * test(cmux-tui-core): a key waits for no client registry lock (R-DAEMON-STATE S1 step 1 red) The previous commit's census test held nine shared locks in turn; this commit splits it by fix step. Step 1 is the client registry lock (ClientRegistry.state), which a key line takes about nine times (handoff checks, remote-client and remote-device checks, the request's actor, the kept terminal, the reply's incognito filter) while every connect, disconnect, attach and detach of every client holds it. The test turns on the stage switch CMUX_TUI_RDS_INPUT (rds_switch.rs, inert in this commit) and drives two keys through the real connection path while another thread holds the lock for 300 ms. A second test keeps Mux.state and the workspace registry off the key on the new path (#31940). The hold hooks of the other locks leave with this commit; they come back with their fix steps. * fix(cmux-tui-core): the person's key reads the client registry's published facts, never its lock (R-DAEMON-STATE S1 step 1) A key line took the client registry lock (ClientRegistry.state) about nine times: the daemon handoff checks, the remote-client and remote-device checks, the request's actor, the kept terminal of the key and the reply's incognito filter. Every connect, disconnect, attach, detach and capability change of every client holds that lock, so an agent storm held back the person's keys (red at 2472251: 300.4 ms behind a 300 ms hold; STATUS 9131). The registry is now a Published value (published.rs): writers still serialize on its mutex, and a guard that changed the value publishes an immutable ClientFacts projection (handoff state; per connection its transport, remote device flag, base actor and kept input terminals) with arc-swap when it drops, under the mutex, so projections follow the order of the changes. With the stage switch CMUX_TUI_RDS_INPUT=1, the line and key paths read the projection with one atomic load: daemon_handoff_in_progress/committed, transport_of (so is_remote_client), is_remote_device (refuse_line, line_to_recheck, prune_reply), connection_actor and attached_input_surface. A poisoned registry still fails closed (the projection comes in the error, as Mutex::lock gives its guard). With the switch off nothing changes. A projection costs O(connections) under the registry lock per change, bounded by the 64-connection cap; stage S2 makes it O(changed). arc-swap 1.9 is already in the lockfile and the shipped notices (through iroh-dns); cmux-tui-core now names it. Tests: rds_input_lane (a key waits for no client registry lock; the session and workspace registry locks stay off; a respawn under the same id sends the next key to the new shell), published.rs unit tests (a change publishes on drop, a read publishes nothing, a reader never waits for a writer, a poisoned value fails closed). The two remote-relay poison tests poison through the new type. * fix(cmux-tui-core): the key reads the session's published routes; one stage switch per mux (R-DAEMON-STATE S1 step 1) The hosted run of f1ea1b6 (https://github.com/manaflow-ai/cmux/actions/runs/41655768031) still failed both lane tests: the key waited 300 ms for the client registry and, new, for Mux.state. Two causes: - The key still used the kept terminal of #31940. That cache is validated by process-wide route generations, and cargo test runs the tests of one binary as threads of one process: another test's mux moved the same generation slot (ids start at 1 in every mux), the kept terminal looked stale, and the key fell back to the locked lookup and then to remember_attached_input (registry lock). The stage switch was a process-wide static too. - So the lane must not depend on a cache with process-wide invalidation at all. Now, with CMUX_TUI_RDS_INPUT=1: - The session's three routing tables (surfaces, terminal_catalog, terminal_catalog_by_runtime) are copy-on-write behind an Arc. A state guard that changed one publishes a RouteSnapshot (three Arc clones) with arc-swap when it drops, under the lock, so publications follow the changes; the snapshot it replaced is freed after the lock is released. Mux::surface reads the snapshot with one atomic load. A respawn, close, move, rollback or projection is seen once its guard is released, as a reader that waited for the lock would see it. Nothing is kept per client, so nothing is invalidated. - input_surface on the lane: the published routes give the terminal, the registry's published facts say whether the client types into it (types_into: the person's input QoS mark). attached_input_surface and remember_attached_input stay only on the old path. - The switch is per mux (rds_switch::InputLaneSwitch, read once from the environment when the mux is built; Mux::set_input_lane_for_test in tests), and it also decides whether anything is published: off, the registry builds no projection and the state holds no route snapshot, so a route write never copies a shared table and the off arm costs what the base costs. Cost on the lane: a guard's first route write copies that table once (O(routes)), bounded by one copy per changing guard; State::clone no longer deep-copies the three tables. Stage S2 makes both O(log n). * test(cmux-tui-core): a key waits for no surface admission lock, with an old-path control (R-DAEMON-STATE S1 step 3 red) Every request of every connection reserves its retained bytes from one daemon-wide budget (ServerSurfaceOperationAdmission) under one mutex before it is queued, and releases them under it after it ran. The S1 census measured a key waiting 300.3 ms behind a 300 ms hold of that lock (hosted run 41648095321). The lane test now has a control: the same two keys with stage S1 off must wait for the held lock, so a lane result can never come from a hold that does not reach the key path (a vacuous green). The control is added to the client registry test too. A test-only hook (cfg(test)) holds the admission lock. Red here: with the lane on, the key still reserves through the mutex. * fix(cmux-tui-core): the surface admission budget reserves with atomics on the input lane (R-DAEMON-STATE S1 step 3) Every request of every connection reserves its retained bytes (and every clear-history its worker) from one daemon-wide budget before it is queued and releases after it ran. Under one mutex, a key waited for any other connection's reservation or release (red at 5d0e4a0: 300 ms behind a 300 ms hold). With CMUX_TUI_RDS_INPUT=1 the two counters are atomics with compare-and-swap reservations (fetch_update: add only while the sum stays within the capacity; release saturates at zero), so nothing waits. The limits and the refusals are the same as the mutex arm's: a worker is refused at 16 reserved, bytes when the sum would pass 16 MiB. Each permit releases to the arm it reserved from. With the switch off the mutex arm runs as before; it goes with the switch when the stage passes (S1.8). The arm is fixed when the mux is built (ServerSurfaceOperationAdmission::new(publish_routes)); tests set it with Mux::set_input_lane_for_test before any request. The two scheduler tests that counted reserved workers through the mutex read reserved_workers() (both arms). * test(cmux-tui-core): a key waits for neither the link peers nor the proxy authors lock (R-DAEMON-STATE S1 step 2 red) Two more shared locks on the person's key line (DESIGN.md Appendix A.2 steps 3, 7, 12 and 13): the link peers map (RemoteRelayState.peers: the remote-client check and the request's actor) and the host proxy authors (AgentSessions.proxy_authors: the proxy-link check on the reader thread). The S1 census measured a full 300 ms wait on each (hosted run 41648095321: 300.5 ms and 300.4 ms). The test holds each lock 300 ms on another thread while two keys go through the real line path. It first proves the hold reaches the old path (stage switch off: the key waits), then expects the stage's path not to wait. Red on this commit: the stage does not publish either value yet. * test(cmux-tui-core): a change cut short by a panic is never published (R-DAEMON-STATE S1 red) A published value's guard publishes its projection in Drop::drop, which also runs while a panic unwinds, before its MutexGuard field drops and marks the mutex poisoned. A writer that panics in the middle of a change so publishes the half-made value, and in the moment between the publication and the poisoning a reader gets it as a good value: a fail-open window for readers that fail closed on a poisoned value (transport_of, peer_checked). Red on this commit: the test sees the half-made value 9 in the poisoned projection. * test(cmux-tui-core): a reservation unshares a published route map; a panicked change publishes no routes (R-DAEMON-STATE S1 red) Two defects the #34014 review found on the input lane (both only with CMUX_TUI_RDS_INPUT on): - RouteTable::try_reserve returns early when the map has room, also when a published route snapshot shares the map. The insert after the durable commit then copies the whole table (Arc::make_mut), an allocation that ResourceMutationPlan::apply forbids ("Plan builders reserve all needed capacities before returning"). - The session state guard publishes the routes of a change that a panic cut short: Drop::drop runs during the unwind, before the MutexGuard field poisons the mutex, so the lane routes by half-made tables. Red on this commit: the insert changes the map's pointer, and the routes snapshot is replaced. * fix(cmux-tui-core): link peers and host proxy authors are published; the S1.1 review follow-ups (R-DAEMON-STATE S1 step 2) A person's key line asked two more shared locks (DESIGN.md Appendix A.2 steps 3, 7, 12, 13): the link peers map (the remote-client check and the request's actor) and the host proxy authors (the proxy-link check on the reader thread). Every remote bind, disconnect and revocation sweep holds the first; every proxy bind and every client's disconnect holds the second. - published.rs moves to cmux-tui-util (daemon primitives, no daemon state) so cmux-tui-remote-access can use it. RemoteRelayState.peers becomes a Published copy of the records; peer_checked reads it with no lock when the mux's stage switch is on (Published::publishing), and a poisoned map still fails closed. AgentSessions.proxy_authors publishes the same way; proxy::author reads it. A disconnect of a client without a record publishes nothing. - Review follow-ups of #34014 (review comment 6612546773): - RouteTable::try_reserve copies a map a published snapshot shares, before the durable commit, so the insert after the commit allocates nothing (ResourceMutationPlan::apply). - A change a panic cuts short is never published, by the state guard (routes) or by a PublishGuard: Drop skips the publication when a panic began while the guard held the lock (std's own poisoning test), so no reader sees half-made tables, not even before the mutex is poisoned. - A PublishGuard frees the projection it replaced after the unlock (as the state guard does). - The two poison tests of the relay run on both arms. - ClientFacts and types_into move to server/client_registry/facts.rs (god-file warning). Same switch, CMUX_TUI_RDS_INPUT, default off: with it off nothing is published and every read takes its lock as on the base. * test(cmux-tui-core): a key waits for no creating-input lock, and for no session lock behind one (R-DAEMON-STATE S1 step 4 red) Every key to a live terminal first asks whether creating input of its terminal must go first (wait_for_creating_input_flush, DESIGN.md Appendix A.2 step 16). It takes the daemon-wide creating-input lock even when no terminal is being created, and that lock's holders (begin_creating_input, knows_terminal_input_target, finish_creating_input) look up the live terminal under it, which takes the session lock. The S1 census measured a full 300 ms wait on the creating-input lock (hosted run 41648095321: 300.3 ms). Two holders, each with an old-path control: the creating-input lock itself, and the session lock held while a reader waits for it under the creating-input lock (with another terminal being created, so the key cannot skip the lock). Red on this commit: the stage does not change these paths yet. * fix(cmux-tui-core): a key skips the creating-input lock while nothing is creating; the lock becomes a leaf (R-DAEMON-STATE S1 step 4) On the input lane (CMUX_TUI_RDS_INPUT, per mux, default off): - CreatingInputs keeps the number of creating terminals in an atomic, stored under its lock after every insert and removal (Release). A key reads it (Acquire) and returns at once when it is zero: no queue can be ahead of it, because a terminal's queue is added before the terminal goes live and removed only after every queued byte was written to it. - The creating-input lock's holders look up the live terminal in the published routes instead of under the session lock, so no holder waits for the session lock while it holds the creating-input lock, and a key that takes it never waits behind one. The routes carry the host-id catalog index for this: terminal_catalog_by_host becomes a copy-on-write route table in the RouteSnapshot, and one lookup (route_table::live_terminal) serves the session state and the snapshot. A commit publishes its routes before it releases the session lock, and finish runs after the commit, so begin, knows and finish give the answers they gave under the session lock. Off, every path takes the locks it took on the base; the by-host index is copy-on-write on both arms, as the other three route tables are since step 1. * test(cmux-tui-core): the surface budget's limits on both arms; a lock control fails on a lost key (#34146 review follow-ups) The #34146 review (comment 6613345630) noted: - Every budget test used the mutex arm, so an atomic arm that admitted everything would have passed. The two mux-wide limit tests now run on both arms, and a new test checks the same limits on both: the 17th worker, the whole byte budget, one byte over, zero bytes, a sum that overflows, and released reservations coming back. - The old-path control passed when a key never arrived (Duration::MAX >= BUDGET); it now fails on a lost key. - ServerSurfaceOperationState and the admission's mutex are private (nothing outside the file reads them), and the doc says which requests reserve (pings, identify, v2 resource lines and reader-thread frame handlers do not). * fix(cmux-tui-core): drop an unnecessary path qualification in a test (clippy unused_qualifications) The lint (linux) job's clippy -D warnings failed on mux/tests/signaled_mutex.rs:120 (TerminalPublicId is in scope); the #34174 review found it behind the base's acpmux red in the same job. * test(cmux-tui-core): a poisoned session state never routes by the published tables (R-DAEMON-STATE S1 red) The #34171 review noted: after a panic poisons Mux::state, the next guard that recovers the poison may publish the half-made route tables, and the lane's Mux::surface reads the published routes with no poison check, where the old path fails (it unwraps the lock). Red on this commit: with the lane on, a surface read after the poisoning returns from the published routes instead of failing. The relay poison tests now poison published values through a helper in the test file. * fix(cmux-tui-core): the lane's surface read takes the old path on a poisoned state; the poison helper is test code (#34171 review) Review comment 6614044759 (FIX at 8f7d405): - published.rs: poison_for_test moved out of the test module with its panic! respelled as panic_any, which hides a hit from the crash ratchet's count. It is test code again: a private helper in the published tests and one in the relay tests, with plain panic!. - Mux::surface on the lane reads the published routes only while the session state is not poisoned; a poisoned state takes the old read and fails as it does (StateMutex::is_poisoned). A guard that recovers the poison may publish the half-made tables of the change that panicked. - Published::load debug-asserts that the value publishes: only a publishing value has a current projection. - The module doc says what load gives on a poisoned value; the caller decides whether to fail closed. The unused_qualifications error at mux/tests/signaled_mutex.rs:120 is fixed in 10f1669. * fix(cmux-tui-core): the creating-input live check reads the routes only on an unpoisoned state; its comment states the lane's linearization (#34174 review) Review comment 6613998735 (SHIP, notes): the begin_creating_input comment still said the check takes the state under the creating lock; on the lane it reads the published routes, and its answer is one the old path can give (a commit in progress has not made the id live, as when the old path takes the state before it). The lane's live lookup now takes the old read on a poisoned state, as Mux::surface does since bb0265f. * test(cmux-tui-core): a key waits for neither the sizing nor the activity lock; a key takes the grid back after another view's click (R-DAEMON-STATE S1 step 5 red) After its PTY write a key notes activity in the terminal's sizing (the newest view sets the grid under the default policy): it takes the daemon-wide client sizing lock twice (the note and the size-state publication), then the VM activity stream's lock. Every attach, detach, resize and size report of every terminal holds the sizing lock, and a PTY resize runs under it. The S1 census measured a full 300 ms wait on each (hosted run 41648095321: 300.2 and 300.3 ms). The first test holds each lock 300 ms (with an old-path control each). The second keeps the fix honest: two views of one terminal, the default policy; the other view's focus click takes the grid, and the next key from the typing view must take it back, on both arms. Red on this commit: the first test (the stage does not change these paths yet). * fix(cmux-tui-util): a test dereferences the published u64 once (E0614 in bb0265f) * fix(cmux-tui-core): a key skips its sizing note when its view is the settled newest; the person's input time is an atomic (R-DAEMON-STATE S1 step 5) On the input lane (CMUX_TUI_RDS_INPUT, per mux, default off): - Each terminal's sizing entry publishes, on the terminal's runtime (shared by every placement), the view a key may skip its sizing note for: the sole newest participant, a client's own view, only while the entry is settled (no grid hold, no handheld with a viewport, and the engine holds the grid or the applied grid and participants are its decision). Noting activity for that view changes no decision under any policy, and the size state carries no activity, so the key compares (client, placement) with one atomic load and takes no sizing lock. - The publication is automatic: Mux::client_sizing is a SizingMutex whose guard republishes, on drop and under the lock, each entry its holder read or wrote (SizingEntries marks them; an apply changes Cells through a shared borrow, so reads count). A removed entry clears its slot; a change a panic cuts short publishes nothing. - Another view's activity makes that view the newest and republishes, so the next key from the first view takes the slow path once and takes the grid back (the second test pins this on both arms). - ActivityStream::note_user_input stores the time in an atomic; it takes the stream lock only while a subscriber exists, once per event the worker sends (a wake_pending flag the worker clears before it reads the time). - TerminalSizingEngine::sole_latest (cmux-terminal-sizing) with a unit test; AppliedSeat::of shares the seat rule between the apply and the settled check; set_workspace_size_policy reads its state before the note (no index). Off, every key takes the sizing and activity locks as on the base; the marks and the guard check one bool. * fix(cmux-tui-core): the sizing test hook went in above another test hook's cfg(test) (dead code and 2 expect hits in production) dfe81bf put hold_client_sizing_for_test between pin_latest_size_policy_for_test's doc and its #[cfg(test)], so pin_latest_size_policy_for_test compiled into production: clippy -D warnings dead_code, and the panic ratchet's cmux-tui-core expect_used 175 > 173 (hosted lint job 126202464363). * fix(cmux-tui-core): the lane's surface read loads the routes before its poison check; two test doc comments on their items (#34171 review 2) Review comment 6615927915 (SHIP at 0fb7c43, notes): Mux::surface checked is_poisoned() and then loaded the routes; between the two another thread could poison the state and a third, recovering the poison, publish half-made tables. Poison never clears on the session state, so routes loaded before an unpoisoned check were published before any poisoning: load first, then check. Two doc comments in remote_relay/tests.rs sat above the wrong item. * test(cmux-tui-core): a key does not wait for another writer's blocked host socket (R-DAEMON-STATE S1 step 6 red) A key to a hosted terminal holds the terminal's runtime lock and its HostAttachment.writer across a blocking write to the host socket (surface/input.rs, attachment.rs send_host_frame). Another client's large paste to the same terminal holds both for its whole write, bounded only by the socket's send timeout, so the person's key waits for automation's work on the same terminal (DESIGN.md section 2.6.1, Appendix A.2 steps 20-22). The test pastes 8 MiB while the fake host reads nothing for 600 ms, then sends one key. A control first proves the old path waits (the key takes at least half the stall); the lane must return the key in < 100 ms, and the host must then read the paste before the key, and the key once. Red on this commit: the lane has no input port yet. * fix(cmux-tui-core): a person's key to a hosted terminal goes to the terminal's input port and never waits for a socket write (R-DAEMON-STATE S1 step 6) On the input lane (CMUX_TUI_RDS_INPUT, per mux, default off), interactive input to a hosted terminal (Surface::write_bytes, Surface::write_paste) goes to the terminal's own input port (surface/input_port.rs): one queue per terminal, drained in order by the terminal's writer thread (terminal-input-port, started on the first input, ended after 30 s idle). The request returns once the port took the bytes; it takes neither the terminal's runtime lock nor the host writer, so another writer of the same terminal (a large paste, receipted input, a cell-pixel change waiting for its acknowledgement) and a host that does not read its socket no longer hold the person's key or the connection's dispatcher. - The writer writes on the route of the current host connection (HostInputRoute: the connection's writer), which the terminal publishes whenever its runtime changes: the hosted and exited constructors, and PtyTerminalRuntime::replace_runtime, which the reconnect install and the host exit now use. So the writer never takes the runtime lock either. - A lost connection is retried as write_bytes did (cx-6so.49), on the port's thread: wait for the reader's next stream change, write to the new host, until HOST_INPUT_RECONNECT_WAIT. - A full port (32 MiB queued) refuses at once (terminal.input_busy, WouldBlock); a write larger than one host frame is refused as before; the exited final screen drops input as before. - Input the port took and then could not write is lost as a failed write was, with a log line: the request was already answered (the reply now means "accepted by the terminal's port", DESIGN.md 2.6.1). - Not in this step (S1.6b, before the default flip): the connection's other frames still write directly, so while the port is behind they can go before a key it still holds. Off, every write takes the path it took on the base; the route is published on both arms (one Arc per change). * test(cmux-tui-core): after a respawn, keys skip the sizing lock again; the lock tests report a viewport (#34261 review red) Review comment 6619473751 (FIX at 846d412): 1. A respawn under the same runtime id keeps the sizing entry, which keeps the dead runtime's fast-path slot: the new runtime's slot is never published, so every later key takes Mux::client_sizing twice. The new test respawns the typing terminal, sends one key, then holds the sizing lock: the next keys must not wait. Red on this commit. 2. The lock tests' view never reported a size, so the engine held the grid and the skip went only through that branch. The typing view now reports 100x30 as the app's view does, so its keys skip through the settled-grid check. * fix(cmux-tui-core): a sizing entry follows its terminal across a respawn; the input time never goes back (#34261 review) Review comment 6619473751 (FIX at 846d412): 1. A respawn under the same runtime id keeps the sizing entry but builds a new terminal runtime with a new fast-path slot. TerminalSizingEntry::follow_runtime_of re-points the entry at the slot of the runtime its keys reach, under the sizing lock, and clears the old slot: on the key's slow path (note_terminal_activity_of has the key's surface) and on every terminal_sizing_entry call (attach, claim, size report). The red test after_a_respawn_keys_wait_for_no_sizing_lock_again (4bf5af5) pins it. 3. The entry takes or follows a slot only from a surface whose runtime id is the entry's, so two entries never share one slot. 4. The lock-free input time is fetch_max, so a key preempted after its clock read never moves it back. 7. The poison comment says what happens: a later holder that recovers the poison republishes what it touches. Lock order is unchanged: terminal_sizing_entry now looks the surface up on each call (it did at creation, and apply_terminal_grid does), under the sizing lock, the established sizing-then-session order. * fix(cmux-tui-core): name the input port module without super:: where surface::* is in scope (clippy unused_qualifications) * test(cmux-tui-core): a settled size-state read takes the session lock once with the switch off (#34261 review 2 red) The sizing entry looks its surface up on every access since 5fe770d, so with the switch off a size-state read takes the session lock twice (once under the sizing lock): expected red at this commit (lane off: 2, not 1). * fix(cmux-tui-core): a sizing entry looks its surface up only when it is made; the respawn follow runs only on the lane (#34261 review 2) 5fe770d looked the surface up on every sizing-entry access, so with the switch off (the default) every attach, size report, claim and size-state read took the session lock under the sizing lock. The surface is looked up only to make the entry again; a respawn's new runtime is followed by its first key's sizing note, which holds the surface, and only while the switch is on. An unchanged fast path is not stored again. The respawn test pins the old slot's clear. * fix(cmux-tui-core): the input port's order test can fail; the port copies outside its lock, counts each frame, recovers from a writer panic and refuses a failed link (#34544 review) - The test compared two Options (None < Some), so a key that passed the paste still passed; both positions are required now. A new test sends 26 keys on the lane and checks the host reads each once, in order. - enqueue copies the bytes before the port lock, and each queued frame counts 64 bytes of bookkeeping against the 32 MiB capacity, so many tiny frames cannot grow the queue without bound. - A writer that unwinds drops the frames after the one it lost (logged) and marks itself stopped, so the next input starts a new writer instead of queueing behind a writer that is gone. - A host connection the reader gave up on (Failed) refuses input at once, as the old path's failed write did. - The deadlines use checked_add (no new arithmetic_side_effects hits); the thread name fits Linux's 15 bytes. * fix(cmux-tui-core): the input port refuses only when the host reader ended, not while a Failed link still retries (#34544 review 2) A Failed link keeps retrying its host, and a successful retry installs the live replacement before the state leaves Failed; the old path wrote to it, the lane refused. The lane now refuses only when the reader ended (no connection will come back), where the old path's write fails too. The capacity holds two of the largest frames with their per-frame cost; the 26-key test also checks that no key is written twice. * fix(cmux-tui-core): a key to a failed host link is refused at once on the input port, as on the old path (#34544 review 3) When the reader moves a link to Failed it shuts the connection down (and keeps retrying); the old path's write then failed and `cmux send` exited non-zero, while the port took the key and only logged its failure. The reader now publishes InputRoute::Failed under the runtime lock when it shuts the connection down; the port refuses it with NotConnected, and the replacement's route (replace_runtime) is accepted as soon as it is installed. Also the 26-key test's tail loop is a while-let (clippy::while_let_loop). Test: a_key_to_a_failed_link_is_refused_until_a_live_route_is_published. * fix(cmux-tui-core): a failed reconnect completion on a failed link also makes the input port refuse (#34544 review 4) A retry that installed a replacement and then failed to complete it (terminal_host_reconnected false) shuts the replacement down while the link stays Failed; the route was still the shut replacement, so the port took keys and only logged them. That path now publishes InputRoute::Failed under the runtime lock too, as the Failed transition does. Not driven by a test: the hosted fixture's live reader thread reacts to the disconnect itself; the route behavior is covered by a_key_to_a_failed_link_is_refused_until_a_live_route_is_published. * test(cmux-tui-core): a frame sent after a key the input port holds must reach the host after it (R-DAEMON-STATE S1.6b red) The port counts the frames it took and finished and offers PortOrder::barrier; nothing calls it yet, so a direct frame (here an Activate under the runtime lock, as viewer sizes and the other direct writers send) passes a key the paused port still holds: expected red in a_frame_sent_after_a_key_the_port_holds_reaches_the_host_after_it. * fix(cmux-tui-core): every other frame to a hosted terminal's host goes after the keys its input port took (R-DAEMON-STATE S1.6b order) HostAttachment gets the terminal's PortOrder (set by the hosted constructor and replace_runtime, so it follows every connection change); each frame it writes besides the port's own (send, receipted input, viewer size, cell pixels, Kitty limits, control requests such as clear history and terminate, and the clipboard replier's replies) first waits until the port finished every frame it had taken (PortOrder::barrier, at most 2 s, as one frame write may block). The wait returns at once while the port's writer waits for a reconnect, since the install it waits for may need the runtime lock the caller holds. With the switch off the port takes nothing and the wait is one uncontended lock. The 9 test literals of HostAttachment take input_order: None. Test: a_frame_sent_after_a_key_the_port_holds_reaches_the_host_after_it (red at 8b9f34a). * test(cmux-tui-core): an automation write answers after the input port wrote it, with its frame's error (R-DAEMON-STATE S1.6b red) On the lane every hosted write answered when the port took the bytes, so the send handler, the creating-input flush (its give-back) and the resource input effect (its indeterminate mark) never saw a write error. Expected red: an_automation_write_answers_after_the_port_wrote_it (it answers while the port is paused) and an_automation_write_gets_the_error_of_its_frame (ok for bytes never written). * fix(cmux-tui-core): on the input lane only the person's keys answer before the write; every other write answers with its frame's result (R-DAEMON-STATE S1.6b) S1.6a answered every hosted write when the port took it, so the send handler for scripts, the creating-input flush (its give-back), the resource input effect (its indeterminate mark) and the image paste (its reservation) never saw a write error. Now Surface::write_bytes and write_paste answer when the port wrote the frame (InputReply::Written: the port reports each frame's result on a channel, bounded at 15 s); only the person's key (the send and send-key handlers, when the client is attached to the terminal it types into) passes InputReply::Accepted and returns at once. The port also keeps one reconnect wait for its queue: frames queued behind a host that does not come back fail together, not 5 s each. The give-back notice of the plan is dropped (DESIGN 2.6.1: the app reads it only on its control connection, and it never retried a failed key). Tests: an_automation_write_answers_after_the_port_wrote_it and an_automation_write_gets_the_error_of_its_frame (red at 189ba58); the_persons_keys_answer_when_taken_and_other_writes_when_written; the person-key tests pass InputReply::Accepted. * test(cmux-tui-core): the input port's reconnect and reply rules from review 1 of #35082 (R-DAEMON-STATE S1.6b red) Red: - a_frame_sent_after_a_replacement_install_waits_for_the_keys_the_port_held (must-fix 1): after replace_runtime and before the reader's stream notify, a direct frame on the new connection passes the key the port held. - a_later_host_loss_waits_for_its_own_reconnect (must-fix 2): the first loss's deadline outlives it, so a later loss's write fails at once. - a_write_whose_wait_ends_before_the_port_takes_it_is_never_written, a_write_whose_wait_ends_during_its_write_may_be_written and the_creating_input_flush_keeps_input_that_may_have_been_written (item 6): a timed-out write stays queued and is written later, and the flush gives back bytes that may still be written. Guards (pass before and after): a_frame_sent_while_the_port_waits_for_a_reconnect_does_not_wait_for_it, writes_queued_behind_a_lost_host_fail_together, receipted_input_sent_after_a_key_the_port_holds_reaches_the_host_after_it. an_automation_write_gets_the_error_of_its_frame waits for the port's count instead of sleeping 100 ms. Test seams: the port's reply and reconnect waits (test overrides), its reconnecting/accepted/queued counts, and the InputMayBeWritten marker with its classifier (nothing produces the marker yet). * fix(cmux-tui-core): a route publication ends the input port's reconnect wait; each frame waits for a reconnect from when the port took it; a timed-out write is cancelled or marked possibly written (#35082 review 1) - Must-fix 1: replace_runtime and input_route_failed call InputPort::route_changed. A port-owned route epoch ends the writer's reconnect wait and clears reconnecting under the port lock, so after the reader installs a live replacement the port writes the keys it holds to it before any other frame (before, the writer slept on the stream revision until the reader's notify after the VT replay, and the barrier skipped meanwhile). - Must-fix 2: each frame waits for a reconnect until its own deadline, HOST_INPUT_RECONNECT_WAIT after the port took it, as the old path's write waited from its call. The queue-wide reconnect_until outlived its loss, so a later loss's writes failed at once. Writes queued behind a host that does not come back still fail together. - Item 6: InputPort::write cancels a frame its waiter gave up on before the writer took it (the error says not written, and it never is); a wait that ends during the write answers InputMayBeWritten, and the creating-input flush does not give those bytes back (that typed them twice). - Item 5: the order barrier ends by the caller's own deadline (control requests, cell pixels). - Item 3: the order-wait comment states the bound; the clipboard replier uses the same deadline. - Item 8: the port and input docs describe the current rules. Tests (red at 2133c10): a_frame_sent_after_a_replacement_install_waits_for_the_keys_the_port_held, a_later_host_loss_waits_for_its_own_reconnect, a_write_whose_wait_ends_before_the_port_takes_it_is_never_written, a_write_whose_wait_ends_during_its_write_may_be_written, the_creating_input_flush_keeps_input_that_may_have_been_written. * test(cmux-tui-core): the review-1 port tests name Frame and input_may_be_written through the module scope (clippy unused_qualifications) lint (linux) at bbc44c2 (runs 41806402684, 41806407494) failed clippy -D warnings on four qualified paths in surface/tests/hosted.rs; both names are in scope through use super::*. * test(cmux-tui-core): a write waiting for a reconnect fails when the reader gives up (R-DAEMON-STATE S1.6c red, #35082 review 2) The reader's end, give-up and failed-reconnect paths now call PtyTerminalRuntime::reader_gave_up, which still only notifies stream_progress (today's behavior). The port's writer waits on its route epoch since #35082 review 1, so the wake does not reach it: a_write_waiting_for_a_reconnect_fails_when_the_reader_gives_up waits its full reconnect deadline (5 s) instead of failing at once. * fix(cmux-tui-core): the reader's end, give-up and the owner's detach end the input port's reconnect wait (R-DAEMON-STATE S1.6c, #35082 review 2) Since review 1 the port's writer waits on its own route epoch, so the reader's stream_progress wake no longer reached it: a Written write behind a host the reader gave up on failed only at its reconnect deadline (5 s), not at once as the old path's write did. InputPort::reconnect_ended moves the epoch with no new route; the writer checks the route and the terminal's state again and fails its frame (Failed state, ended reader, or owner detaching). - PtyTerminalRuntime::reader_gave_up (WakeInputOnReaderEnd, wait_or_fail's give-up, a failed reconnect completion) notifies the stream and ends the port's wait. - The four owner_detaching stores in shutdown.rs end the port's wait (only the port lock, under the runtime lock). Test: a_write_waiting_for_a_reconnect_fails_when_the_reader_gives_up (red at b6b0b37). * test(cmux-tui-core): a viewer size behind a held key does not wait and goes after it; the settle barrier ends by its deadline; a frame the writer lost may be written (R-DAEMON-STATE S1.6c part 2 red) Red: - a_viewer_size_behind_a_key_the_port_holds_goes_after_it_without_waiting (#35082 review 1 item 4): the viewer size waits up to 2 s in the order barrier (under the sizing lock on a key's turn), then passes the held key. - a_viewer_size_settle_ends_by_its_own_deadline_behind_a_held_frame (review 2 should-do 2): the attach's settle waits the barrier's 2 s, past its own 200 ms deadline. - a_write_the_writer_lost_after_taking_it_may_be_written (review 2 should-do 3): a frame the writer took and lost answers 'not written'. Test seams: hosted_surface_with (configure the fake host's attachment); InputPort::panic_after_take_for_test. * fix(cmux-tui-core): viewer-size frames go on the input port behind its frames, with no wait; the settle barrier ends by its deadline; a lost taken frame may be written (R-DAEMON-STATE S1.6c part 2) - #35082 review 1 item 4: a key that moves the grid sends a ViewerSize on its own turn under the global sizing lock (apply_terminal_grid -> resize -> send_viewer_size). It waited in the order barrier for up to 2 s behind the port's frames (a slow paste), holding the sizing lock, then went first. ViewerSize and ReleaseViewer now go on the port when it holds frames (PortOrder::enqueue_after_pending): ordered after them, with no waiter; a newer size replaces a queued one that is still the last frame, so sizes cannot pile up. With nothing pending (or the lane off: the port holds nothing) they are written directly, as before. - Review 2 should-do 2: begin_viewer_size_barrier takes the attach's deadline (direct_writer_until), so an attach with a 1 s settle no longer waits the barrier's 2 s. - Review 2 should-do 3: a Written frame whose writer took it and was lost (a panic during the write) answers InputMayBeWritten, so the creating-input flush keeps those bytes; one still queued answers not written. Tests (red at ac7ffa2): a_viewer_size_behind_a_key_the_port_holds_goes_after_it_without_waiting, a_viewer_size_settle_ends_by_its_own_deadline_behind_a_held_frame, a_write_the_writer_lost_after_taking_it_may_be_written. * test(cmux-tui-core): the reply-timeout tests take their frame on a running writer, not a new thread (#35082 review 2, should-do 4) a_write_whose_wait_ends_during_its_write_may_be_written and the_creating_input_flush_keeps_input_that_may_have_been_written needed the port's writer thread to start and take the frame within the 300 ms reply wait. running_paused_writer starts the writer on a key first and holds it paused; the next frame is taken at once by the running writer, and the reply wait is 2 s. * test(cmux-tui-core): the reader's real give-up and the daemon's shutdown detach end a write's reconnect wait (#35280 review, should-do 1) The S1.6c part-1 test called reader_gave_up by hand, so a call site that lost its wake turned no test red. These drive the real call sites: an exhausted TerminalHostReconnectBackoff's wait_or_fail, and Surface::disconnect_for_daemon_shutdown, each while a Written write waits for a reconnect. * docs(cmux-tui-core): the input port docs name reconnect_ended and state the lock rule the right way round (#35280 review, should-do 2) The port lock is a leaf: no holder of it waits for another lock, so the runtime lock's holders may take it. The shutdown.rs comments said the reverse. * fix(cmux-tui-core): a viewer size queued behind a held key no longer takes the port lock twice (S1.6c part 2) PortOrder::enqueue_after_pending computed its frame's reconnect deadline with reconnect_wait while it held the port lock, and in test builds reconnect_wait took that same std Mutex again to read the test override. The second lock on one thread deadlocks: the hosted focused runs at 10b577c, 67023f5 and 476e0bf hung for 40 minutes in a_viewer_size_behind_a_key_the_port_holds_goes_after_it_without_waiting ("has been running for over 60 seconds", run 41819499722, cancelled). reconnect_wait now reads the guard its caller holds and never locks; both callers (the port's enqueue and enqueue_after_pending) call it under the lock. Release builds never took the second lock (the override is cfg(test)), so this changes no shipped behaviour, only the deadlock the tests hit. * test(cmux-tui-core): a viewer size or release sent while the input port waits for a reconnect is the new host's last size frame (#35375 review, red) enqueue_after_pending writes a size directly while the port waits for a reconnect, but a size queued before the loss stays on the port. The direct write goes to the lost connection; after the install the port writes the stale queued size to the new host, so the host keeps 97x31 while the daemon's lease says 100x30 (or keeps a lease the daemon released), and a later send of the same lease writes nothing. Red: a_viewer_size_sent_during_a_reconnect_wait_is_the_new_hosts_last_size and a_viewer_size_release_during_a_reconnect_wait_is_the_new_hosts_last_size_frame read the replacement host's size frames and require the newest last. * fix(cmux-tui-core): a viewer size sent while the input port waits for a reconnect goes on the port, so the new host's last size is the newest (#35375 review) enqueue_after_pending returned false while the port's writer waited for a reconnect, so the caller wrote the size directly, to the lost connection, while a size queued before the loss stayed on the port. After the install the port wrote that stale size to the new host after the install's replay of the newest lease: the host kept the old size (or a lease the daemon had released), and a later send of the same lease wrote nothing. It now queues whenever the port holds frames, during a reconnect wait too: the newest size replaces a trailing queued one, so the last size frame the new host reads is the newest lease or release. The barrier's reconnect skip exists because its callers hold the runtime lock the install needs; this enqueue never waits, so it needs no skip. Tests (red at 5ae437a): a_viewer_size_sent_during_a_reconnect_wait_is_the_new_hosts_last_size, a_viewer_size_release_during_a_reconnect_wait_is_the_new_hosts_last_size_frame. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
lawrencecchen
added a commit
that referenced
this pull request
Nov 5, 2026
…_RDS_INPUT=0 keeps the old path for one nightly (R-DAEMON-STATE S1.7) (#35413) * test(cmux-tui-core): the person's key waits for no shared lock (R-DAEMON-STATE S1 red) A key line from an attached view goes through the real connection path (the line handler, the connection's dispatcher, the send handler) while another thread holds one shared lock for 300 ms. #31940 took Mux.state off the key; the key still waits for the client registry lock (five takes per line), the link peers lock, the host proxy authors lock, the surface admission lock, the creating-input lock, the client sizing lock and the activity lock. Their other holders are every connect, attach, resize and request of every client. Mux.state and the workspace registry rows are controls (pass on the base). Two keys go in a row, so a lock taken after the PTY write (sizing, activity) shows as a delay of the second key on the connection's dispatcher. Test-only hold hooks (cfg(test)) in creating_input.rs, terminal_sizing.rs, activity.rs and agent_session_attach.rs. No product change. Design: handoff/rewrites/R-DAEMON-STATE/DESIGN.md section 2.6 (stage S1). * test(cmux-tui-core): a key waits for no client registry lock (R-DAEMON-STATE S1 step 1 red) The previous commit's census test held nine shared locks in turn; this commit splits it by fix step. Step 1 is the client registry lock (ClientRegistry.state), which a key line takes about nine times (handoff checks, remote-client and remote-device checks, the request's actor, the kept terminal, the reply's incognito filter) while every connect, disconnect, attach and detach of every client holds it. The test turns on the stage switch CMUX_TUI_RDS_INPUT (rds_switch.rs, inert in this commit) and drives two keys through the real connection path while another thread holds the lock for 300 ms. A second test keeps Mux.state and the workspace registry off the key on the new path (#31940). The hold hooks of the other locks leave with this commit; they come back with their fix steps. * fix(cmux-tui-core): the person's key reads the client registry's published facts, never its lock (R-DAEMON-STATE S1 step 1) A key line took the client registry lock (ClientRegistry.state) about nine times: the daemon handoff checks, the remote-client and remote-device checks, the request's actor, the kept terminal of the key and the reply's incognito filter. Every connect, disconnect, attach, detach and capability change of every client holds that lock, so an agent storm held back the person's keys (red at 2472251831862: 300.4 ms behind a 300 ms hold; STATUS 9131). The registry is now a Published value (published.rs): writers still serialize on its mutex, and a guard that changed the value publishes an immutable ClientFacts projection (handoff state; per connection its transport, remote device flag, base actor and kept input terminals) with arc-swap when it drops, under the mutex, so projections follow the order of the changes. With the stage switch CMUX_TUI_RDS_INPUT=1, the line and key paths read the projection with one atomic load: daemon_handoff_in_progress/committed, transport_of (so is_remote_client), is_remote_device (refuse_line, line_to_recheck, prune_reply), connection_actor and attached_input_surface. A poisoned registry still fails closed (the projection comes in the error, as Mutex::lock gives its guard). With the switch off nothing changes. A projection costs O(connections) under the registry lock per change, bounded by the 64-connection cap; stage S2 makes it O(changed). arc-swap 1.9 is already in the lockfile and the shipped notices (through iroh-dns); cmux-tui-core now names it. Tests: rds_input_lane (a key waits for no client registry lock; the session and workspace registry locks stay off; a respawn under the same id sends the next key to the new shell), published.rs unit tests (a change publishes on drop, a read publishes nothing, a reader never waits for a writer, a poisoned value fails closed). The two remote-relay poison tests poison through the new type. * fix(cmux-tui-core): the key reads the session's published routes; one stage switch per mux (R-DAEMON-STATE S1 step 1) The hosted run of f1ea1b69709 (https://github.com/manaflow-ai/cmux/actions/runs/41655768031) still failed both lane tests: the key waited 300 ms for the client registry and, new, for Mux.state. Two causes: - The key still used the kept terminal of #31940. That cache is validated by process-wide route generations, and cargo test runs the tests of one binary as threads of one process: another test's mux moved the same generation slot (ids start at 1 in every mux), the kept terminal looked stale, and the key fell back to the locked lookup and then to remember_attached_input (registry lock). The stage switch was a process-wide static too. - So the lane must not depend on a cache with process-wide invalidation at all. Now, with CMUX_TUI_RDS_INPUT=1: - The session's three routing tables (surfaces, terminal_catalog, terminal_catalog_by_runtime) are copy-on-write behind an Arc. A state guard that changed one publishes a RouteSnapshot (three Arc clones) with arc-swap when it drops, under the lock, so publications follow the changes; the snapshot it replaced is freed after the lock is released. Mux::surface reads the snapshot with one atomic load. A respawn, close, move, rollback or projection is seen once its guard is released, as a reader that waited for the lock would see it. Nothing is kept per client, so nothing is invalidated. - input_surface on the lane: the published routes give the terminal, the registry's published facts say whether the client types into it (types_into: the person's input QoS mark). attached_input_surface and remember_attached_input stay only on the old path. - The switch is per mux (rds_switch::InputLaneSwitch, read once from the environment when the mux is built; Mux::set_input_lane_for_test in tests), and it also decides whether anything is published: off, the registry builds no projection and the state holds no route snapshot, so a route write never copies a shared table and the off arm costs what the base costs. Cost on the lane: a guard's first route write copies that table once (O(routes)), bounded by one copy per changing guard; State::clone no longer deep-copies the three tables. Stage S2 makes both O(log n). * test(cmux-tui-core): a key waits for no surface admission lock, with an old-path control (R-DAEMON-STATE S1 step 3 red) Every request of every connection reserves its retained bytes from one daemon-wide budget (ServerSurfaceOperationAdmission) under one mutex before it is queued, and releases them under it after it ran. The S1 census measured a key waiting 300.3 ms behind a 300 ms hold of that lock (hosted run 41648095321). The lane test now has a control: the same two keys with stage S1 off must wait for the held lock, so a lane result can never come from a hold that does not reach the key path (a vacuous green). The control is added to the client registry test too. A test-only hook (cfg(test)) holds the admission lock. Red here: with the lane on, the key still reserves through the mutex. * fix(cmux-tui-core): the surface admission budget reserves with atomics on the input lane (R-DAEMON-STATE S1 step 3) Every request of every connection reserves its retained bytes (and every clear-history its worker) from one daemon-wide budget before it is queued and releases after it ran. Under one mutex, a key waited for any other connection's reservation or release (red at 5d0e4a0e2c1: 300 ms behind a 300 ms hold). With CMUX_TUI_RDS_INPUT=1 the two counters are atomics with compare-and-swap reservations (fetch_update: add only while the sum stays within the capacity; release saturates at zero), so nothing waits. The limits and the refusals are the same as the mutex arm's: a worker is refused at 16 reserved, bytes when the sum would pass 16 MiB. Each permit releases to the arm it reserved from. With the switch off the mutex arm runs as before; it goes with the switch when the stage passes (S1.8). The arm is fixed when the mux is built (ServerSurfaceOperationAdmission::new(publish_routes)); tests set it with Mux::set_input_lane_for_test before any request. The two scheduler tests that counted reserved workers through the mutex read reserved_workers() (both arms). * test(cmux-tui-core): a key waits for neither the link peers nor the proxy authors lock (R-DAEMON-STATE S1 step 2 red) Two more shared locks on the person's key line (DESIGN.md Appendix A.2 steps 3, 7, 12 and 13): the link peers map (RemoteRelayState.peers: the remote-client check and the request's actor) and the host proxy authors (AgentSessions.proxy_authors: the proxy-link check on the reader thread). The S1 census measured a full 300 ms wait on each (hosted run 41648095321: 300.5 ms and 300.4 ms). The test holds each lock 300 ms on another thread while two keys go through the real line path. It first proves the hold reaches the old path (stage switch off: the key waits), then expects the stage's path not to wait. Red on this commit: the stage does not publish either value yet. * test(cmux-tui-core): a change cut short by a panic is never published (R-DAEMON-STATE S1 red) A published value's guard publishes its projection in Drop::drop, which also runs while a panic unwinds, before its MutexGuard field drops and marks the mutex poisoned. A writer that panics in the middle of a change so publishes the half-made value, and in the moment between the publication and the poisoning a reader gets it as a good value: a fail-open window for readers that fail closed on a poisoned value (transport_of, peer_checked). Red on this commit: the test sees the half-made value 9 in the poisoned projection. * test(cmux-tui-core): a reservation unshares a published route map; a panicked change publishes no routes (R-DAEMON-STATE S1 red) Two defects the #34014 review found on the input lane (both only with CMUX_TUI_RDS_INPUT on): - RouteTable::try_reserve returns early when the map has room, also when a published route snapshot shares the map. The insert after the durable commit then copies the whole table (Arc::make_mut), an allocation that ResourceMutationPlan::apply forbids ("Plan builders reserve all needed capacities before returning"). - The session state guard publishes the routes of a change that a panic cut short: Drop::drop runs during the unwind, before the MutexGuard field poisons the mutex, so the lane routes by half-made tables. Red on this commit: the insert changes the map's pointer, and the routes snapshot is replaced. * fix(cmux-tui-core): link peers and host proxy authors are published; the S1.1 review follow-ups (R-DAEMON-STATE S1 step 2) A person's key line asked two more shared locks (DESIGN.md Appendix A.2 steps 3, 7, 12, 13): the link peers map (the remote-client check and the request's actor) and the host proxy authors (the proxy-link check on the reader thread). Every remote bind, disconnect and revocation sweep holds the first; every proxy bind and every client's disconnect holds the second. - published.rs moves to cmux-tui-util (daemon primitives, no daemon state) so cmux-tui-remote-access can use it. RemoteRelayState.peers becomes a Published copy of the records; peer_checked reads it with no lock when the mux's stage switch is on (Published::publishing), and a poisoned map still fails closed. AgentSessions.proxy_authors publishes the same way; proxy::author reads it. A disconnect of a client without a record publishes nothing. - Review follow-ups of #34014 (review comment 6612546773): - RouteTable::try_reserve copies a map a published snapshot shares, before the durable commit, so the insert after the commit allocates nothing (ResourceMutationPlan::apply). - A change a panic cuts short is never published, by the state guard (routes) or by a PublishGuard: Drop skips the publication when a panic began while the guard held the lock (std's own poisoning test), so no reader sees half-made tables, not even before the mutex is poisoned. - A PublishGuard frees the projection it replaced after the unlock (as the state guard does). - The two poison tests of the relay run on both arms. - ClientFacts and types_into move to server/client_registry/facts.rs (god-file warning). Same switch, CMUX_TUI_RDS_INPUT, default off: with it off nothing is published and every read takes its lock as on the base. * test(cmux-tui-core): a key waits for no creating-input lock, and for no session lock behind one (R-DAEMON-STATE S1 step 4 red) Every key to a live terminal first asks whether creating input of its terminal must go first (wait_for_creating_input_flush, DESIGN.md Appendix A.2 step 16). It takes the daemon-wide creating-input lock even when no terminal is being created, and that lock's holders (begin_creating_input, knows_terminal_input_target, finish_creating_input) look up the live terminal under it, which takes the session lock. The S1 census measured a full 300 ms wait on the creating-input lock (hosted run 41648095321: 300.3 ms). Two holders, each with an old-path control: the creating-input lock itself, and the session lock held while a reader waits for it under the creating-input lock (with another terminal being created, so the key cannot skip the lock). Red on this commit: the stage does not change these paths yet. * fix(cmux-tui-core): a key skips the creating-input lock while nothing is creating; the lock becomes a leaf (R-DAEMON-STATE S1 step 4) On the input lane (CMUX_TUI_RDS_INPUT, per mux, default off): - CreatingInputs keeps the number of creating terminals in an atomic, stored under its lock after every insert and removal (Release). A key reads it (Acquire) and returns at once when it is zero: no queue can be ahead of it, because a terminal's queue is added before the terminal goes live and removed only after every queued byte was written to it. - The creating-input lock's holders look up the live terminal in the published routes instead of under the session lock, so no holder waits for the session lock while it holds the creating-input lock, and a key that takes it never waits behind one. The routes carry the host-id catalog index for this: terminal_catalog_by_host becomes a copy-on-write route table in the RouteSnapshot, and one lookup (route_table::live_terminal) serves the session state and the snapshot. A commit publishes its routes before it releases the session lock, and finish runs after the commit, so begin, knows and finish give the answers they gave under the session lock. Off, every path takes the locks it took on the base; the by-host index is copy-on-write on both arms, as the other three route tables are since step 1. * test(cmux-tui-core): the surface budget's limits on both arms; a lock control fails on a lost key (#34146 review follow-ups) The #34146 review (comment 6613345630) noted: - Every budget test used the mutex arm, so an atomic arm that admitted everything would have passed. The two mux-wide limit tests now run on both arms, and a new test checks the same limits on both: the 17th worker, the whole byte budget, one byte over, zero bytes, a sum that overflows, and released reservations coming back. - The old-path control passed when a key never arrived (Duration::MAX >= BUDGET); it now fails on a lost key. - ServerSurfaceOperationState and the admission's mutex are private (nothing outside the file reads them), and the doc says which requests reserve (pings, identify, v2 resource lines and reader-thread frame handlers do not). * fix(cmux-tui-core): drop an unnecessary path qualification in a test (clippy unused_qualifications) The lint (linux) job's clippy -D warnings failed on mux/tests/signaled_mutex.rs:120 (TerminalPublicId is in scope); the #34174 review found it behind the base's acpmux red in the same job. * test(cmux-tui-core): a poisoned session state never routes by the published tables (R-DAEMON-STATE S1 red) The #34171 review noted: after a panic poisons Mux::state, the next guard that recovers the poison may publish the half-made route tables, and the lane's Mux::surface reads the published routes with no poison check, where the old path fails (it unwraps the lock). Red on this commit: with the lane on, a surface read after the poisoning returns from the published routes instead of failing. The relay poison tests now poison published values through a helper in the test file. * fix(cmux-tui-core): the lane's surface read takes the old path on a poisoned state; the poison helper is test code (#34171 review) Review comment 6614044759 (FIX at 8f7d40500e8): - published.rs: poison_for_test moved out of the test module with its panic! respelled as panic_any, which hides a hit from the crash ratchet's count. It is test code again: a private helper in the published tests and one in the relay tests, with plain panic!. - Mux::surface on the lane reads the published routes only while the session state is not poisoned; a poisoned state takes the old read and fails as it does (StateMutex::is_poisoned). A guard that recovers the poison may publish the half-made tables of the change that panicked. - Published::load debug-asserts that the value publishes: only a publishing value has a current projection. - The module doc says what load gives on a poisoned value; the caller decides whether to fail closed. The unused_qualifications error at mux/tests/signaled_mutex.rs:120 is fixed in 10f16693a03. * fix(cmux-tui-core): the creating-input live check reads the routes only on an unpoisoned state; its comment states the lane's linearization (#34174 review) Review comment 6613998735 (SHIP, notes): the begin_creating_input comment still said the check takes the state under the creating lock; on the lane it reads the published routes, and its answer is one the old path can give (a commit in progress has not made the id live, as when the old path takes the state before it). The lane's live lookup now takes the old read on a poisoned state, as Mux::surface does since bb0265f43d5. * test(cmux-tui-core): a key waits for neither the sizing nor the activity lock; a key takes the grid back after another view's click (R-DAEMON-STATE S1 step 5 red) After its PTY write a key notes activity in the terminal's sizing (the newest view sets the grid under the default policy): it takes the daemon-wide client sizing lock twice (the note and the size-state publication), then the VM activity stream's lock. Every attach, detach, resize and size report of every terminal holds the sizing lock, and a PTY resize runs under it. The S1 census measured a full 300 ms wait on each (hosted run 41648095321: 300.2 and 300.3 ms). The first test holds each lock 300 ms (with an old-path control each). The second keeps the fix honest: two views of one terminal, the default policy; the other view's focus click takes the grid, and the next key from the typing view must take it back, on both arms. Red on this commit: the first test (the stage does not change these paths yet). * fix(cmux-tui-util): a test dereferences the published u64 once (E0614 in bb0265f43d5) * fix(cmux-tui-core): a key skips its sizing note when its view is the settled newest; the person's input time is an atomic (R-DAEMON-STATE S1 step 5) On the input lane (CMUX_TUI_RDS_INPUT, per mux, default off): - Each terminal's sizing entry publishes, on the terminal's runtime (shared by every placement), the view a key may skip its sizing note for: the sole newest participant, a client's own view, only while the entry is settled (no grid hold, no handheld with a viewport, and the engine holds the grid or the applied grid and participants are its decision). Noting activity for that view changes no decision under any policy, and the size state carries no activity, so the key compares (client, placement) with one atomic load and takes no sizing lock. - The publication is automatic: Mux::client_sizing is a SizingMutex whose guard republishes, on drop and under the lock, each entry its holder read or wrote (SizingEntries marks them; an apply changes Cells through a shared borrow, so reads count). A removed entry clears its slot; a change a panic cuts short publishes nothing. - Another view's activity makes that view the newest and republishes, so the next key from the first view takes the slow path once and takes the grid back (the second test pins this on both arms). - ActivityStream::note_user_input stores the time in an atomic; it takes the stream lock only while a subscriber exists, once per event the worker sends (a wake_pending flag the worker clears before it reads the time). - TerminalSizingEngine::sole_latest (cmux-terminal-sizing) with a unit test; AppliedSeat::of shares the seat rule between the apply and the settled check; set_workspace_size_policy reads its state before the note (no index). Off, every key takes the sizing and activity locks as on the base; the marks and the guard check one bool. * fix(cmux-tui-core): the sizing test hook went in above another test hook's cfg(test) (dead code and 2 expect hits in production) dfe81bff4d8 put hold_client_sizing_for_test between pin_latest_size_policy_for_test's doc and its #[cfg(test)], so pin_latest_size_policy_for_test compiled into production: clippy -D warnings dead_code, and the panic ratchet's cmux-tui-core expect_used 175 > 173 (hosted lint job 126202464363). * fix(cmux-tui-core): the lane's surface read loads the routes before its poison check; two test doc comments on their items (#34171 review 2) Review comment 6615927915 (SHIP at 0fb7c43a810, notes): Mux::surface checked is_poisoned() and then loaded the routes; between the two another thread could poison the state and a third, recovering the poison, publish half-made tables. Poison never clears on the session state, so routes loaded before an unpoisoned check were published before any poisoning: load first, then check. Two doc comments in remote_relay/tests.rs sat above the wrong item. * test(cmux-tui-core): a key does not wait for another writer's blocked host socket (R-DAEMON-STATE S1 step 6 red) A key to a hosted terminal holds the terminal's runtime lock and its HostAttachment.writer across a blocking write to the host socket (surface/input.rs, attachment.rs send_host_frame). Another client's large paste to the same terminal holds both for its whole write, bounded only by the socket's send timeout, so the person's key waits for automation's work on the same terminal (DESIGN.md section 2.6.1, Appendix A.2 steps 20-22). The test pastes 8 MiB while the fake host reads nothing for 600 ms, then sends one key. A control first proves the old path waits (the key takes at least half the stall); the lane must return the key in < 100 ms, and the host must then read the paste before the key, and the key once. Red on this commit: the lane has no input port yet. * fix(cmux-tui-core): a person's key to a hosted terminal goes to the terminal's input port and never waits for a socket write (R-DAEMON-STATE S1 step 6) On the input lane (CMUX_TUI_RDS_INPUT, per mux, default off), interactive input to a hosted terminal (Surface::write_bytes, Surface::write_paste) goes to the terminal's own input port (surface/input_port.rs): one queue per terminal, drained in order by the terminal's writer thread (terminal-input-port, started on the first input, ended after 30 s idle). The request returns once the port took the bytes; it takes neither the terminal's runtime lock nor the host writer, so another writer of the same terminal (a large paste, receipted input, a cell-pixel change waiting for its acknowledgement) and a host that does not read its socket no longer hold the person's key or the connection's dispatcher. - The writer writes on the route of the current host connection (HostInputRoute: the connection's writer), which the terminal publishes whenever its runtime changes: the hosted and exited constructors, and PtyTerminalRuntime::replace_runtime, which the reconnect install and the host exit now use. So the writer never takes the runtime lock either. - A lost connection is retried as write_bytes did (cx-6so.49), on the port's thread: wait for the reader's next stream change, write to the new host, until HOST_INPUT_RECONNECT_WAIT. - A full port (32 MiB queued) refuses at once (terminal.input_busy, WouldBlock); a write larger than one host frame is refused as before; the exited final screen drops input as before. - Input the port took and then could not write is lost as a failed write was, with a log line: the request was already answered (the reply now means "accepted by the terminal's port", DESIGN.md 2.6.1). - Not in this step (S1.6b, before the default flip): the connection's other frames still write directly, so while the port is behind they can go before a key it still holds. Off, every write takes the path it took on the base; the route is published on both arms (one Arc per change). * test(cmux-tui-core): after a respawn, keys skip the sizing lock again; the lock tests report a viewport (#34261 review red) Review comment 6619473751 (FIX at 846d412ccf7): 1. A respawn under the same runtime id keeps the sizing entry, which keeps the dead runtime's fast-path slot: the new runtime's slot is never published, so every later key takes Mux::client_sizing twice. The new test respawns the typing terminal, sends one key, then holds the sizing lock: the next keys must not wait. Red on this commit. 2. The lock tests' view never reported a size, so the engine held the grid and the skip went only through that branch. The typing view now reports 100x30 as the app's view does, so its keys skip through the settled-grid check. * fix(cmux-tui-core): a sizing entry follows its terminal across a respawn; the input time never goes back (#34261 review) Review comment 6619473751 (FIX at 846d412ccf7): 1. A respawn under the same runtime id keeps the sizing entry but builds a new terminal runtime with a new fast-path slot. TerminalSizingEntry::follow_runtime_of re-points the entry at the slot of the runtime its keys reach, under the sizing lock, and clears the old slot: on the key's slow path (note_terminal_activity_of has the key's surface) and on every terminal_sizing_entry call (attach, claim, size report). The red test after_a_respawn_keys_wait_for_no_sizing_lock_again (4bf5af5c6ae) pins it. 3. The entry takes or follows a slot only from a surface whose runtime id is the entry's, so two entries never share one slot. 4. The lock-free input time is fetch_max, so a key preempted after its clock read never moves it back. 7. The poison comment says what happens: a later holder that recovers the poison republishes what it touches. Lock order is unchanged: terminal_sizing_entry now looks the surface up on each call (it did at creation, and apply_terminal_grid does), under the sizing lock, the established sizing-then-session order. * fix(cmux-tui-core): name the input port module without super:: where surface::* is in scope (clippy unused_qualifications) * test(cmux-tui-core): a settled size-state read takes the session lock once with the switch off (#34261 review 2 red) The sizing entry looks its surface up on every access since 5fe770df4ff, so with the switch off a size-state read takes the session lock twice (once under the sizing lock): expected red at this commit (lane off: 2, not 1). * fix(cmux-tui-core): a sizing entry looks its surface up only when it is made; the respawn follow runs only on the lane (#34261 review 2) 5fe770df4ff looked the surface up on every sizing-entry access, so with the switch off (the default) every attach, size report, claim and size-state read took the session lock under the sizing lock. The surface is looked up only to make the entry again; a respawn's new runtime is followed by its first key's sizing note, which holds the surface, and only while the switch is on. An unchanged fast path is not stored again. The respawn test pins the old slot's clear. * fix(cmux-tui-core): the input port's order test can fail; the port copies outside its lock, counts each frame, recovers from a writer panic and refuses a failed link (#34544 review) - The test compared two Options (None < Some), so a key that passed the paste still passed; both positions are required now. A new test sends 26 keys on the lane and checks the host reads each once, in order. - enqueue copies the bytes before the port lock, and each queued frame counts 64 bytes of bookkeeping against the 32 MiB capacity, so many tiny frames cannot grow the queue without bound. - A writer that unwinds drops the frames after the one it lost (logged) and marks itself stopped, so the next input starts a new writer instead of queueing behind a writer that is gone. - A host connection the reader gave up on (Failed) refuses input at once, as the old path's failed write did. - The deadlines use checked_add (no new arithmetic_side_effects hits); the thread name fits Linux's 15 bytes. * fix(cmux-tui-core): the input port refuses only when the host reader ended, not while a Failed link still retries (#34544 review 2) A Failed link keeps retrying its host, and a successful retry installs the live replacement before the state leaves Failed; the old path wrote to it, the lane refused. The lane now refuses only when the reader ended (no connection will come back), where the old path's write fails too. The capacity holds two of the largest frames with their per-frame cost; the 26-key test also checks that no key is written twice. * fix(cmux-tui-core): a key to a failed host link is refused at once on the input port, as on the old path (#34544 review 3) When the reader moves a link to Failed it shuts the connection down (and keeps retrying); the old path's write then failed and `cmux send` exited non-zero, while the port took the key and only logged its failure. The reader now publishes InputRoute::Failed under the runtime lock when it shuts the connection down; the port refuses it with NotConnected, and the replacement's route (replace_runtime) is accepted as soon as it is installed. Also the 26-key test's tail loop is a while-let (clippy::while_let_loop). Test: a_key_to_a_failed_link_is_refused_until_a_live_route_is_published. * fix(cmux-tui-core): a failed reconnect completion on a failed link also makes the input port refuse (#34544 review 4) A retry that installed a replacement and then failed to complete it (terminal_host_reconnected false) shuts the replacement down while the link stays Failed; the route was still the shut replacement, so the port took keys and only logged them. That path now publishes InputRoute::Failed under the runtime lock too, as the Failed transition does. Not driven by a test: the hosted fixture's live reader thread reacts to the disconnect itself; the route behavior is covered by a_key_to_a_failed_link_is_refused_until_a_live_route_is_published. * test(cmux-tui-core): a frame sent after a key the input port holds must reach the host after it (R-DAEMON-STATE S1.6b red) The port counts the frames it took and finished and offers PortOrder::barrier; nothing calls it yet, so a direct frame (here an Activate under the runtime lock, as viewer sizes and the other direct writers send) passes a key the paused port still holds: expected red in a_frame_sent_after_a_key_the_port_holds_reaches_the_host_after_it. * fix(cmux-tui-core): every other frame to a hosted terminal's host goes after the keys its input port took (R-DAEMON-STATE S1.6b order) HostAttachment gets the terminal's PortOrder (set by the hosted constructor and replace_runtime, so it follows every connection change); each frame it writes besides the port's own (send, receipted input, viewer size, cell pixels, Kitty limits, control requests such as clear history and terminate, and the clipboard replier's replies) first waits until the port finished every frame it had taken (PortOrder::barrier, at most 2 s, as one frame write may block). The wait returns at once while the port's writer waits for a reconnect, since the install it waits for may need the runtime lock the caller holds. With the switch off the port takes nothing and the wait is one uncontended lock. The 9 test literals of HostAttachment take input_order: None. Test: a_frame_sent_after_a_key_the_port_holds_reaches_the_host_after_it (red at 8b9f34a9be8). * test(cmux-tui-core): an automation write answers after the input port wrote it, with its frame's error (R-DAEMON-STATE S1.6b red) On the lane every hosted write answered when the port took the bytes, so the send handler, the creating-input flush (its give-back) and the resource input effect (its indeterminate mark) never saw a write error. Expected red: an_automation_write_answers_after_the_port_wrote_it (it answers while the port is paused) and an_automation_write_gets_the_error_of_its_frame (ok for bytes never written). * fix(cmux-tui-core): on the input lane only the person's keys answer before the write; every other write answers with its frame's result (R-DAEMON-STATE S1.6b) S1.6a answered every hosted write when the port took it, so the send handler for scripts, the creating-input flush (its give-back), the resource input effect (its indeterminate mark) and the image paste (its reservation) never saw a write error. Now Surface::write_bytes and write_paste answer when the port wrote the frame (InputReply::Written: the port reports each frame's result on a channel, bounded at 15 s); only the person's key (the send and send-key handlers, when the client is attached to the terminal it types into) passes InputReply::Accepted and returns at once. The port also keeps one reconnect wait for its queue: frames queued behind a host that does not come back fail together, not 5 s each. The give-back notice of the plan is dropped (DESIGN 2.6.1: the app reads it only on its control connection, and it never retried a failed key). Tests: an_automation_write_answers_after_the_port_wrote_it and an_automation_write_gets_the_error_of_its_frame (red at 189ba5886e5); the_persons_keys_answer_when_taken_and_other_writes_when_written; the person-key tests pass InputReply::Accepted. * test(cmux-tui-core): an activity subscriber hears the person's keys on both arms; the sizing-note skip sets the grids the old path sets (R-DAEMON-STATE S1.7 prerequisites) The two test gaps the #34261 review named before the default flip: - an_activity_subscriber_hears_the_persons_keys_on_both_arms: the snapshot has a key typed before it, the next key is an event at once, a key inside the one-second spacing comes in the trailing event with a newer time. - the_sizing_note_skip_sets_the_grids_the_old_path_sets: for the latest, smallest, largest and fixed policies, with a desktop or a phone as the other view (keys from both, the other view leaving: a phone starts the departed-grid hold), and with a relay's phone sub-view, the grid after every step is the same on the lane and the old path. * test(cmux-tui-core): the input lane is on unless CMUX_TUI_RDS_INPUT=0 turns it off (R-DAEMON-STATE S1.7 red) The switch is read through InputLaneSwitch::from_value so a test can check the default without the process environment. Red: with no value the lane is still off. * fix(cmux-tui-core): the input lane is the default; CMUX_TUI_RDS_INPUT=0 keeps the old path for one nightly (R-DAEMON-STATE S1.7) The S1 typing A/B (STATUS 11230) met A4 with the lane on: the daemon part of a key p99 1.56-2.29 ms, max 3.50-3.75 ms; with it off max 11.2-14.1 ms (the lock waits S1 removes). Now the lane is on unless the daemon's environment (or the app knob CMUX_NEXT_TUI_RDS_INPUT) sets 0. S1.8 deletes the old path and the switch after one nightly. The kept-terminal tests of the old path (server/tests/input_lane.rs) pin the switch off, so the fallback keeps its coverage while it ships; every other test now runs on the lane. Test: rds_switch::tests::the_input_lane_is_on_unless_the_environment_turns_it_off (red at 9674a79fd8f). * test(cmux-tui-core): the input port's reconnect and reply rules from review 1 of #35082 (R-DAEMON-STATE S1.6b red) Red: - a_frame_sent_after_a_replacement_install_waits_for_the_keys_the_port_held (must-fix 1): after replace_runtime and before the reader's stream notify, a direct frame on the new connection passes the key the port held. - a_later_host_loss_waits_for_its_own_reconnect (must-fix 2): the first loss's deadline outlives it, so a later loss's write fails at once. - a_write_whose_wait_ends_before_the_port_takes_it_is_never_written, a_write_whose_wait_ends_during_its_write_may_be_written and the_creating_input_flush_keeps_input_that_may_have_been_written (item 6): a timed-out write stays queued and is written later, and the flush gives back bytes that may still be written. Guards (pass before and after): a_frame_sent_while_the_port_waits_for_a_reconnect_does_not_wait_for_it, writes_queued_behind_a_lost_host_fail_together, receipted_input_sent_after_a_key_the_port_holds_reaches_the_host_after_it. an_automation_write_gets_the_error_of_its_frame waits for the port's count instead of sleeping 100 ms. Test seams: the port's reply and reconnect waits (test overrides), its reconnecting/accepted/queued counts, and the InputMayBeWritten marker with its classifier (nothing produces the marker yet). * fix(cmux-tui-core): a route publication ends the input port's reconnect wait; each frame waits for a reconnect from when the port took it; a timed-out write is cancelled or marked possibly written (#35082 review 1) - Must-fix 1: replace_runtime and input_route_failed call InputPort::route_changed. A port-owned route epoch ends the writer's reconnect wait and clears reconnecting under the port lock, so after the reader installs a live replacement the port writes the keys it holds to it before any other frame (before, the writer slept on the stream revision until the reader's notify after the VT replay, and the barrier skipped meanwhile). - Must-fix 2: each frame waits for a reconnect until its own deadline, HOST_INPUT_RECONNECT_WAIT after the port took it, as the old path's write waited from its call. The queue-wide reconnect_until outlived its loss, so a later loss's writes failed at once. Writes queued behind a host that does not come back still fail together. - Item 6: InputPort::write cancels a frame its waiter gave up on before the writer took it (the error says not written, and it never is); a wait that ends during the write answers InputMayBeWritten, and the creating-input flush does not give those bytes back (that typed them twice). - Item 5: the order barrier ends by the caller's own deadline (control requests, cell pixels). - Item 3: the order-wait comment states the bound; the clipboard replier uses the same deadline. - Item 8: the port and input docs describe the current rules. Tests (red at 2133c10ae2f): a_frame_sent_after_a_replacement_install_waits_for_the_keys_the_port_held, a_later_host_loss_waits_for_its_own_reconnect, a_write_whose_wait_ends_before_the_port_takes_it_is_never_written, a_write_whose_wait_ends_during_its_write_may_be_written, the_creating_input_flush_keeps_input_that_may_have_been_written. * test(cmux-tui-core): the review-1 port tests name Frame and input_may_be_written through the module scope (clippy unused_qualifications) lint (linux) at bbc44c24ae4 (runs 41806402684, 41806407494) failed clippy -D warnings on four qualified paths in surface/tests/hosted.rs; both names are in scope through use super::*. * test(cmux-tui-core): a write waiting for a reconnect fails when the reader gives up (R-DAEMON-STATE S1.6c red, #35082 review 2) The reader's end, give-up and failed-reconnect paths now call PtyTerminalRuntime::reader_gave_up, which still only notifies stream_progress (today's behavior). The port's writer waits on its route epoch since #35082 review 1, so the wake does not reach it: a_write_waiting_for_a_reconnect_fails_when_the_reader_gives_up waits its full reconnect deadline (5 s) instead of failing at once. * fix(cmux-tui-core): the reader's end, give-up and the owner's detach end the input port's reconnect wait (R-DAEMON-STATE S1.6c, #35082 review 2) Since review 1 the port's writer waits on its own route epoch, so the reader's stream_progress wake no longer reached it: a Written write behind a host the reader gave up on failed only at its reconnect deadline (5 s), not at once as the old path's write did. InputPort::reconnect_ended moves the epoch with no new route; the writer checks the route and the terminal's state again and fails its frame (Failed state, ended reader, or owner detaching). - PtyTerminalRuntime::reader_gave_up (WakeInputOnReaderEnd, wait_or_fail's give-up, a failed reconnect completion) notifies the stream and ends the port's wait. - The four owner_detaching stores in shutdown.rs end the port's wait (only the port lock, under the runtime lock). Test: a_write_waiting_for_a_reconnect_fails_when_the_reader_gives_up (red at b6b0b37f3b0). * test(cmux-tui-core): a viewer size behind a held key does not wait and goes after it; the settle barrier ends by its deadline; a frame the writer lost may be written (R-DAEMON-STATE S1.6c part 2 red) Red: - a_viewer_size_behind_a_key_the_port_holds_goes_after_it_without_waiting (#35082 review 1 item 4): the viewer size waits up to 2 s in the order barrier (under the sizing lock on a key's turn), then passes the held key. - a_viewer_size_settle_ends_by_its_own_deadline_behind_a_held_frame (review 2 should-do 2): the attach's settle waits the barrier's 2 s, past its own 200 ms deadline. - a_write_the_writer_lost_after_taking_it_may_be_written (review 2 should-do 3): a frame the writer took and lost answers 'not written'. Test seams: hosted_surface_with (configure the fake host's attachment); InputPort::panic_after_take_for_test. * fix(cmux-tui-core): viewer-size frames go on the input port behind its frames, with no wait; the settle barrier ends by its deadline; a lost taken frame may be written (R-DAEMON-STATE S1.6c part 2) - #35082 review 1 item 4: a key that moves the grid sends a ViewerSize on its own turn under the global sizing lock (apply_terminal_grid -> resize -> send_viewer_size). It waited in the order barrier for up to 2 s behind the port's frames (a slow paste), holding the sizing lock, then went first. ViewerSize and ReleaseViewer now go on the port when it holds frames (PortOrder::enqueue_after_pending): ordered after them, with no waiter; a newer size replaces a queued one that is still the last frame, so sizes cannot pile up. With nothing pending (or the lane off: the port holds nothing) they are written directly, as before. - Review 2 should-do 2: begin_viewer_size_barrier takes the attach's deadline (direct_writer_until), so an attach with a 1 s settle no longer waits the barrier's 2 s. - Review 2 should-do 3: a Written frame whose writer took it and was lost (a panic during the write) answers InputMayBeWritten, so the creating-input flush keeps those bytes; one still queued answers not written. Tests (red at ac7ffa2b4e1): a_viewer_size_behind_a_key_the_port_holds_goes_after_it_without_waiting, a_viewer_size_settle_ends_by_its_own_deadline_behind_a_held_frame, a_write_the_writer_lost_after_taking_it_may_be_written. * test(cmux-tui-core): the reply-timeout tests take their frame on a running writer, not a new thread (#35082 review 2, should-do 4) a_write_whose_wait_ends_during_its_write_may_be_written and the_creating_input_flush_keeps_input_that_may_have_been_written needed the port's writer thread to start and take the frame within the 300 ms reply wait. running_paused_writer starts the writer on a key first and holds it paused; the next frame is taken at once by the running writer, and the reply wait is 2 s. * test(cmux-tui-core): the reader's real give-up and the daemon's shutdown detach end a write's reconnect wait (#35280 review, should-do 1) The S1.6c part-1 test called reader_gave_up by hand, so a call site that lost its wake turned no test red. These drive the real call sites: an exhausted TerminalHostReconnectBackoff's wait_or_fail, and Surface::disconnect_for_daemon_shutdown, each while a Written write waits for a reconnect. * docs(cmux-tui-core): the input port docs name reconnect_ended and state the lock rule the right way round (#35280 review, should-do 2) The port lock is a leaf: no holder of it waits for another lock, so the runtime lock's holders may take it. The shutdown.rs comments said the reverse. * fix(cmux-tui-core): a viewer size queued behind a held key no longer takes the port lock twice (S1.6c part 2) PortOrder::enqueue_after_pending computed its frame's reconnect deadline with reconnect_wait while it held the port lock, and in test builds reconnect_wait took that same std Mutex again to read the test override. The second lock on one thread deadlocks: the hosted focused runs at 10b577c0a0b, 67023f50a46 and 476e0bfc4a0 hung for 40 minutes in a_viewer_size_behind_a_key_the_port_holds_goes_after_it_without_waiting ("has been running for over 60 seconds", run 41819499722, cancelled). reconnect_wait now reads the guard its caller holds and never locks; both callers (the port's enqueue and enqueue_after_pending) call it under the lock. Release builds never took the second lock (the override is cfg(test)), so this changes no shipped behaviour, only the deadlock the tests hit. * test(cmux-tui-core): a viewer size or release sent while the input port waits for a reconnect is the new host's last size frame (#35375 review, red) enqueue_after_pending writes a size directly while the port waits for a reconnect, but a size queued before the loss stays on the port. The direct write goes to the lost connection; after the install the port writes the stale queued size to the new host, so the host keeps 97x31 while the daemon's lease says 100x30 (or keeps a lease the daemon released), and a later send of the same lease writes nothing. Red: a_viewer_size_sent_during_a_reconnect_wait_is_the_new_hosts_last_size and a_viewer_size_release_during_a_reconnect_wait_is_the_new_hosts_last_size_frame read the replacement host's size frames and require the newest last. * fix(cmux-tui-core): a viewer size sent while the input port waits for a reconnect goes on the port, so the new host's last size is the newest (#35375 review) enqueue_after_pending returned false while the port's writer waited for a reconnect, so the caller wrote the size directly, to the lost connection, while a size queued before the loss stayed on the port. After the install the port wrote that stale size to the new host after the install's replay of the newest lease: the host kept the old size (or a lease the daemon had released), and a later send of the same lease wrote nothing. It now queues whenever the port holds frames, during a reconnect wait too: the newest size replaces a trailing queued one, so the last size frame the new host reads is the newest lease or release. The barrier's reconnect skip exists because its callers hold the runtime lock the install needs; this enqueue never waits, so it needs no skip. Tests (red at 5ae437af1df): a_viewer_size_sent_during_a_reconnect_wait_is_the_new_hosts_last_size, a_viewer_size_release_during_a_reconnect_wait_is_the_new_hosts_last_size_frame. * test(cmux-tui-core): the input port's writer runs at user-interactive QoS once it carries a person's key (R-DAEMON-STATE S1.6d red, #35413 review) DESIGN 2.6.1 asks for the port's writer at user-interactive QoS while it carries the person's input, as the connection threads that carry the key run (server/input_qos.rs, TYPING FIX 2 F7). The writer thread is spawned at the default QoS and never raises itself, so under full CPU the hop from daemon.send.written to the host's PTY write waits for a CPU: the build 3b ON keys over 4 ms waited there (STATUS 11804). Red (macOS only; QoS is a no-op elsewhere): the_input_ports_writer_runs_at_user_interactive_qos_once_it_carries_a_persons_key. Test seam: the writer records its QoS each time it takes a frame (InputPort::writer_qos_for_test). * fix(cmux-tui-core): the input port's writer runs at user-interactive QoS once it carries a person's key (R-DAEMON-STATE S1.6d, #35413 review) The person's keys cross the connection's three threads at user-interactive QoS (server/input_qos.rs, TYPING FIX 2 F7), then the terminal's port writer, which ran at the default QoS: under full CPU it waited for a CPU between the key's acceptance (daemon.send.written) and its write to the host. DESIGN 2.6.1 asks for it at user-interactive while it carries the person's input; it was listed and not built. The writer now raises itself (input_qos::raise_this_thread, once per thread) when it takes a frame with no waiter that is not a viewer size: InputPort::enqueue, which only the person's keys use (InputReply::Accepted). Automation's writes (InputPort::write, a waiter) and viewer sizes do not raise it, so a port only automation writes to keeps the default QoS. Off macOS this sets nothing. Test (red at b0786647b9e, macOS): the_input_ports_writer_runs_at_user_interactive_qos_once_it_carries_a_persons_key. * test(cmux-tui-core): false, off and no turn the input lane off too (R-DAEMON-STATE S1.7 red, #35413 review) * fix(cmux-tui-core): false, off and no turn the input lane off too, and the daemon logs which path its keys take (R-DAEMON-STATE S1.7, #35413 review) Only the exact value 0 turned the lane off, so an operator who set CMUX_TUI_RDS_INPUT=false or off for the one-nightly fallback kept the lane on with no sign of it. 0, false, off and no (any case, spaces ignored) now turn it off, and the first switch a process reads is logged: "cmux-tui: input lane on, CMUX_TUI_RDS_INPUT unset" or "... off (the old path), CMUX_TUI_RDS_INPUT=\"0\"". Test (red at 5fab0a55b51): false_off_and_no_turn_the_input_lane_off_too. * test(cmux-tui-core): the port writer's QoS test seam exists only in macOS test builds, where its test runs (clippy dead_code on Linux) * fix(cmux-tui-core): the lane log's match arm ends with a semicolon (clippy semicolon_if_nothing_returned) * test(cmux-tui): a terminal's next output frame must reach it while a publication waits for the registry (#35413 echo stall; red) The output readers (hosted and local) make the durable commits their output asks for (directory, progress, activity, last output) inline, under the workspace registry lock. Under L6 that lock waits up to 1.5 s and the next output frame, the echo of a person's key, waits behind it (s17d: all 16 echo gaps over 20 ms began inside one of those commits, lane on and off). Both tests hold the workspace registry on another thread, let a first output frame make a publication due, then send the next frame: it must reach the terminal within 2 s. Today it reaches it only after the registry is free. * fix(cmux-tui): durable publications of terminal output commit on a per-mux publisher, not the output reader (#35413 echo stall) Root cause of the L6 typing echo stalls (s17d diagnostic rounds, lane on and off): after each output frame the terminal's output reader published the durable facts the frame changed (cwd, OSC 9;4 progress, OSC 7501 program status, the command and prompt facts, and "last output", pushed every 5 s while the terminal prints) itself, as resource commits under the workspace registry lock. Under load that lock waits up to 1.5 s; the next frame, the echo of a person's key, waited in the reader's queue behind it. All 16 echo gaps over 20 ms in s17d (to 501 ms) began inside one of those commits, 26 to 27 of them per round, about every 5.1 s. The reader also posted OSC 9/777/99 notifications and bell attention marks, durable commits of the same kind. Now the reader only checks, in memory under the terminal's own locks, whether a publication is due (new has_change / has_publication_change peeks that mirror each take exactly) and queues it on the mux's terminal publisher (mux/terminal_publisher.rs): one worker thread that runs the queue in order. A terminal has at most one publication queued, so changes that arrive while it waits are published with it, as the latest state, which is what every publication commits. Notifications and bell marks queue one by one, in output order. All four reader sites move: hosted output, hosted resize, hosted reconnect and the local PTY reader. Adoption, the first view, a terminal read and the topology commits still publish directly on their own threads. At shutdown, after the readers stop, the publisher finishes its queue within the same reader grace before the journal stops, as when the readers published inline. Tests: output_publications (red at 2c75845dec6: the next frame did not reach the terminal while the registry was held) for the hosted and the local reader; terminal_publisher: a queued publication commits once the registry is free and not before; notifications and the bell mark post in output order. * test(cmux-tui): output publication tests use the channel names already in scope (unused-qualifications) * test(cmux-tui): an exit commits the last reported directory; an alert posts after earlier notifications (#35765 review; red) Two orderings the inline publication on the output reader kept, and the terminal publisher (a91fb3a343e) lost: - a_terminal_exit_commits_the_directory_its_shell_reported_last: the shell's last cwd report is queued, then the host's Exit frame arrives; the queued publication runs after the exit and commits nothing (an ended terminal settles without publishing), so the exited tab and its restart keep the old directory. - a_programs_alert_posts_after_the_notifications_of_earlier_output: an OSC 9 notification from one frame, then an OSC 7501 alert from a later frame, while the registry is held; the publication job ran ahead of the earlier frame's notification job and took and posted the alert itself, first. Test-only seams: Mux::hold_terminal_publisher_for_test (the worker takes no job until the hold drops) and wait_for_terminal_publisher_for_test. Also the `closed` doc: queued jobs still run after the close. * fix(cmux-tui): an exit commits the last reported directory; alerts keep output order (#35765 review) Review findings on the terminal publisher (a91fb3a343e), red at 19bcc9ec556: - Exit (should-fix 4): the hosted reader used to commit the shell's last cwd before it handled the host's Exit frame. With the publisher, that queued publication could run after the exit, and an ended terminal settles without publishing, so `cd x; exit` kept the old directory on the exited tab, its exit record and its restart. mark_hosted_surface_exited now publishes the pending directory itself before the exit commit (on the thread that commits the exit: the reader at its Exit frame, or the exit detach retry). - Alert order (note 9): the publication job took and posted the OSC 7501 alerts itself, so an alert from later output could post before an earlier frame's OSC 9 notification (and pass the rate limit at a different time). The reader now takes and rate-limits the alerts at the output that raised them, as the inline publication did, and queues them after the publication of their records; the publisher's job (Surface::publish_pending_state) leaves alerts alone. The program-status peek no longer counts alerts. Direct callers keep publish_pending_progress unchanged. - Docs (should-fix 7, 8): the reader still takes Mux::state for its events, so the comment now says it never waits for the registry; the module doc names the real direct callers (adoption, first view, terminal.get, topology commits, a hosted exit) and why the queue stays bounded; the shutdown comment covers Mux::drop. - The worker reports with writeln to stderr, not eprintln, which panics when stderr fails and would have stopped the worker for good (note 2). The close race the review mentions (a tab closed while its cwd publication waits) is not new: the inline reader waited for the same registry lock as the close and lost the same race. * test(cmux-tui): a host loss commits the directory the shell reported last (#35765 review 2; red) Second review, should-fix 1: when a hosted terminal's host connection is lost, the reader commits the terminal Adopting before the publication it queued for the terminal publisher runs. An adopting terminal keeps a directory publication pending without committing it, and a host declared dead then ends it, so the dead tab and its L2 respawn kept the old directory. The inline reader always committed the directory before it read the end of the stream. a_host_loss_commits_the_directory_the_shell_reported_last: a seeded Running terminal whose catalog runtime is a live hosted reader on a fake host socket; with the publisher held, an OSC 7 report reaches the terminal, the host socket is dropped and the reader commits the loss; after the publisher drains, the terminal must present the reported directory. Test-only seams: Mux::install_live_terminal_runtime_for_test and next_surface_id_for_test. * fix(cmux-tui): a terminal's stream end settles its publications before it leaves Running (#35765 review 2) Second review, should-fix 1 (red at 3108cf0bb75): a host connection lost just after a cwd report kept the old directory. The reader commits the terminal Adopting when its stream is lost, and the publication it queued for the terminal publisher then commits nothing (an adopting or ended terminal never does), so the dead tab and its L2 respawn kept the old directory; progress and activity taken by such a job were dropped too (review note 2). Mux::settle_terminal_publications: take the terminal's queued publication off the publisher's queue, wait while the worker runs one for that terminal, then publish its directory, progress, activity and program status on the calling thread. Called - by the hosted reader when its stream ends by an exit or a loss, before the Exited or Adopting commit (a resync keeps the terminal running: its queued publication still commits); - by Mux::surface_exited for every exit (local PTY exits, dead hosts), before the creation fences, so a registry wait there no longer holds creations (review note 5). The flush moves out of mark_hosted_surface_exited. This is the order the inline reader had: it published before it read the end of its stream. No output frame waits behind it: the stream has ended. Publications are matched by terminal runtime, not by view, so a settle also finds a publication queued through another view of the same terminal. Docs (review note 6): post_terminal_notifications and the program status module now say that the reader takes the alerts and the publisher posts them. * test(cmux-tui): the host-loss test adopts its seeded terminal before the live reader starts At 3108cf0bb75 the test failed at its setup, not at its assertion, on both the old and the new code (fleet step 7612c04c51d785247a6fd3c2: "terminal is not awaiting host topology publication"): a hosted reader starts only for a launching or adopting terminal, and the seeded terminal was running. The test now commits the seeded terminal Adopting with its incarnation, starts the live reader, makes it the catalog runtime, releases its pending host binding and commits the terminal Running, as an adoption does. Also drops a redundant clone (clippy). Test-only seam: Mux::commit_terminal_lifecycle_for_test. * test(cmux-tui): the host-loss test moves its workspace key (clippy redundant_clone) and imports the lifecycle at the top * test(cmux-tui): the tree entity check must hold when a reader changes the tab's title between its builds (red) Full Linux suite at b2ebe6809df (hq run 41899477440): live_root_transfer_revocation_survives_restart_and_both_abort_paths_clear_it failed because the test-only check in tree_entity_json (server/tree_json.rs) panicked on the terminal publisher thread: "the TabChanged entity of 7 differs from the tree's", title "" vs the shell's title. The check builds the entity, then the whole tree, and compares them; both read live fields (title, size, connection state, end) without the state lock, so a reader that sets its title between the two builds fails it, and the panic, under the state lock, poisons the session (STATUS 12320 reported the same race in a title test). Before #35765 a terminal's directory TabChanged was built on its own reader, which could not set the title meanwhile; the publisher builds it on its own thread, so the race now hits every shell that reports its directory and title at a prompt. the_entity_check_holds_when_a_reader_changes_the_title_between_its_builds: a test seam (set_entity_check_hook_for_test) runs between the entity build and the whole tree build and sets the tab's title, as the reader would. * fix(cmux-tui): the tree entity check compares the two builders only on inputs that held still Red at 2f52b7eb82e (the_entity_check_holds_when_a_reader_changes_the_title_between_its_builds). The test-only check in tree_entity_json now builds the entity again after the whole tree and compares them only when the entity built before and after the tree agree: then the live fields both builders read without the state lock (title, size, connection state, end) held still across the tree's build, and a difference is a real builder difference. Live fields that moved are read again (at most 16 times; a field that never stops moving leaves nothing to compare). The check's builds still do not count as built panes. Production code is unchanged: the check is cfg(test). This was a base race (STATUS 12320, reported to the #32408 owner); #35765 makes it frequent, because a terminal's directory TabChanged is now built on the terminal publisher while the terminal's reader can set its title. * test(cmux-tui): a person's key to an idle input port is on the host socket when its request returns (#35413 A/B; red) The s17fa A/B on build 5cd1899b (STATUS 12857, 12866): with the input lane on, the daemon part of a key was +0.025 / +0.17 / +0.53 ms at p50 / p95 / p99 over the lane off, all of it in daemon.send.written -> host.pty.write. The request thread puts the key on the terminal's input port and returns; the port's writer thread then wakes to write the host socket, and under the L6 CPU load the wake is the extra time. With the lane off the request thread writes the socket itself. The test holds the port's writer before it writes anything, sends a key, and reads the host socket: the key must already be there. * fix(cmux-tui): a person's key to an idle input port is written on its own turn, with no thread hand-off (#35413 A/B) Red at dd67a789756. The s17fa A/B on build 5cd1899b (STATUS 12857, 12866): with the S1 input lane on, the daemon part of a key was +0.025 / +0.17 / +0.53 ms at p50 / p95 / p99 over the lane off, all of it between daemon.send.written and the host's PTY write (OFF 0.016-0.019 / 0.14-0.17 / 0.38-0.48 ms, ON 0.038-0.046 / 0.27-0.32 / 0.67-0.99 ms, in every round, also in s17d). The request thread put each key on the terminal's input port; the port's writer thread then woke to write the host socket, and under the L6 CPU load that wake was the extra time. With the lane off the request thread wrote the socket itself. Now a key (no waiter, at most 1 KB) to an idle port (nothing queued or in flight, no reconnect wait) is written on the request's own turn, under the port lock, when HostInputRoute::try_send_now can write it at once: try_lock on the connection's writer and a non-blocking send (MSG_DONTWAIT, and MSG_NOSIGNAL off macOS) that a Unix stream socket takes whole or not at all for a frame this small (a partial write would shut the connection down, as a failed write does). A busy writer, a full socket, an error or a frame ahead leaves the key to the port as before, so a key still never waits for another writer or a host that stopped reading (S1.6). The key counts as one of the port's frames, taken and finished at once, so the order barrier and the viewer-size queue see the same state as before. Tests: the port's writer tests that hold a key at a paused writer opt out of the inline write (queue_every_write_for_test), so they still exercise the writer's order, reconnect and QoS. * test(cmux-tui): a notification queued before a terminal's exit names its terminal (#35765 review 3; red) Third review, should-fix 2: the settle at a terminal's stream end and exit took only the terminal's queued publication. A notification its last output asked for (OSC 9/777/99, an OSC 7501 alert, a bell mark) stayed queued and posted after the exit removed the terminal's tab, so it named no terminal. The inline reader posted it before it handled the exit. a_notification_queued_before_an_exit_names_its_terminal: a seeded running terminal (closes on exit); with the publisher held, a notification is queued for it, then it exits; the posted notification must name the terminal. * fix(cmux-tui): a terminal's settle takes its notifications too; no surface is dropped under the publisher lock (#35765 review 3) Red at 37fd891b8a3 (a_notification_queued_before_an_exit_names_its_terminal). - should-fix 2: TerminalPublisher::settle now takes every queued job of the terminal (its publication, notifications and bell…
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
R-DAEMON-STATE stage S1 step 6c, part 1 (handoff/rewrites/R-DAEMON-STATE/DESIGN.md, section 3). It was stacked on #35082 (S1.6b), which landed as 65d098c; the branch now merges that squash and feat ca73823, with its own diff unchanged (patch id 66e943183f3e against the feat tip). Behind the per-mux switch
CMUX_TUI_RDS_INPUT(default off). Review 2 of #35082 asked for this fix before S1.7 turns the switch on.The defect. Since #35082 review 1, the input port's writer waits for a reconnect on its own route epoch, so a route publication ends the wait at once. But the reader's other wakes (it ended, it gave the host up, a reconnect completion failed) and the owner's detach still only notified
stream_progress, which the writer no longer waits on. So aWrittenwrite (a script'scmux send, a resource input effect) behind a host that will not come back failed only at its own reconnect deadline, 5 s later. The old path, and S1.6b before review 1, failed it at once.The fix.
InputPort::reconnect_endedmoves the port's route epoch with no new route. The writer then checks the route and the terminal's state again and fails its frame: aFailedlink, an ended reader, or a detaching owner.PtyTerminalRuntime::reader_gave_upnotifies the stream and ends the port's wait. It is called at the three reader points:WakeInputOnReaderEnd, the give-up inwait_or_fail, and a failed reconnect completion.owner_detachingstores inshutdown.rsend the port's wait. That takes the port lock under the runtime lock, which is safe because the port lock is a leaf: no holder of it waits for another lock. The stream notify is not added there.The person's keys (
InputReply::Accepted) do not wait for any write, so this changes only when other writers learn of the failure.Testing
Focused command:
./scripts/verify-cmux-tui-hosted.sh --filter a_write_waiting_for_a_reconnect_fails_when_the_reader_gives_up(Linuxtest (linux)).--filter hostedat 860edd9, run https://github.com/manaflow-ai/cmux/actions/runs/41814956814: 69 passed, 0 failed.cmux-cloud-environmentclippy fix), solint (linux)can check this crate again. Run https://github.com/manaflow-ai/cmux/actions/runs/41817024458 at 1e84ed3:test (linux)69 passed, 0 failed. Itslint (linux)checked cmux-tui-core with no error; it is red only in thecmux-tuicrate, which this PR does not change:cmux-tui/src/cloud_environment.rs:22(needless borrow, from feat(cloud): select NEXT backend at runtime in Rust #35133) and the panic ratchetcmux-tuiindexing_slicing 841 > 831 (frontend_journal.rs, frame_geometry.rs, agent_browser_provider.rs, ...). Own-diff patch-id unchanged (66e943183f3e).Compile: fleet job 5ba29eeee3ae225fb683cfb0 at the head 32f0563: CmuxNext, app and cmux-tui passed. Before that, job 2106afc90bf8ca0497254bad at 1e84ed3 passed. (Job bf827818dd60d90f91e018a2 at 860edd9 failed only on the
relaunch.rsbase red from #35133, which that commit predates; #35172 fixed it on feat.)Observed / Not run
cmux-tui-core/srcis the same at the head 32f0563 as at 1e84ed3 (the cascade after fix(cmux-tui-core): no frame passes a key the input port holds; only the person's keys answer before the write (R-DAEMON-STATE S1.6b) #35082 changed no file there). A--filter hostedrun at the head: https://github.com/manaflow-ai/cmux/actions/runs/41825140915.lint (linux)was red on the base findingcmux-cloud-environment/src/lib.rs:262(collapsible_if from feat(cloud): select NEXT backend at runtime in Rust #35133). cmux-tui-core depends on that crate through cmux-tui-cloud-conversations, so clippy never reached this PR's code there; the run at 1e84ed3 checks it.Changelog
none
Proof
No capture: daemon-only change behind a default-off switch, no UI.
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Fixes the input port's reconnect wait so a
Writtenwrite fails at once when the reader ends, gives the host up, or the owner detaches, instead of waiting out its full 5 s reconnect deadline.Since #35082 review 1 the writer waits on its own route epoch, and those wakes still only notified
stream_progress. NowInputPort::reconnect_endedmoves the route epoch with no new route; the writer checks the route and terminal state again and fails its frame (aFailedlink, an ended reader, or a detaching owner).PtyTerminalRuntime::reader_gave_upcovers the three reader wakes, and the fourowner_detachingstores inshutdown.rsalso end the port's wait, taking only the port lock.Behind the per-mux switch
CMUX_TUI_RDS_INPUT(default off). Covered bya_write_waiting_for_a_reconnect_fails_when_the_reader_gives_upinsurface/tests/hosted.rs.Written for commit 32f0563. Summary will update on new commits.