Repository navigation
fix: retain the selected assistant and conversation after reconnect - #1795
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. What this changesPreserves assistant and conversation selection across reconnects while retaining legacy session access when assistant listing is unsupported. Example: Reconnect while using assistant research and conversation agent:research:fixture-other.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR: current main still loses the remembered assistant during transient roster clearing, and the patch provides a useful, tested repair with no actionable findings. Priority: P2 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe authenticated Gateway client publishes roster observations through GatewayService and AppState; Workspace projects those observations into assistant selection, conversation rows, and creation readiness. flowchart LR
Gateway[Authenticated Gateway] --> Client[Operator client]
Client --> Service[GatewayService]
Service --> State[AppState]
State --> Selection[Workspace selection]
Selection --> Window[Workspace controls]
Window --> Composer[Native composer]
Composer --> Gateway
Technical reviewBest possible solution: Land the focused continuity repair through the existing client, application-state, and Workspace owners. Do we have a high-confidence way to reproduce the issue? Current-main source shows disconnect clearing the roster and selection projection returning null; submitted native proof exercises the repaired reconnect path, but this review ran no target code. Is this the best way to solve the issue? The patch separates remembered intent from roster availability in one focused owner, preserves explicit unsupported-method fallback, and avoids a second connection or chat implementation. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 2ff4d168588d. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes: No label changes. Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (5 earlier review cycles)
Reviewed November 2, 2026, 5:46 PM ET / 22:46 UTC (Revision 6). |
|
Update from GitHub Copilot triage (not Scott personally): I closed this draft during a batch review without first getting Scott's approval. That was my mistake, and the PR is open again. Sorry for the confusing message. The selected-assistant and conversation continuity problem is worth solving. The current review asks that gateways without |
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
|
Native Windows validation of #1795 Windows validation report (roboclaw/validate-pr-1795-20261027)1. Identity
2. Host
3. Commands and results (attempt 2, evidence
|
| Lane | Command | Exit | TRX total/executed/passed/failed/skipped |
|---|---|---|---|
| prereq diag | setup-dev.ps1 -CheckOnly |
0 | n/a |
| build | .\build.ps1 |
0 | n/a |
| Shared | dotnet test ...Shared.Tests --no-restore (trx) |
0 | 4677 / 4652 / 4652 / 0 / 25 not run (integration-gated) |
| Tray | dotnet test ...Tray.Tests --no-restore (trx) |
0 | 4585 / 4585 / 4585 / 0 / 0 |
| docs | validate-docs.ps1 |
0 | n/a |
| roster wire | run-proof-tests.ps1 ... SessionRosterWireTests |
0 | 3 / 3 / 3 / 0 / 0 |
| navigation | run-proof-tests.ps1 ... WorkspaceNavigationTests|NativeCompletionPresentationTests |
0 | 135 / 135 / 135 / 0 / 0 |
| native continuity | run-proof-tests.ps1 UITests, handoff filter, -RuntimeIdentifier win-x64, OPENCLAW_RUN_GATEWAY_FIXTURE_UI=1, screenshots on |
0 | 3 / 3 / 3 / 0 / 0 |
| broader smoke (optional) | test-gateway-fixture.ps1 -AppPath <exe> -Screenshots |
1 | profile/app 32/32 passed; fixture-ui 34 / 34 / 30 / 4 / 0 (classified below) |
Native continuity cases that passed: SelectedAssistantAndConversationSurviveTransientRosterLossButNotAuthoritativeRemoval, UnsupportedAgentListing_PreservesLegacySessionsAndDefaultCreation, WindowManagerNativeHandoffTests.ShutdownRejectsNativeHandoffWithoutOpeningAWindowOrWaitingForConnection.
4. Controls
- No failing-before control was requested for fix: retain the selected assistant and conversation after reconnect #1795. Owner isolation for the smoke failures used main and original-head comparators (section 6).
5. Inspected evidence (fixture-backed: real Companion/client/UI, synthetic loopback Gateway, no provider or pairing)
continuity-before.png: Fixture Research selected; "Fixture: research conversation" selected with accent bar.continuity-transient.png(agents.list held): selector shows "No agents loaded" and is disabled (UIAIsEnabled=falseasserted); transcript stays visible; session list shows "No conversations to show".continuity-returned.png: same assistant and conversation restored; UIA asserted selector enabled and session selected.- Observation: the accent bar is on Home, and the conversation row has only the selected-gray fill. In
before, the bar was on the conversation. UIA selection assertions pass. This may be the selection indicator still animating at capture time. A settled recapture would decide whether it's a stale navigation highlight. Not classified as a defect.
- Observation: the accent bar is on Home, and the conversation row has only the selected-gray fill. In
continuity-removed.png: authoritative roster without Research leaves "Select assistant" and requires a fresh choice.continuity-unsupported-*.png:agents.listunsupported keeps legacy sessions listed and creates "new conversation 1" with the default target.- Protocol (
gateway-requests.jsonl, continuity run): 3connect, 7agents.list, plus history/sessions/models reads, allok; nochat.send, no errors. - Captures are owned-HWND PrintWindow of the app window with synthetic data only. No redaction needed.
6. Failures retained and classified
- Attempt 1 (
1795-81b97e2a...):build.ps1andsetup-dev -CheckOnlyexit 1 ("Missing compatible Visual Studio C++ Redistributable"). Tray had 4584/4585 pass, 1 failure:NativeSpeechStackRuntimeTests.TestHost_AppLocalVCRuntime_MeetsOnnxRuntimeFloor(fallback CRT 14.29 < 14.38). Environmental: my guest prereq installedVC.Redist.14.Latestwithout the x64 CRT payload. Fixed by addingVC.Tools.x86.x64. Attempt 2 is fully green. - Broader smoke, 4 of 34 failed. Each was rerun on the same guest at the candidate, main
48cb8a18, and original head182aee93:ComposerNew_InitialConversationBackAndForwardandComposerNew_DelayedCompletionRetainsNotificationsfail on all three (unexpectedprojects.listreturningMETHOD_NOT_FOUND). This is a proven base failure on main, not caused by fix: retain the selected assistant and conversation after reconnect #1795.projects.listcallers exist in both parents (ChatPlacement.cs); the fixture doesn't serve it.SidebarSwitchesRealHistoriesAndShowsMessage240AtBothWidthsfails on all three ("host did not provide two meaningfully different window widths"). Environmental: the guest is 1024x768.BackgroundMediaRetransmitAndRetainedToolOutcomePreserveNewerNativeTurntimed out once in the full smoke, then passed on the candidate rerun and both comparators. Retained as an unresolved timing/order issue, not reproduced in isolation.
7. Reviews, decisions, unrun lanes
- No new source. No repair proposed for fix: retain the selected assistant and conversation after reconnect #1795.
- Ordinary composer send into the retained conversation after reconnect: UNVERIFIED at 47c2a5c. No existing fixture test covers it.
ReconnectKeepsDefinitelyUnsentQueuedMessageFailedAndVisiblecovers a different case: a pre-reconnect queuedapp.chat.sendthat fails visibly. Earlier send evidence belongs to 182aee9 and stays historical. No test was added, per instructions. - Follow-ups outside this PR: the main-branch
projects.listfixture gap breaks theComposerNew_*fixture tests. Settled recapture of the returned sidebar indicator. - Unrun: ARM64, live provider/pairing (not needed for this fixture contract), larger-display rerun of
AtBothWidths.
8. Cleanup
- Guest released via
crabbox stop --provider hyperv cbx_4b1664424425;crabbox listandGet-VMconfirm it's gone. - Evidence:
files\evidence\1795\evidence-1795.zip(sha256 B94F95B60945751FF2989E948015A0ABBEFC364A18A7FF1180C09D0E396C8F6C), unpacked underfiles\evidence\1795\unpacked, sidebar crops underfiles\evidence\1795\crops. No credentials (synthetic fixture tokens only).
182aee9 to
90dd8b8
Compare
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
Extract the independently reviewed reliability seam without the held native timeout consumer. No change to executor releases, package pins or sandbox enforcement.
99e9597 to
a7c170a
Compare
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
Resolve conflicts with the assistant-selection refactor (#1795): pins and Home now resolve against SelectedAgentId, and the sidebar projection keeps this PR's HomeSessionKey and Sessions(mainSessionKey) next to main's IsAgentSelectionReady. Retire the Skills pin because main removed the Skills page (#1932): the catalog drops skills (a saved skills tag stays persisted but hidden, like bindings), and the pin tests, fixture UI test and ARCHITECTURE ledger follow. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
What Problem This Solves
Fixes: reconnecting or reloading Hub data can move the visible assistant or conversation away from the user-selected target.
User Impact
User impact: users retain their intended assistant and conversation through transient roster loss. A Gateway that explicitly does not support
agents.listcan still show its legacy conversations and create a Gateway-default conversation rather than waiting forever for a roster.Source tracing resolved the apparent
composeTarget=agent:main:maindiscrepancy: that field is the Gateway handshake's canonical default-send target, not mounted-window selection readback. The native composer sendsCurrentThread.Idexplicitly. Local MCP without an explicit thread uses canonical ComposeTarget before remembered DefaultThreadId. No compose-target change or external Companion create/select API is introduced here.Why This Change Was Made
Keep assistant/session selection with its existing state owner and preserve it across refresh and reconnection. Do not replace the authoritative connection manager or introduce a separate chat owner.
The introduced unsupported-listing gate failure is corrected with a typed observation through the real Gateway client and existing owners. Explicit unsupported fallback and a cached authoritative roster cannot coexist; a false/reset observation is not a fresh roster. Disconnected menus keep connection guidance without authorizing session creation. Lost native-handoff source-marker guards are replaced by actual held-response/full-app and shutdown behavior checks, not retargeted marker assertions.
Continuity and onboarding automation remove different, non-overlapping old cases in
NativeCompletionPresentationTests.cs. Rebase whichever lands second before final review.Evidence
Rebase on main (
a7c170a5, current head)Current head
a7c170a5e69267df75daff7d1d464f02c6a0b8aeis99e9597brebased cleanly ontomainat2ff4d168(includes #1941, #1791 and #1797); no conflicts and no content changes. Local:./build.ps1passed;OpenClaw.Shared.Tests4812 passed, 25 skipped, 0 failed;OpenClaw.Tray.Tests4799 passed, 0 failed;OpenClaw.Tray.UITestscompiles; docs validation passed. Throwaway crabbox Hyper-V guest (leasecbx_0333aede319c, exact head): fixture profile/app 34/34. Fixture UI 45/47:SidebarSwitchesRealHistoriesAndShowsMessage240AtBothWidthsneeds a wider display than the 1024x768 guest.ReconnectKeepsDefinitelyUnsentQueuedMessageFailedAndVisiblefailed 1 of 5 reruns on this head and 3 of 4 onmainin the same guest, so it is pre-existing.Hosted CI on this head is green. The current-head proof below is from
99e9597b, which has the same product and test content.Proof on
99e9597b(pre-rebase)Head
99e9597b63774b9c0ba509330b818f8e4e442e9d= the rebase onmain(90dd8b83) plus a test-only commit. That commit adds an ordinary composer send toSelectedAssistantAndConversationSurviveTransientRosterLossButNotAuthoritativeRemovalafter the held/released roster reconnect, and requires the fixture Gateway to record a completedchat.sendfor the restored conversation. Product code is unchanged from90dd8b83.Local on this head:
./build.ps1passed;OpenClaw.Shared.Tests4810 passed, 25 skipped, 0 failed;OpenClaw.Tray.Tests4778 passed, 0 failed;OpenClaw.Tray.UITestscompiles;scripts/validate-docs.ps1passed.Run in a throwaway crabbox Hyper-V guest (lease
cbx_cbc09cb6594f, Windows 11 Insider 10.0.29683 x64, 1024x768, interactive session, WindowsAppRuntime 2.6.4). The head was cloned and pinned by commit and tree; the app was built in-guest (AppVersion ...Sha.99e9597b).scripts/test-gateway-fixture.ps1 -Screenshots: fixture profile/app 34/34; fixture UI 44/46. The continuity cases pass:SelectedAssistantAndConversationSurviveTransientRosterLossButNotAuthoritativeRemoval(including the new send),UnsupportedAgentListing_PreservesLegacySessionsAndDefaultCreationandVerifiedSetupBindingClearsBeforeNewConversationResponse. Two failures are not continuity cases.SettingsDisconnectPruning_...(connection)failed once on a blank-capture check and passed 3/3 when rerun without screenshots.SidebarSwitchesRealHistoriesAndShowsMessage240AtBothWidthsreports "The host did not provide two meaningfully different window widths", because the guest display is 1024x768 and too narrow for that case; it is environmental. CI runs both cases.agents.listis held, releases the roster and returns to chat. The same assistant and conversation are selected. Finally it sends throughChatComposerInputandChatComposerPrimaryAction. The fixture Gateway's record ends withchat.send session=agent:research:fixture-other outcome=ok(request log).agents.list: legacy sessions + default creationAll data is synthetic fixture data. More captures and UIA metadata are on assets-only commit
b244f35d(1795/). The guest has been deleted.Rebase on main (
90dd8b83, product code unchanged at the head above)Current head
90dd8b83fa384752d12582d5bb8c56435b202fd9is a rebase of182aee93ontomainat4cbbe51c; it adds no new behavior. Conflicts came from #1911 (Settings and Chat in one Companion window):WorkspaceWindowkeeps main's retained Settings host and replaces_agentIdwithWorkspaceAgentSelection/SelectedAgentIdeverywhere, including the Settings and command-catalog agent fallbacks;59c47b16became empty because that routing is now in the first commit. The source-inspection testNativeChatHandoff_UsesWorkspaceBindingWithoutReopeningCompanionChatstays deleted as in the original change.GatewayFixtureWarningUiTestskeeps main's progress-window/Workspace checks plus the new binding-invalidation case;docs/ARCHITECTURE.mdanddocs/ACCESSIBILITY.mdkeep both sides. On this head:./build.ps1passed;OpenClaw.Tray.UITestscompiles;OpenClaw.Shared.Tests4810 passed, 25 skipped, 0 failed (includesSessionRosterWireTests3/3);OpenClaw.Tray.Tests4778 passed, 0 failed (includesWorkspaceNavigationTests). Windowed UI and GatewayFixture cases were not rerun locally (no desktop windows); CI runs them. Real-app evidence below is from182aee93.Pre-rebase head:
182aee9334edacb5291b7b13a8ee43deb5fb4b92, based on main2b7cfce17bb496ec5d8f0c563fb71ebb46e015fa. Published SDK 1.0.0 pins are unchanged. Current-head CI has 15 successful checks, zero failed checks, including full UI.New standalone real-pixel proof: the independent
182aee93executable ran on the authorized Default-desktop slot with its own throwaway profile and disposable loopback Gateway fixture. No startup/fixture sibling changes are in this executable. The earlier combined preflight remains separate and is not relabeled.The native selector and conversation were chosen through UIA. The actual Gateway WebSocket closed/reconnected while agents.list was held and then released. The same agent/conversation returned. An ordinary native composer send, not
app.chat.sendand with no explicit send-thread override, reached that conversation's actual Gateway session key. A controlled fixture reply rendered in the retained thread.Inspect the actual selected UIA state and routing record, binary/image/cleanup provenance and capture qualification. All three public image URLs returned HTTP200 and byte-identical sanitized files. Visible content is entirely synthetic fixture data; inspection found no endpoint, credential or real user information. PNG metadata was removed without changing any RGBA pixels.
Change Type
Scope
winnodeRequired proof pools
windows-winui-interactive: visible assistant/session selection and reconnect continuity.windows-wsl-gateway-e2e: actual paired Gateway reconnection and continued selected-session routing.Validation
Independent current source retains its complete mandatory closeout:
.\build.ps1dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restoredotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore182aee93) hosted Build and Test, including full UIThe current app was rebuilt with
-r win-x64 -p:Platform=x64 -p:DevBuild=false.Its embedded version contains the complete 182aee9 SHA and its actual binary hash is
in the public diagnostics. The visible run used an explicit 90-second expiry,
quiet error-mode/WER controller scopes, inherited error-mode flags, no shell
error dialog and exact PID/path/start binding. Capture was strict-nonblank
PrintWindow of the owned HWND only, never a desktop capture. The slot was
released immediately after exact cleanup, before sanitization/publication.
Earlier outcomes remain attributed: combined291 executed7with6passes/1startup
token-read sharing error, then separate same-source handoff1pass; combined2dcf
native menu2passes and later driver preflight passed. Neither is this standalone
run. Both earlier introduced popup-query CI races were fixed in182and their
failed runs remain retained.
A separate startup-only baseline control failed at the new pending-roster
disablement check. It did not complete the after-reconnect/send sequence, so it
is not credited as a full base-failure/head-pass comparison for that
contract. The current independent runtime evidence above is direct; no missing
baseline result is invented.
Real Behavior Proof
99e9597b63774b9c0ba509330b818f8e4e442e9d(see "Current-head proof"). The older real-app items below are from pre-rebase182aee93, not combined2dcf. Full AppVersion and binary hash are in the linked diagnostics.ChatComposerInput; normal Invoke onChatComposerPrimaryAction; observe the actual Gateway request; publish a controlled owned-fixture reply and observe it in the native timeline.Actual safe record produced by the owned Gateway after ordinary native Send:
{ "Method": "chat.send", "SessionKey": "agent:research:fixture-other", "Outcome": "ok", "origin": "Native composer ValuePattern and primary Invoke. No app.chat.send call or explicit send-thread override." }The origin string is the driver's invocation annotation; Method, SessionKey and
Outcome come from the actual Gateway request record. After the controlled reply
rendered, native UIA still reported the same assistant and selected
conversation at 22:35:29.8146461Z. Exact app PID 29172 exited by 22:35:30.5526281Z.
Yes: three inspected owned-window frames and sanitized actual UIA/routing/provenance JSON are public and verified. No endpoint, token, user path or raw profile was uploaded.ComposeTarget remains the canonical handshake default-send target, not mounted
selection readback. This proof uses native UIA for selection and the actual
ordinary-send wire record for routing; it introduces no external Companion
create/select API and does not equate ambient Gateway ownership with UI choice.
Security Impact
NoNoNoNoNoYes, explain the risk and mitigation: No new capability, token handling, network endpoint or data-access scope. Existing authenticated chat/connection owners are preserved. Native selected-target reconnect and ordinary-send proof is supplied above; canonical MCP default-target semantics are unchanged.Compatibility and Migration
Yes. Existing authenticated connection, consent and ownership contracts remain authoritative.Nopersistent source-contract change.NoReview Conversations
Pre-rebase head (
182aee93) hosted review passed: Overall diamond lobster (5/6), Proof diamond lobster (5/6), Patch diamond lobster (5/6). No merge is authorized.