Skip to content

fix(chat): keep session picker open during status renders#973

Draft
arturict wants to merge 1 commit into
openclaw:mainfrom
arturict:fix/970-stable-session-picker
Draft

fix(chat): keep session picker open during status renders#973
arturict wants to merge 1 commit into
openclaw:mainfrom
arturict:fix/970-stable-session-picker

Conversation

@arturict

@arturict arturict commented Jul 12, 2026

Copy link
Copy Markdown

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 ComboBox and closing its open flyout before a user could switch chats.

Why This Change Was Made

The composer now retains one native session ComboBox across 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 suppresses SelectionChanged callbacks in a try/finally block.

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 thinking session 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:

  • structurally equivalent session groups from a new render match the retained snapshot, so the picker items are not cleared or rebuilt;
  • changed titles or ordering invalidate the snapshot and trigger reconciliation;
  • a source contract verifies that the composer retains the native picker via UseRef and 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.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 Urgent regression or broken agent/channel workflow affecting real users now. labels Jul 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 12, 2026, 6:19 AM ET / 10:19 UTC.

Summary
The PR retains one native grouped session ComboBox across composer renders, rebuilds its items only when a structural session snapshot changes, reconciles selection under callback suppression, and adds focused tray tests.

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.

  • Patch surface: 5 files, +159/-49. The patch is focused on one native picker lifecycle, two small extracted types, and regression tests.
  • Regression tests: 3 added. Coverage checks structural equivalence, title/order invalidation, and the intended retained-control source shape.
  • Required validation: 3 required commands reported passing. The PR now reports the full build, shared tests, and tray tests clean on the current head.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #970
Summary: This PR is the direct candidate fix for the canonical render-triggered session-picker issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Post a redacted current-head WinUI recording showing the open picker survive repeated Agent thinking renders and complete a session switch.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR reports clean builds/tests and a successful app launch, but explicitly did not exercise the changed behavior in a connected session; add a redacted current-head recording, update the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] The defining user-visible result has not been observed after the fix: automated and source-contract tests cannot establish that the native flyout stays open and selectable during repeated live status renders.

Maintainer options:

  1. Decide the mitigation before merge
    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.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] No concrete source defect warrants an automated repair; the contributor must supply real connected WinUI behavior proof before merge.

Security
Cleared: The focused WinUI state-management and test diff adds no dependency, permission, secret, downloaded artifact, or other concrete security or supply-chain concern.

Review details

Best 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 changes

Label justifications:

  • P1: The PR addresses a broken chat-navigation workflow that makes session switching unreliable while an agent is actively thinking.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR reports clean builds/tests and a successful app launch, but explicitly did not exercise the changed behavior in a connected session; add a redacted current-head recording, update the PR body to trigger re-review, or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • christineyan4: Authored the grouped ID-based session-picker implementation whose native control lifecycle this PR changes. (role: feature introducer; confidence: high; commits: a482ca9e1713; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawComposer.cs)
  • calebeden: Recent current-main work touches FunctionalUI render identity and adjacent chat composer rerender behavior. (role: recent renderer and chat contributor; confidence: high; commits: 02f2421a5e75, c9b5476a873f; files: src/OpenClawTray.FunctionalUI/FunctionalUI.cs, src/OpenClaw.Tray.WinUI/Chat/OpenClawComposer.cs)
  • Scott Hanselman: Merged the original grouped-picker work and appears in baseline composer provenance for the current picker block. (role: merger and baseline area contributor; confidence: medium; commits: 604e7e195cdb, 4166e0fd63f8; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawComposer.cs, src/OpenClawTray.FunctionalUI/FunctionalUI.cs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-07-12T09:34:46.688Z sha 2e36a17 :: needs real behavior proof before merge. :: none

@arturict

Copy link
Copy Markdown
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows Companion: "Agent thinking" animation closes the chat/session selectbox before a different chat can be chosen

1 participant