fix(chat): keep session picker open during status renders#973
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 12, 2026, 6:19 AM ET / 10:19 UTC. Summary Reproducibility: yes. at source level with high confidence: current main recreates the native picker during composer renders, and #970 supplies concrete live reproduction steps. This review did not execute the Windows UI. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Keep the retained-control and structural-snapshot approach, then merge only after a redacted current-head recording shows the picker opened during Agent thinking, surviving multiple status renders, and successfully switching sessions. Do we have a high-confidence way to reproduce the issue? Yes, at source level with high confidence: current main recreates the native picker during composer renders, and #970 supplies concrete live reproduction steps. This review did not execute the Windows UI. Is this the best way to solve the issue? Yes. Retaining the existing native picker at FunctionalUI's stable render path addresses the remount cause without suppressing status updates, while the snapshot preserves legitimate session-list changes. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 06b4f6362594. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (1 earlier review cycle)
|
|
I reran the complete required validation from the current PR head in a clean Windows checkout with the repository's expected CRLF line endings. The earlier three failures were checkout-line-ending artifacts; the full tray suite is now clean: 1,672 passed, 0 failed. The full build also passes, and Shared tests report 2,739 passed / 31 skipped / 0 failed. I updated the PR body with the exact results. The connected Agent-thinking WinUI recording remains explicitly blocked because the isolated app has no configured gateway/session, so I am leaving the PR as draft rather than overstating that proof. |
Closes #970
What Problem This Solves
Thinking-status snapshots rerender the chat composer. The grouped session picker was recreated inside the render setter each time, replacing the native
ComboBoxand closing its open flyout before a user could switch chats.Why This Change Was Made
The composer now retains one native session
ComboBoxacross renders. A structural snapshot rebuilds its items only when the visible group labels, session IDs, titles, or ordering actually change. Selection is reconciled independently, callbacks stay current through a ref, and internal reconciliation suppressesSelectionChangedcallbacks in atry/finallyblock.This keeps ordinary thinking/status renders from touching the picker instance or its items while still reflecting real session-list changes.
User Impact
The chat/session picker can remain open while an agent is thinking, so users can reliably select another chat without suppressing live status updates.
Validation
./build.ps1— passed; Shared, CLI, WinNode CLI, SetupEngine, and WinUI built successfully.dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore— 2,739 passed, 31 skipped, 0 failed.dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore— 1,672 passed, 0 failed in a clean Windows checkout with the repository's expected CRLF line endings.git diff --check— passed.Real behavior proof
The current-head WinUI executable built and launched successfully on Windows. The isolated/fresh app stopped at the required first-run setup screen, so a connected
Agent thinkingsession was not available without configuring a gateway. End-to-end flyout video proof is therefore not verified / blocked in this environment.The focused regression proof covers both parts of the render contract:
UseRefand gates item rebuilding on the structural snapshot.Review
A rubber-duck review checked that the update guard is restored by
finally, the event handler is registered once, the snapshot includes every visible field plus order and IDs, and selection reconciliation does not mutate items or emit an external callback.AI-assisted: yes. I reviewed and understand the stable-control lifecycle, item reconciliation, callback suppression, and regression tests in this change.