fix(seats): report Streaming while a client is attached - #49
Merged
Conversation
Closes #43. OnConnectAppLauncher.ProcessSeat opened with if (_options.LaunchOnConnect.Length == 0) return; which switched off the entire watcher -- the connect/disconnect DETECTION included -- and that option is empty by default. So on a normal host nothing ever observed a client connecting, and a seat streaming happily still reported Ready. The dashboard showed an active seat as idle, and anything reasoning about Streaming, including a person deciding whether a seat was safe to tear down, got the wrong answer. Detection is now unconditional; only launching and killing apps stays gated. ProcessSeat returns whether the status changed, and SessionHealthCheck broadcasts on that the same way it already does for a geometry change. Only Ready and Streaming are touched. A seat that is Provisioning, TearingDown or Error is mid-something that matters more than a client edge, and stamping Streaming over it would lose that and trip the transition table. 4 new test methods (6 cases), 546 passing. They drive the REAL ProcessSeat with LaunchOnConnect EMPTY -- the 13 existing tests only exercise the static log helpers, which is exactly why none of them ever noticed. Verified by restoring the old gate in place: 2 fail, on the changed flag and on the status. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQvL62WkT8xDWXqyjFCGDw
Merged
vibesoftwarecoder
added a commit
that referenced
this pull request
Sep 13, 2026
version.txt 0.6.4 -> 0.6.5, plus the notes the release workflow reads. No code changes -- four merged commits ship: #49 (a streaming seat reported Ready), #50 (a streaming standalone Apollo reported idle), #51 (that warning overstated the stall), #52 (the audit gate failed on a registry outage). ⭐ Three of the four are runtime-verified on the reference host rather than only CI-green: #49 Ready -> Streaming in 6s on connect, back in 3s on disconnect #50 streaming signal correct in BOTH directions, including a live stream the old serverinfo check reported as idle #51 266 ms measured, and the person streaming did not notice it⚠️ And master CI was RED before this. 45f46b0 failed on a 400 from registry.npmjs.org, not on our code -- the same commit had passed its own PR 45 minutes earlier and passed locally afterwards. #52 fixed the gate; the master push on b5feaac is green across all three jobs, checked before cutting rather than assumed. That is worth recording: "the tests pass" is not "master is green", and I reported the first while implying the second. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SQvL62WkT8xDWXqyjFCGDw
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
1 participant
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.
Closes #43.
The bug
OnConnectAppLauncher.ProcessSeatopened with:That switched off the entire watcher — the connect/disconnect detection included — and
MultiSeat:LaunchOnConnectis empty by default. So on a normal host nothing ever observed a client connecting, and a seat streaming happily still reportedReady.The dashboard showed an active seat as idle, and anything reasoning about
Streaming— including a person deciding whether a seat was safe to tear down — got the wrong answer.The fix
Detection is now unconditional. Only launching and killing apps stays gated on
LaunchOnConnect.ProcessSeatreturns whether the status changed, andSessionHealthCheckbroadcasts on that the same way it already does for a geometry change — the pattern was already there, two lines below the call.Only
ReadyandStreamingare touched. A seat that isProvisioning,TearingDownorErroris mid-something that matters more than a client edge; stampingStreamingover it would lose that information and trip the transition table.Testing
546 passing, 4 new test methods (6 cases).
They drive the real
ProcessSeatwithLaunchOnConnectempty — which is the point. The 13 existing tests for this class only exercise the static log helpers (LastMarkerIsConnected,ReadLatestState,SeedState), neverProcessSeatitself. That is exactly why none of them ever noticed the bug, and why adding more of the same kind would not have either.Verified by restoring the old gate in place: 2 tests fail, on the changed flag and on the resulting status. The other 18 correctly pass either way, since they assert no change.
A live check needs a provisioned seat, and the host was streaming when this was finished — seat churn during a stream is exactly what that rule exists to prevent.
The check to run when the host is idle: provision a seat, append a
CLIENT CONNECTEDline to itsapollo.log, and confirmGET /api/seats/{id}flips toStreamingwithin a health-check tick. The watcher tails the log, so that exercises the real path without needing a client.🤖 Generated with Claude Code
https://claude.ai/code/session_01SQvL62WkT8xDWXqyjFCGDw