Repository navigation
[compass] Fix: a display change that ends during a stop reports the stopped seat detached - #44
Merged
Conversation
…t detached (B-039) DisplayAsync now owns the seat state after a display change. A reconnect that strands the viewer throws ViewerDetachedException, and DisplayAsync reports the seat detached only if the stop's token, checked under the lifecycle gate, is not cancelled. The success path sets the display and "ready" under the same check, so a late change can't report on, or carry its display into, a seat started after the stop. ForwardToSeatAsync checks and sets "lost contact" under the gate. New quick check: stop during display change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ched; CHANGELOG wording A Reconnect refused by a prerequisite also cancels the bring-up token, so a later live display change answered "stopped". DisplayAsync now compares _stopVersion, which Stop counts under the gate together with _stopRequested. A recovering reconnect sets ready only while the host is there. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…6-10-14-stop-detached-race
skulitom
added a commit
that referenced
this pull request
Oct 15, 2026
… registry off 0.11.0 (#47) * [compass] Anode 0.11.3 release candidate (C-058) - Version 0.11.3 in Anode.csproj, server.json, the MCPB manifest and the Claude Code plugin, and in INSTALL.md's pinned installer example. - CHANGELOG: Unreleased becomes 0.11.3, with one line for the docs and release-tooling changes (#35, #40, #43, #45, #46). - .github/RELEASE_NOTES.md: what's new in 0.11.3 (#34, #36 to #44), known issues and upgrade. - docs/RELEASE-READINESS.md: a 0.11.3 section with the scope, the checks run without a seat, and a place for the owner's release-check summary. Package manifests (Scoop, winget) change after the release, as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [compass] 0.11.3 candidate: .NET 10.0.13, and review fixes - RuntimeFrameworkVersion and System.ServiceProcess.ServiceController move to 10.0.13, Microsoft's security update of 2026-10-13 (CONTRIBUTING: the current patch before each release). CHANGELOG, release notes and the readiness record say so. - The log-times entries say exactly what changed: log lines read 2026-10-13 23:34:16; screenshot names and backup dates use the Gregorian calendar. - RELEASE-READINESS 0.11.3: scope by group, including #19 (the Business licence terms, first released here); the live-check summary is recorded on main after the run; coverage limits and known issues. - RELEASE_NOTES links the 0.11.3 section of the validation record. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * [compass] 0.11.3 candidate: B-040's known issue covers only a display change that reconnects the viewer Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
skulitom
added a commit
that referenced
this pull request
Oct 15, 2026
…isplay reconnects answer "stopped" and the next disconnect drops the seat (#48) * Fix: a refused Reconnect leaves a running seat alone (B-040) ReconnectAsync cancelled the bring-up before it checked the machine's prerequisites, then set the seat to error when one was missing. A running seat kept its host, but every later seat_display reconnect got the cancelled token and answered "The seat was stopped while its display was changing", and the next viewer disconnect, seen in state error, dropped the seat host as a failed startup. The prerequisites are now read before anything changes. With the seat's host serving, a refusal only logs and tells the user in the viewer; a failed or stopped seat still reports error as before. New quick check "refused reconnect keeps the seat"; the #44 case it supersedes moved in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Review: read prerequisites only for a press that can act; clearer refusal text From #48's second review: a press during a reconnect, a display change, a stop or a quit returns before the prerequisite probe (up to about 1.5 s on the UI thread); the viewer's refusal puts the missing setting first; the quick check drops two counts an ok reply already proves; the CHANGELOG names the listener and says "a" display reconnect, since a password seat's is refused earlier. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Review: a prerequisite probe that throws refuses Reconnect and says why From the Codex connector's review of f348f49: the probe had moved out of ReconnectAsync's try/catch, and the toolbar discards the task, so a probe that threw (an unreadable registry key, say) failed silently. Its failure is now logged and treated as a missing prerequisite: a running seat keeps its state, a seat that isn't running reports error with the reason. The quick check covers both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Review: list #48 in the 0.11.3 record; name the probe failure accurately From the second reviews: the 0.11.3 release notes and validation record (#47) now include this fix, drop B-040 from the known issues and say the candidate's quick suite is 109 checks after it; a probe that throws now reads "could not check this machine's seat prerequisites", since the probe reads more than Remote Desktop's settings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Report
ANODE-BUGS.md B-039 (filed 2026-10-13 by compass-anode-fixer from #42's final review): a display change that fails while the seat stops can set
detachedafterstopped.Root cause
Each of these was a check followed by an act that a stop could slip between, outside
_lifecycleGate(the lockStopSeatAsync→CancelBringUptakes to set_stopRequestedand cancel the bring-up token):ChangeViewerDisplayAsync): the catch'swhen (!cancel.IsCancellationRequested)filter ran beforeawait OnViewerAsync(ConnectionState). A stop that ran during that await, even all the way tostopped, was followed bySetState("detached", …). The signed-out seat then said "still running … press Reconnect".ForwardToSeatAsync):!_stopRequestedwas read outside the lock, thenSetState("detached").DisplayAsync):if (!_stopRequested) _display = target. A new start right after the stop clears_stopRequested(PrepareStart), so a change from the stopped seat could carry its display into the new seat's viewer connection. A late failure in the same window setdetachedduring the new start, which ended that start with "Seat startup ended in state 'detached'". AlsoSetState("ready")ran after the lock was released.Fix
DisplayAsyncnow owns the state after a change.ChangeViewerDisplayAsynconly throwsViewerDetachedException(inner exception = why).DisplayAsyncreportsdetachedunder the gate, and only if the change's own bring-up token (which the stop cancels, and a later start replaces rather than resets) is not cancelled. Otherwise it answers "The seat was stopped while its display was changing." (the reply the cancelled path already gave), with noviewer_detachedcode, so the seat host doesn't ask for a reconnect._display, and setsreadyinside the gate.ForwardToSeatAsyncchecksReferenceEquals(_seat, seat) && !_stopRequestedand setsdetachedunder the gate (a restarted seat has a new host client, so the reference check covers that generation).StopSeatAsync's workaround line from [compass] Fix: a detached seat can't be reconnected by the button its message names, and agents aren't told capture is down #42 (clearing adetachedadvice that slipped in during the stop) is removed: nothing can setdetachedafterstoppingnow.stoppingstill clears the advice of a seat that was detached before the stop (covered bydaemon detached viewer).Tests
stop during display change(Cli/DiagnosticsChecks.cs), with injected viewer and display hooks and no live seat. A stranded reconnect reportsdetachedwith its advice. One stranded during a stop leavesstoppedwith nolastError. After a stop and a new start, a late failure or a late success leaves the startconnectingand_displayunset, and the start still ends at its stop.selftest --quick: 109/109. FOCUS test command (scripts\build.ps1 -QuickTest -OutputDirectory artifacts\pkg-build): 109/109, docs consistent.detached;_stopRequestedinstead of the token (detached and success paths); the oldif (!_stopRequested) _display = target; nodetachedat all; noreadyon recovery.ForwardToSeatAsyncwindow is two adjacent statements, with no seam for a deterministic test. The fix there is by construction: one lock acquisition, which the stop also takes.Manual check in a real seat
None needed for the fix itself: the race needs a stop at the moment a display reconnect fails. A normal
seat_displayreconnect, followed later by Stop, should behave as before.Edge noted, not changed: a Remote Desktop control fatal error during a display change also cancels the token. Such a change now answers "stopped while its display was changing", as the cancelled path already did, and no longer overwrites the
errorstate withdetached.🤖 Generated with Claude Code