Repository navigation
[compass] Fix: a refused Reconnect sets a running seat to error, so display reconnects answer "stopped" and the next disconnect drops the seat - #48
Conversation
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>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f348f4918d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| internal async Task ReconnectAsync(bool promptForCredentials = false) | ||
| { | ||
| // Read before anything changes, so that a refused reconnect can leave a running seat as it was. | ||
| string? blocked = _blockingSummary(); |
There was a problem hiding this comment.
Handle failures from the prerequisite probe
When a prerequisite read throws—for example, because access to an HKLM key queried by Preconditions.BlockingSummary is denied—this call now occurs before the method's try/catch. The toolbar handlers discard the returned task with _ = ReconnectAsync(), so the fault is unobserved and the button appears to do nothing, without the logging and error status provided by the previous placement. Keep the probe before lifecycle mutation, but include it in the exception-handling path.
Useful? React with 👍 / 👎.
…usal 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>
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>
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>
…g, so the next set or tap sends it (#49) * Fix: a refused gamepad_set leaves part of its change for the next report (B-041) GamepadManager.Apply set each button, axis and trigger as it read them. The pads run with AutoSubmitReport off, so when a later name was unknown the call threw before SubmitReport and the earlier changes stayed in the pending report, for the agent's next gamepad_set or gamepad_tap to send. Every name is now resolved first; the changes are set and submitted together. A trigger passed as a button got "Unknown gamepad button 'lt'. Use ... lt or rt."; it is now told it is a trigger. gamepad_tap recognises the trigger names the triggers take (l2, r2). The stand-in pad in the quick checks now keeps what was set until a report sends it, and the new check "gamepad set all or nothing" covers all three. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Review: simpler stand-in pad with its report history; CLI-true trigger message From #49's second review: ViGEm's Equals(object) compares type and index, so the stand-in keys by control alone; it keeps each sent report so the check proves a tapped trigger was pulled to 255, and counts reports instead of an Unsent test that is always false after a submit; the trigger error says gamepad_set's triggers are an MCP option. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Review: list #49 in the 0.11.3 release notes and validation record From #49's second review: the 0.11.3 candidate's record (#47) now names this fix, its stand-in coverage limit and the 110 quick checks after #48 and #49. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Review: the release note names triggers among what a refused set left behind Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Report
ANODE-BUGS.md B-040 (filed 2026-10-14 by compass-anode-fixer from #44's second review): after a Reconnect refused by a missing prerequisite, a display reconnect answers "The seat was stopped".
Root cause
ReconnectAsync(src/Anode/Daemon/AnodeDaemon.cs) cancelled_bringUpCancellationfirst, so that an old host startup lets go of the seat gate, and only then, under the gate, checked the machine's prerequisites (_blockingSummary()). When one was missing (Remote Desktop turned off, the listener gone, child sessions disabled) it returned withSetState("error", ...), beforePrepareStartmade a fresh token. A seat that was running kept its host (_hostReadystayed true), so agents carried on, but two things were now wrong:seat_displayreconnect failed.DisplayAsynchands the cancelled token to the viewer's reconnect, which disconnects the viewer and then gives up at its first wait: "The seat was stopped while its display was changing", though nothing had stopped, and the viewer was left disconnected.OnViewerDisconnectedtreats a disconnect in stateerroras the end of a failed startup: it cancels the bring-up, sets_hostReady = falseand disposes the seat-host connection. Agents then got "The seat is not ready (error)" while the seat and its programs were still running.The viewer's Reconnect and Sign in… buttons are enabled in every state except during a reconnect or a display change, and their tooltip says "The seat and its programs keep running", so this path is reachable whenever a prerequisite goes missing under a running seat (a policy or update turning Remote Desktop off, for example).
Fix
ReconnectAsyncnow reads the prerequisites once, before it changes anything (and only for a press that can act: one during a reconnect, a display change, a stop or a quit still returns at once, without the probe, which can take about 1.5 s and used to run on the UI thread under the lifecycle lock). If one is missing, or the probe itself fails, and the seat's host is serving (_hostReady), it changes nothing: no cancelled bring-up, noerrorstate, the seat'sreadyordetachedstate and advice stay. It logs the refusal and tells the user in the viewer's status line what is missing and that the seat keeps running. With no running host (a failed or stopped seat), a refused Reconnect still cancels the bring-up and reportserrorwith the missing prerequisite, as before, using the same reading.Tests
refused reconnect keeps the seat(Cli/DiagnosticsChecks.cs,RefusedReconnect): a ready seat with a switchable prerequisite and an injected viewer change that, like the real one, gives up when its token is cancelled. A refused Reconnect and a refused Sign in each leave the seatreadywith its host; a live and a reconnect display change both succeed after each (the reconnect also proves the refused Sign in left automatic sign-in alone); a later viewer disconnect makes the seatdetachedwith its host, not a failed start; a refused Reconnect on the detached seat keeps its state and advice, and the automatic seat's display reconnect still brings it back; once the prerequisite returns, Reconnect reconnects as before.stop during display changemoved into the new check, which covers it more strictly (a live change only, before; now live and reconnect).viewer sign-in's existing case still covers a refused Sign in on a seat that isn't running (lastErrornames the missing prerequisite).selftest --quick): 6 of 6 caught: dropping the running-seat refusal (the old code), a refusal that keeps the state but still cancels the bring-up (fails with B-040's exact "stopped" reply on the reconnect change), one that setserrorordetached, one applied to seats that aren't running, and one that stops reporting the prerequisite for a stopped seat. Every restore byte-identical131aca4: the probe after the busy check, the refusal text with the missing setting first, two redundant counts dropped from the check, the CHANGELOG naming the listener and saying "a" display reconnect (a password seat's is refused earlier), and this description's button sentence. The mutations above were re-run on131aca4: 6 of 6 caught; the same reviewer found no credible problem in131aca4.f348f49found one P2, applied incefd3cd: the probe had moved out of the method'stry/catch, and the toolbar discards the task, so a probe that threw would fail silently. A probe failure is now logged and treated as a missing prerequisite (a running seat keeps its state; a seat that isn't running reportserrorwith the reason), and the quick check covers both. Final mutation run oncefd3cd: 7 of 7 caught (the six above plus a probe failure that lets Reconnect go ahead); evidence in the fixer's scratch folder.scripts\build.ps1 -QuickTest -OutputDirectory artifacts\pkg-build): passed: Release publish, 109/109 quick checks, docs consistentselftest --quick: 109/109Manual check in a real seat
Optional and low value: with a seat running, turn Remote Desktop off (Settings → System → Remote Desktop), press Reconnect in the viewer, and check the status line names the missing setting,
anode statusstill saysreadywithagentReady: true, and agents keep working; turn Remote Desktop back on. The quick check covers the logic; this only confirms the viewer's text.CHANGELOG: one bullet under 0.11.3, which isn't released yet.
🤖 Generated with Claude Code