feat(tui_gateway): announce session mirroring and stamp turn origins on the wire - #87371
Draft
ryantuc wants to merge 5 commits into
Draft
feat(tui_gateway): announce session mirroring and stamp turn origins on the wire#87371ryantuc wants to merge 5 commits into
ryantuc wants to merge 5 commits into
Conversation
19 tasks
…nsport slot A session held exactly one transport, and prompt.submit, session.resume, session.activate, and the queued-prompt drain all rebound that slot. A second client therefore took the stream away from the first: the earlier client stopped receiving the turn it was already rendering, and either client disconnecting parked the whole session on the drop sentinel. FanoutTransport goes in the same slot and satisfies the same Transport protocol, so write_json and every other reader of the slot are unchanged. It delivers each frame to a snapshot of its peers, concurrently when more than one peer is attached and the caller is not on an event loop, and prunes any peer that returns False or raises. A dead client is dropped; a slow one costs the emitter at most one write timeout per frame rather than one per peer. Request/response RPCs are unaffected: they still answer on the request's context-bound transport, so a client only ever sees replies to its own calls. The rebind sites become attach sites through _attach_session_transport, whose ladder keeps the single-client shape identical. The same object already in the slot is a no-op; an empty, stdio, or parked slot is taken outright; only the arrival of a second live client wraps both. The queued-prompt drain is included because it pinned the drained turn to the queuer and silenced everyone else. A non-peer newcomer such as stdio or the drop sentinel never displaces a live client, so an activate dispatched without a bound websocket cannot silence the socket that owns the session. Disconnect detaches first. A session that retains another client keeps streaming and is neither parked nor reaped, and only the clientless ones follow the existing close_on_disconnect and park-sentinel path, so a single-client disconnect, the orphan reaper, and its grace window behave as before. _ws_session_is_orphaned is unchanged: it still asks whether the drop sentinel is in the slot, and a fan-out is never the sentinel, so a session that still has a peer is never reported as orphaned. Attach performs no entitlement check: any authenticated peer may mirror any session. The fan-out architecture follows the approach in NousResearch#40822 by @OmarB97. What the slot's later history forces. _close_sessions_for_transport drops the NousResearch#83716 rebind-to-the-most-recent-surviving-viewer, which fan-out membership subsumes — a pop-out window is a peer, so a session that still shows in one is never returned as clientless — and keeps the NousResearch#77129 revalidation before parking, now expressed as a liveness check under _session_transport_lock so it is race-free against attach and detach. _transport_is_live_peer defers its last answer to _transport_is_dead: a socket that already latched _closed is a departed client, and admitting it would keep a session out of both the park and the reap. _transport_is_dead also learns the fan-out: a FanoutTransport with no live peer is dead, so a session whose peers were all pruned by failed writes cannot outlive the TTL and LRU reapers. The upstream test that pinned the NousResearch#83716 rebind, test_close_transport_rebinds_session_to_remaining_viewer, is re-expressed in fan-out terms: both windows attached, the pop-out closes, the session stays with the main window unparked and still receiving frames.
… fan-out subagent.steer resolves authority by comparing the request's context-bound transport with the session's transport slot. Once a session mirrors to more than one client that slot holds a FanoutTransport, so the comparison fails for every client, the peer that commissioned the subagent included, and every steer is rejected. Fan-out without this check ships that regression, and no existing test catches it because the suite only exercises single-client sessions. Authority now asks whether the request's transport is attached to the session, directly or through the fan-out. The single-client case is unchanged: a bare slot still compares by identity. This widens authority. Any client attached to a mirrored session can steer that session's subagents, not only the peer that commissioned them. Narrowing it back to the commissioning peer requires recording that peer per subagent, which this change does not do. tests/tui_gateway/test_multi_client_fanout.py pins the commissioning peer's authority inside a fan-out, pins the widened case, and keeps a single-client control in which an unattached client is still refused. The four browser.controller.* handlers gate on the session transport slot exactly as subagent.steer did, through one shared gate in the _controller_method decorator, and the conversion missed them. Once a session mirrors to more than one client the slot holds a FanoutTransport, which is identical to no peer's WSTransport, so browser.controller.register, .result, .heartbeat and .detach all answer "session is not owned by this transport" for every client, the peer that registered the controller included. Browser control is therefore unusable on any mirrored session. No existing test catches it because the browser-control suite only exercises single-client sessions. All four now ask the same question steer asks: is the request's transport attached to this session, directly or through the fan-out. The single-client case is unchanged, because a bare slot still compares by identity. Only registration widens. On .result, .heartbeat and .detach the broker's is_owner check sits below the session gate and compares controller.owner is owner against the transport recorded at attach time (gateway/browser_control_broker.py), so a mirrored peer that did not register the controller is still refused there, now with "controller is not owned by this transport" instead of the session message. browser.controller.register has no such check, so any client attached to a mirrored session may register a controller for it; the broker's principal lane keeps that inside one authenticated identity, and a second identity in the same lane hard-replaces the first. tests/tui_gateway/test_multi_client_fanout.py pins the registering peer's access inside a fan-out, pins the widened and broker-refused cases, and keeps a single-client control in which an unattached client is still refused on all three scope-gated handlers.
…ed prompt _drain_queued_prompt attaches the transport pinned to the queued envelope, but the client that queued the prompt may have disconnected while the prompt sat in the queue. Attaching it then pins a dead peer into the session's fan-out, where it costs one failed write before the fan-out prunes it. That is not a regression — the line this replaced rebound the whole slot to that same dead transport, which cost the session for the entire drained turn rather than one write — but the guard is one condition, so it goes in. _transport_is_dead is the predicate already used by the reaper and the orphan check: the parked drop sentinel, or a transport carrying _closed. Drain semantics are otherwise unchanged. The prompt still runs, and the clients already attached keep their stream; only the dead peer's attachment is skipped. tests/tui_gateway/test_multi_client_fanout.py pins that the drained prompt still dispatches while the disconnected queuer stays out of the slot, alongside the existing case where a live queuer is attached and the single-client control where the queuer takes an unoccupied slot. Raised by the automated review on NousResearch#86784.
…on events A client attached to a mirrored session receives every frame of a turn a peer started and has no way to tell that turn from its own, so it cannot attribute output or decide whose composer to latch. Each accepted WebSocket now mints an opaque per-connection id and gateway.ready returns it as origin alongside a session_mirroring capability flag, following the existing hardcoded-bool style of change_events. The session records the origin of the client that started the current turn, and _event_frame stamps it onto the params of every session event: prompt.submit records the submitter, a queued-prompt drain records the queuer, and the auto-continue kickoff records "auto_continue" so a recovery stream is not attributed to anyone. Each site records the origin where it claims the turn, under the session's history lock, so a mid-turn submit that redirects, queues, or is refused leaves the running turn's frames attributed to the client that actually started it. The stamp is purely additive and the key is omitted whenever the origin is unknown, which covers a stdio TUI, a session-less broadcast, and every frame in a deployment that never had a second client. A client detects the contract by reading session_mirroring from gateway.ready; its absence means the backend does not stamp origins.
…s watching session.resume and session.activate now return watching in their result: true when another live client was already attached to the session at the moment the caller arrived, false on every path where the caller registered the session itself. All four such resume paths — cold, eager, deferred and the lazy child-watch window — build their result through one shared payload helper, so all four carry the key; the deferred resume is included deliberately, since a client that resumes with defer_history registers the session exactly as the others do and a resume shape that silently omitted the key would be the worse contract. The value is read before the attach, so it answers "did I join a stream in progress" rather than "is anyone attached now", which is always true once the caller is on. Together with the per-turn origin stamp this is enough for a client to render a session it is mirroring differently from one it owns. There is no request-side counterpart. Resume attaches unconditionally, so an intent flag would report nothing the result does not already report, and resume ignores request params it does not recognize. tests/fixtures/session-resume-active-turn.json gains the key. That fixture is the shared resume contract, and its test in tests/tui_gateway/test_protocol.py asserts the real serialized result equals it exactly, so a new result key has to land there or that assertion fails. The value is false, read off the actual payload rather than written by hand, because that scenario has no second client attached. The desktop test importing the same file reads only the timer fields and is unaffected. The resume tests added here drive the real handler rather than a double, so they restore the config and launch-database globals tui_gateway.server caches — the same ones tests/tui_gateway/test_stale_provider_resume_live.py repoints at an isolated HERMES_HOME, and which would otherwise carry over to it.
ryantuc
force-pushed
the
feat/session-mirroring-wire
branch
from
September 5, 2026 05:43
b3411b3 to
34d4861
Compare
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.
What does this PR do?
A draft stacked on #86784, the transport fan-out fix, carrying the wire-contract half. Five commits show and the first three are #86784's at
df52ccfd40, since a branch on an unmerged base cannot show its delta alone; the delta is the last two, and the tip is34d48614a5. Both branches sit on79445a496c, so this one is re-ported over the same decomposition (#102117) #86784 is:tests/fixtures/session-resume-active-turn.jsonand its exact-equality test still pin thesession.resumeresult to the byte, so the commit that adds a result key updates the fixture. #86784's comment offered the whole wire contract as a draft rather than a surprise after the fix landed, and this is it. Close it without ceremony if it is noise. It flips to ready when #86784 lands and this rebases onto main.Under fan-out, two clients on one session get the same frames and no frame says whose turn produced it. A client cannot attribute what it renders, nor tell whether to latch its composer for its own user or leave it free for a peer. Resume and activate have the mirror gap: joining a stream someone else started returns a result identical to registering the session yourself. This PR adds the minimum for both. It does not add a user-message event, so a client still learns what its peer typed only when it reconciles history at the end of a turn; what it adds is the ability to tell whose turn is streaming.
session_mirroringgateway.readyparams.payloadtrueon the websocket gatewayorigingateway.readyparams.payloadorigin_event_frameparams, alongsidetypeandsession_id"auto_continue"for the crash-recovery kickoff, and omitted entirely when no turn origin is knownwatchingsession.resumeandsession.activateresultstruewhen another live client was already attached at the moment the caller arrivedEach accepted websocket mints an opaque id at accept (
uuid4().hex, never reused across connections), returned to that client asoriginongateway.ready. The session records who started the current turn:prompt.submitthe submitter, the queued drain the client that queued the prompt rather than whoever is driving when it fires, the auto-continue kickoff"auto_continue"so a crash-recovery stream is not attributed to a person._event_frame(tui_gateway/server.py:571at79445a496c) reads that value and addsorigintoparams.The submit and the kickoff record the origin where they claim the turn, under the history lock and beside the line marking the session running; the drain records the queuer at drain time, where its turn is claimed. That placement is the correctness argument. A prompt arriving mid-turn starts no turn: under the default busy policy a redirect-capable agent folds the correction into the live turn and answers
redirected, and otherwise the prompt is queued for the next turn or refused. Recording at the point the submitting client attaches would repaint the running turn's remaining frames,message.completeincluded, with the id of the peer that merely typed over it, inverting attribution both ways: the starter reads its own completion as a peer's, and the peer reads a turn it never started as its own. The drain stamps the queuer whether or not that client is still connected, since the turn is its work either way.prompt.submitreads the submitter's origin where it attaches and hands it to the private helper that claims the turn, rather than re-reading it at the claim, so the stamp carries the origin read at attach time and the claim site cannot pick up a different transport.The change is additive. The key is omitted wherever the origin is unknown, which covers a stdio TUI, a session-less broadcast, and every frame on a backend that never had a second client attach, so no existing frame changes shape and a client ignoring all three fields behaves as it does today.
Feature detection is
gateway.ready: nosession_mirroringin the payload means the backend stamps no origins, sooriginmust not be relied on.tui_gateway/entry.py's secondgateway.readyfor the stdio gateway (tui_gateway/entry.py:257-261at79445a496c) is deliberately left alone, since that path serves one client, mints no per-connection id and stamps no origin, so the missing flag is the correct answer rather than an omission.Two design calls a reviewer may want to argue with:
originrides onparamsbesidetypeandsession_idrather than insidepayload, because payloads are built by each emitting site and there are many, so stamping there means touching every emitter and inventing a rule for payloads that are a list or absent.paramsis the envelope_event_framebuilds for every event emitted through_emit, so the stamp is one line covering current and future emitters. The cost is thatoriginsits a level up from the data it describes.The stamp misses two frames, named here rather than left to a grep.
tui_gateway/host_supervisor.py:430at79445a496chand-builds anerrorframe when a compute-host turn fails, so that frame carries noorigin. Under the opt-in per-turn process isolation the child builds the turn's frames and the parent relays them verbatim with no copy of the parent's turn origin, sooriginis absent for the whole isolated turn rather than one frame of it. Neither is a regression and both are safe the same way: a missing key reads as unknown, which is what it is. Closing them means carrying the origin across the host boundary, which belongs with the process-isolation work.session_mirroringis a hardcoded bool rather than a capability-list entry, matching the house style for this handshake:gateway.readyalready carrieschange_eventsas a baretruefor the same purpose (tui_gateway/ws.py:275at79445a496c), letting clients demote a legacy poll. A list is a better long-term shape, a worse fit for a two-field addition, and a larger protocol change than this PR asks for.Related Issue
Depends on #86784, which carries the credit for the architecture:
FanoutTransport, the attach and detach ladder, and the invariant that RPC replies stay on the request-bound transport are a port of @OmarB97's #40822. #86784 extracts the backend half so it can land without the session-presence substrate #40822 stacks on (#40814, still open, carried whole inside #40822's first commit). If #40822 lands first, both PRs should be closed or rebased down to the delta.Related, not fixed here: #79064, whose first root cause is the transport rebind #86784 fixes, and #55564, which reports the
prompt.submithalf of the same rebind. This PR adds nothing to that fix; it makes the mirrored result usable by a client that needs to attribute a turn.Type of Change
Changes Made
Everything below is in the two commits on top of #86784: the gateway files for the origin machinery, one for
watching, the shared resume fixture, and the fan-out test file.Per-file inventory
tui_gateway/ws.py: eachWSTransportmintstransport_idat accept, andgateway.readygainssession_mirroringandoriginin its payload alongside the existingskinandchange_events.tui_gateway/server.py:_transport_originreads a transport'stransport_idback off it and returns""for a transport that has none, such as stdio.tui_gateway/server.py:_event_framestampsparams["origin"]from the session'sturn_origin, omitting the key when it is unknown.tui_gateway/session_auto_continue.py:_drain_queued_promptrecords the queuing client as the drained turn's origin, before and independently of the guard that skips attaching a queuer which has since disconnected, so that guard governs the attach and not the attribution._maybe_schedule_auto_continuerecords"auto_continue".tui_gateway/methods_prompt.py:prompt.submitcaptures the submitter's origin where it attaches and passes it to_lock_in_submit_turn, which records it under the history lock at the point the prompt claims the turn, so a submit that redirects, queues or is refused leaves the running turn's origin untouched.tui_gateway/methods_session.py:session.resumeandsession.activatecomputewatchingbefore attaching the caller, so it answers "did I join a stream already in progress" rather than "is anyone attached now".session.resumecomputes it in_resume_reuse_live, the single live-reuse path (methods_session.py:646at34d48614a5), and writesfalsein_resume_response, the single builder every register-the-session path shares (:677); between them they cover every success return ofsession.resumeexcept_resume_live_unpersisted(:521), the never-persisted Bot Chat reattach, which returns a minimal lazy payload and carried nowatchingbefore the decomposition either.session.activatecomputes it once (:912). Sofalsenow covers every path that registers the live session for the caller: cold, eager, deferred and the lazy child-watch window, because the decomposition made all four share one payload builder. The deferred resume did not carry the key before this rebase and now does; it is a deliberate widening and the commit message says so.tests/fixtures/session-resume-active-turn.json: one key,"watching": false.test_session_resume_active_turn_payload_matches_desktop_fixturecompares it for exact equality against a real serializedsession.resumeresult, so any new result key lands there. The value is what the gateway serializes for that scenario, which has no second client attached, read off the failing assertion rather than written by hand. Its other consumer, the desktopuse-session-actionstest, reads only the turn-timer fields and needed no change.tests/tui_gateway/test_multi_client_fanout.py: 14 tests in three sections, covering thegateway.readyannouncement and per-connection id; the origin stamp on a submitted turn, a drained turn, and a drained turn whose queuer disconnected; the two mid-turn cases where a peer's prompt must not take the origin; the omitted key when the origin is unknown;watchingon the live-reuse, already-attached, cold, lazy and activate paths; and tolerance of an unrecognized request parameter.How to Test
Two clients, one session: start a turn in one and open that session from the other. Both render it, every event frame carries
originequal to the first client'sgateway.readyid, so the second can tell the turn is not its own, and its resume result carrieswatching: true.The origin stamp without a UI. Save the script below at the repository root and run
python repro_origin_stamp.py. Importing the gateway rebindssys.stdouttostderr, since stdout is its JSON-RPC channel, so these lines arrive on stderr.Repro script and observed output
On #86784's branch without this one, neither client can attribute the frame:
On this branch the frame names the client whose turn it is:
pytest tests/tui_gateway/test_multi_client_fanout.py -qgives54 passed, which is fix(tui_gateway): attach resuming clients instead of stealing the live event stream #86784's 40 plus this branch's 14. The file cannot be run at fix(tui_gateway): attach resuming clients instead of stealing the live event stream #86784's head at all, because it needs_transport_origin,session["turn_origin"]andWSTransport.transport_id, none of which exist there. The attribution measurement is the other way round: the whole directory on this branch with these 14 tests deselected gives4 failed, 1008 passed, identical to fix(tui_gateway): attach resuming clients instead of stealing the live event stream #86784's head in population, count and failure list.pytest tests/tui_gateway/test_protocol.py -qgives68 passed; it holdstest_session_resume_active_turn_payload_matches_desktop_fixture, which without this PR's fixture line fails withLeft contains 1 more item: {'watching': False}, every other item identical.pytest tests/tui_gateway/ -qon Windows 11, with the same deselect and ignore fix(tui_gateway): attach resuming clients instead of stealing the live event stream #86784 uses, gives4 failed, 1022 passed, 1 deselectedhere against4 failed, 1008 passed, 1 deselectedat fix(tui_gateway): attach resuming clients instead of stealing the live event stream #86784's head; every failing name is pre-existing on this platform and some flap run to run. Details below.Each commit is green alone: the first, the origin machinery and its tests, gives
48 passedon the fan-out file with nowatchingtests present, which is fix(tui_gateway): attach resuming clients instead of stealing the live event stream #86784's 40 plus that commit's 8.Checklist
Code
fix(scope):,feat(scope):, etc.)tui_gateway/session_auto_continue.py, which took two ofserver.py's hunks in the decomposition.pytest tests/ -qand all tests pass. Not claimed.tests/tui_gateway/was run with a control run of the same directory at fix(tui_gateway): attach resuming clients instead of stealing the live event stream #86784's tip; the remaining failures are pre-existing on this platform and are listed below.Documentation & Housekeeping
docs/, docstrings) — or N/A. The three fields are documented in comments at the sites that emit them, including whyoriginis omitted rather than sent empty.cli-config.yaml.exampleif I added/changed config keys — or N/A. No config keys.CONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A.scripts/check-windows-footguns.py --diffover this delta reports nothing across the six files it scans, andruff checkpasses on the six Python files among the seven.Screenshots / Logs
Focused file on this branch:
The protocol file, which owns the resume fixture's exact-equality test:
Full tests/tui_gateway run, branch vs control at #86784's head, back to back on one machine
Full directory on this branch:
The same directory at #86784's head, same machine and runner, taken back to back with the run above:
Every failing name is pre-existing here. Three are the stable core and appear on every tree in every round: a tempfile write, a compute-host line-JSON exchange, and
signal.SIGPIPEnot existing on Windows. Everything past those three is a timing or subprocess flake that fires at #86784's head at least as often as here, and the whole directory was run twice on this branch rather than once so the flake set is measured rather than assumed. Collected rises from 1012 to 1026, exactly the 14 tests this branch adds, and that count is the stable signal, so expect a handful of failures from this set on Windows and read the collected number instead.Uncertainty
turn_originis written when a turn is claimed and never cleared, so an event emitted while the session is idle carries the most recent turn's origin rather than none. The guarantee is the in-turn case for a turn claimed by a submit, a drain, or the kickoff: from the claim until that turn ends, every frame_event_framebuilds names the claimant, and a peer submitting mid-turn does not change it. The idle case is not guaranteed, and nothing treats the field as a liveness signal. Clearing it means finding every teardown path, interrupted and errored included, and getting that wrong drops the origin off a turn's last frames, which is worse. If a maintainer wants it cleared, the contract narrows and the docs above need the narrower wording.The same gap has a second face. Several gateway-internal turns claim the session without recording an origin, so their frames carry whatever the previous turn left: the loop and wakeup ticks, the kanban notification batches, the delegation completion turns, and the goal continuation all set the session running and dispatch directly. The three sites recorded here are the ones a mirrored client is trying to attribute; the synthesized ones want a fourth value meaning "the gateway did this", and naming it is a wider conversation than this PR.
A drained turn is stamped with the queuer's id even after that client disconnects, which is honest, the turn is its prompt, but does mean an
origincan name a connection nobody holds. Attached clients read it as not theirs, which is what they need.test_a_drained_turn_is_the_queuers_even_after_the_queuer_disconnectspins it, because the alternative leaves a drained turn wearing the previous turn's origin.Verified on Windows only. Nothing here is platform-specific and the new tests touch no platform surface, but they have not run on Linux or macOS.
The
gateway.readytest drives a fake websocket through the realhandle_ws, closer to the wire than the rest of the file but still not a real socket. Every other test uses fake client objects, this directory's convention.The origin is an opaque per-connection id, so it changes on reconnect: a client that reconnects mid-turn sees its earlier turn stamped with an id it no longer holds and reads that turn as a peer's. Making the id survive a reconnect means giving it a lifetime and an owner, a session-identity question rather than a wire-format one.
What I did not do
No user-message event.
originsays whose turn is streaming; it does not carry what that client typed, and nothing on the wire does, so a peer still reconstructs a mirrored user's prompt from history at the end of the turn. Adding such an event is a protocol addition with its own privacy and ordering questions and is not attempted here.No request parameter. A client cannot ask to mirror or ask not to, because the attach is unconditional and a flag would report intent rather than fact.
originis not persisted: it appears in no stored transcript, nosession.history, and no REST response, only on live event frames and thegateway.readyhandshake.No identity on an origin. It names a connection, not a user or a device, and nothing in it resolves to a principal. Per-client identity on the wire needs the ticket auth path to carry a principal first, which #86784 describes and does not attempt either.
No capability negotiation. The client is told what the backend does; it cannot ask for anything different.
tui_gateway/entry.py, the stdio gateway, is untouched and announces no mirroring.