fix(chat): keep session picker open during status renders#973
Conversation
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
Codex review: needs real behavior proof before merge. Reviewed July 13, 2026, 6:33 PM ET / 22:33 UTC. Summary Reproducibility: yes. from source and the linked report: status snapshots rerender the composer, and recreating the native ComboBox closes its flyout. A real current-main failing run was not independently executed during this read-only review. Review metrics: 2 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: Land the stable native-picker design after current-head Windows proof shows at least one active thinking/status render while the session flyout remains expanded and can complete a selection. Do we have a high-confidence way to reproduce the issue? Yes from source and the linked report: status snapshots rerender the composer, and recreating the native ComboBox closes its flyout. A real current-main failing run was not independently executed during this read-only review. Is this the best way to solve the issue? Yes. Preserving the caller-owned native ComboBox across unrelated declarative renders fixes the remount root cause, while structural snapshot and selection reconciliation preserve legitimate updates. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 77c42440376e. 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 (5 earlier review cycles)
|
|
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. |
Keep the session picker ComboBox attached as a declarative native child across FunctionalUI renders, and harden native control lifecycle handling for mount, ownership, and event routing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Pushed an update in Proof:
UI proof note: isolated launch reached first-run setup instead of the chat picker, and the focused WinUI UI tests still block before assertions with WinAppSDK bootstrap |
|
Follow-up UI proof from a non-isolated local run:
Relevant live AX proof: |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0da0de62b
ℹ️ 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".
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
Looking at the code, we should be updating our functionalUI combobox primitive to support our scenario. It apparently is using the new ComboBox() as a bypass since the combobox helper only supports flat string items: openclaw-windows-node/src/OpenClaw.Tray.WinUI/Chat/OpenClawComposer.cs at 77c42440376e2b8cd503290bb2… The fix should be to update the combobox primitive rather than creating a whole new native element UI lifecycle path. |
…Box primitive (supersedes #973) (#991) * fix(chat): keep session picker stable during renders * fix(chat): stabilize native session picker host Keep the session picker ComboBox attached as a declarative native child across FunctionalUI renders, and harden native control lifecycle handling for mount, ownership, and event routing. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(ui): clean up native controls across render paths * refactor(chat): replace NativeElement escape hatch with FunctionalUI ComboBox primitive The #970 fix originally reached around FunctionalUI with a raw NativeElement snapshot to keep the session picker open across status re-renders. Promote that into a first-class rich ComboBox primitive so re-render stability is a framework guarantee, not a per-call escape hatch. - Add ComboItem + ItemComboBoxElement with static ItemsEqual; ConfigureItemComboBox only rebuilds items when the list actually changes, preserving an open flyout. - Migrate both the session picker and the model picker (which had the same unfixed #970-class bug) onto the primitive. - Remove NativeElement and its tests entirely. - Add pure ItemComboBoxTests and source-contract ComposerSessionPickerTests guarding against escape-hatch regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: arturict <arturict@users.noreply.github.com> Co-authored-by: bakudies@microsoft.com <bakudies@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Superseded by #991, which contains all three commits from this PR and replaces the broad NativeElement lifecycle escape hatch with the reviewed FunctionalUI ComboBox primitive. |
4 participants
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 keeps the native session picker hosted as a stable child across FunctionalUI renders. A structural snapshot rebuilds its items only when visible group labels, session IDs, titles, or ordering actually change. Selection is reconciled independently, callbacks stay current, and native lifecycle and event cleanup are handled explicitly.
The renderer also removes stale paths when a caller-owned native element moves, and clears native children when virtualized subtrees recycle. WinUI test controls are created on the UI thread.
This keeps ordinary thinking/status renders from replacing the picker while still reflecting real session-list changes.
User Impact
The chat/session picker can remain open while status updates arrive, so users can reliably select another chat without suppressing live updates.
Validation
Validated from current head
9c1b9268:./build.ps1— passed.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 CRLF checkout.dotnet test ./tests/OpenClawTray.FunctionalUI.Tests/OpenClawTray.FunctionalUI.Tests.csproj --no-restore— 10 passed, 0 failed.dotnet test ./tests/OpenClaw.Tray.UITests/OpenClaw.Tray.UITests.csproj --filter FullyQualifiedName~FunctionalUiNativeElementTests -p:Platform=x64 -r win-x64— 6 passed, 0 failed../run-app-local.ps1 -NoBuild -AllowNonMain— launched the updated tray from this branch.Real behavior proof
A non-isolated local run opened
openclaw://chatwith the Companion Chat page connected. The nativeSessionpicker was opened and remained expanded after waiting, withOpenClaw Windows Traystill selected. The live accessibility state reportedAXPopUpButton (expanded) SessionandAXRow (selected) OpenClaw Windows Tray.The isolated UI-test environment still cannot complete WinAppSDK bootstrap (
Package dependency criteria could not be resolved), so the connected local run is the available runtime proof.Review
The current implementation was reviewed for stable native-child ownership, stale-path reparenting, virtual-stack cleanup, UI-thread affinity, mount and disposal behavior, event routing, snapshot invalidation, and callback suppression. Regression tests cover path changes and virtualized native-element recycling.
AI-assisted: yes. I reviewed and understand the stable-control lifecycle, item reconciliation, native event cleanup, and regression tests in this change.