Skip to content

Redesign Sessions page (Fluent 2 + deep-link to Chat)#514

Merged
shanselman merged 2 commits into
openclaw:masterfrom
bkudiess:sessions-page
May 22, 2026
Merged

Redesign Sessions page (Fluent 2 + deep-link to Chat)#514
shanselman merged 2 commits into
openclaw:masterfrom
bkudiess:sessions-page

Conversation

@bkudiess
Copy link
Copy Markdown
Contributor

@bkudiess bkudiess commented May 22, 2026

Restructures SessionsPage to match the Permissions/Connection shell and removes UI drift against the openclaw-design skill.

Highlights

  • Layout: ScrollViewer + StackPanel (MaxWidth=900, 24 px page padding, 4 px grid throughout) — matches every other app page.
  • Status dot is now a 4-state Fluent map with a hover tooltip:
    State Brush Trigger
    🟢 Running SystemFillColorSuccessBrush Status ∈ {active, running}
    🟡 Aborted last run SystemFillColorCautionBrush AbortedLastRun == true
    🔴 Error SystemFillColorCriticalBrush Status ∈ {error, failed, failure}
    ⚪ Idle SystemFillColorNeutralBrush everything else
  • Primary action is an AccentButton ("Open chat") — text-only Fluent 2 button so AccentButtonStyle owns rest/hover/press/disabled brushes per spec.
  • Destructive actions moved into a MenuFlyout behind the row's overflow () button: Reset, Compact, Delete (Delete uses SystemFillColorCriticalBrush).
  • Cron sessions filtered out (key shape agent:<id>:cron) — they overcrowd the conversation list and have a dedicated Cron page.
  • Deep-link to Chat: clicking Open chat stores the session key on HubWindow.PendingChatSessionKey and navigates; ChatPage.ShowFunctionalSurface consumes the key and remounts OpenClawChatRoot with initialThreadId. Both the timeline and the composer's session dropdown render the selected session on first paint.
  • Robust MenuFlyout click routing via ResolveSessionKey — falls back to MenuFlyout.Target when DataContext propagation into popup items fails (canonical WinUI binding quirk).
  • Removed emoji empty-state and ad-hoc LimeGreen/Gray/IndianRed colors; all glyphs are Segoe Fluent (PUA) values mirrored from FluentIconCatalog.

Files

  • src/OpenClaw.Tray.WinUI/Pages/SessionsPage.xaml / .xaml.cs
  • src/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.cs (session-key consumer + thread remount)
  • src/OpenClaw.Tray.WinUI/Windows/HubWindow.xaml.cs (PendingChatSessionKey field)

Validation

  • ./build.ps1
  • dotnet test OpenClaw.Shared.Tests --no-restore1891 passed, 29 skipped
  • dotnet test OpenClaw.Tray.Tests --no-restore1178 passed
  • Manual smoke: rows render in light/dark, status dot tooltip works, Open chat deep-links into Chat with composer dropdown pre-selected, Reset/Compact/Delete fire from the overflow menu, cron-slot sessions no longer appear.
image

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 22, 2026

Codex review: needs real behavior proof before merge.

Latest ClawSweeper review: 2026-05-22 23:02 UTC / May 22, 2026, 7:02 PM ET.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The branch redesigns SessionsPage into a Fluent scrollable layout, filters cron sessions, moves row actions to an overflow menu, and adds a Sessions-to-Chat pending-session handoff through HubWindow and ChatPage.

Reproducibility: yes. for the review finding: source inspection of the PR head shows new hard-coded SessionsPage strings without matching resource entries. The Chat handoff itself is not reproduced in a live app run; the submitted screenshot is not enough to verify that path.

PR rating
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Summary: The PR has useful UI direction and a plausible source-level handoff fix, but it remains not quality-ready because proof is incomplete and localization coverage regresses.

Rank-up moves:

  • Add resource-backed entries for the new SessionsPage labels, tooltips, automation names, and status tooltip strings across existing locales.
  • Post redacted real UI proof showing Open chat selects the clicked session after changing Chat's dropdown.
  • Show or log that Reset, Compact, and Delete still target the correct row from the overflow flyout.
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.

Real behavior proof
Needs stronger real behavior proof before merge: The screenshot shows the redesigned Sessions list, but not the after-fix Chat thread handoff or overflow action routing; add redacted recording/live output/logs and update the PR body for re-review, or ask a maintainer to comment @clawsweeper re-review if it does not rerun automatically.

Mantis proof suggestion
A desktop recording would directly exercise the visible Sessions row, selected-thread handoff, and overflow action routing that the screenshot does not prove. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify Sessions Open chat selects the clicked session after changing Chat's dropdown, and show overflow Reset/Compact/Delete routing with private details redacted.

Risk before merge

  • Merging as-is would leave new SessionsPage labels, tooltips, automation names, empty-state copy, and status tooltips in English on non-English builds despite the current localized resource baseline.
  • The screenshot proves the redesigned list renders, but it does not prove Open chat selects the clicked thread in Chat or that Reset/Compact/Delete still route to the intended row from the flyout.
  • The PR changes Chat's active thread selection path; if the handoff is wrong in an unobserved edge case, the next composed message can target the wrong session.

Maintainer options:

  1. Localize and prove the handoff (recommended)
    Restore resource-backed SessionsPage strings and require redacted UI proof that Open chat selects the clicked session after the Chat dropdown has been changed.
  2. Split visual cleanup from routing
    Maintainers can land the layout cleanup separately and keep the Chat selected-thread handoff in a narrower PR with focused proof.
  3. Accept source-only routing risk
    Maintainers may merge after explicitly accepting that the session-routing behavior has source inspection and claimed validation but no observed handoff proof.

Next step before merge
Contributor or maintainer action is needed because the PR needs a localization code fix plus real behavior proof from an observed UI run; automation cannot supply contributor-side proof.

Security
Cleared: The diff changes WinUI layout and in-process chat/session selection state; I found no concrete security or supply-chain regression.

Review findings

  • [P2] Localize the new Sessions page strings — src/OpenClaw.Tray.WinUI/Pages/SessionsPage.xaml:190-192
Review details

Best possible solution:

Keep the Fluent Sessions redesign and deep-link after all new visible/accessibility strings are resource-backed across locales and observed proof shows Open chat selects the clicked session while overflow actions target the correct row.

Do we have a high-confidence way to reproduce the issue?

Yes for the review finding: source inspection of the PR head shows new hard-coded SessionsPage strings without matching resource entries. The Chat handoff itself is not reproduced in a live app run; the submitted screenshot is not enough to verify that path.

Is this the best way to solve the issue?

No, not as submitted. The direction is maintainable, but the best path is to preserve localization coverage and add observed proof for the session handoff and flyout routing before merge.

Label justifications:

  • P2: This is a normal-priority UI improvement with limited blast radius but a concrete localization regression and proof gap before merge.
  • merge-risk: 🚨 message-delivery: The PR changes which Chat thread is selected from a Sessions row, so a bad handoff can send the next message to the wrong session.
  • merge-risk: 🚨 session-state: The PR adds pending session state on HubWindow and forces Chat remounts, which can mis-associate selected session state if the handoff path is wrong.
  • rating: 🧂 unranked krab: Current PR rating is 🧂 unranked krab because proof is 🦪 silver shellfish, patch quality is 🦐 gold shrimp, and The PR has useful UI direction and a plausible source-level handoff fix, but it remains not quality-ready because proof is incomplete and localization coverage regresses.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The screenshot shows the redesigned Sessions list, but not the after-fix Chat thread handoff or overflow action routing; add redacted recording/live output/logs and update the PR body for re-review, or ask a maintainer to comment @clawsweeper re-review if it does not rerun automatically.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The screenshot shows the redesigned Sessions list, but not the after-fix Chat thread handoff or overflow action routing; add redacted recording/live output/logs and update the PR body for re-review, or ask a maintainer to comment @clawsweeper re-review if it does not rerun automatically.

Full review comments:

  • [P2] Localize the new Sessions page strings — src/OpenClaw.Tray.WinUI/Pages/SessionsPage.xaml:190-192
    The redesigned row adds visible and accessible text such as Open chat, its tooltip, and its automation name without an x:Uid or resource-backed equivalent, while the current Sessions page is localized through .resw entries. Non-English builds will now show these new controls in English; please add resource entries for the new visible strings, tooltips, automation names, and status text across the existing locale files.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.87

Acceptance criteria:

  • ./build.ps1
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore

What I checked:

Likely related people:

  • bkudiess: Recent current-main SessionsPage and resource history includes the localized baseline and Sessions page edits that this PR now changes. (role: recent Sessions/localization area contributor; confidence: medium; commits: d4b35d4941c0, 6544d03ec219; files: src/OpenClaw.Tray.WinUI/Pages/SessionsPage.xaml, src/OpenClaw.Tray.WinUI/Pages/SessionsPage.xaml.cs, src/OpenClaw.Tray.WinUI/Strings/en-us/Resources.resw)
  • Christine Yan: Recent merged chat work introduced and maintained OpenClawChatRoot, MountFunctionalChat, initialThreadId, session switching, and scroll-preservation behavior affected by the deep-link handoff. (role: chat feature owner; confidence: high; commits: cd4acf013e38, 488f26509ced; files: src/OpenClaw.Tray.WinUI/Chat/OpenClawChatRoot.cs, src/OpenClaw.Tray.WinUI/Chat/FunctionalChatHostExtensions.cs, src/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.cs)
  • Ranjesh Jaganathan: git blame ties the current ChatPage.ShowFunctionalSurface mounting path that the PR modifies to recent current-main work. (role: recent ChatPage area contributor; confidence: medium; commits: 8dea8acbae8f; files: src/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.cs)
  • Scott Hanselman: Recent current-main work changed shared async loading behavior on SessionsPage, which the redesign continues to rely on. (role: recent list-page contributor; confidence: medium; commits: 1fe8e5216ca7; files: src/OpenClaw.Tray.WinUI/Pages/SessionsPage.xaml.cs, tests/OpenClaw.Tray.Tests/AsyncListLoadingPageWiringTests.cs)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 0afeaca8f87f.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. labels May 22, 2026
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 22, 2026

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@clawsweeper clawsweeper Bot added the proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. label May 22, 2026
bkudiess pushed a commit to bkudiess/openclaw-windows-node that referenced this pull request May 22, 2026
_mountedThreadId only records what we asked for, not what the user

later picked inside OpenClawChatRoot's composer dropdown. Comparing

against it could leave the chat root selected on a different session

than the row the user just clicked, sending the next message to the

wrong thread (raised in PR openclaw#514 review).

Treat any pending session key from SessionsPage as an unconditional

remount so the deep-link always wins.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bkudiess and others added 2 commits May 22, 2026 15:56
Restructures SessionsPage to match the Permissions/Connection shell

(ScrollViewer + StackPanel, MaxWidth 900, 4 px grid) and removes UI

drift against the openclaw-design skill:

- Replace literal emoji empty-state and ad-hoc colors with Segoe

  Fluent glyphs and SystemFillColor* theme brushes.

- Status dot is now a 4-state Fluent map (Success / Caution /

  Critical / Neutral) with a hover tooltip describing the state.

- Primary action is an AccentButton 'Open chat' (Fluent 2

  rest/hover/press/disabled via AccentButtonStyle, no Foreground

  overrides); destructive Reset/Compact/Delete moved to a MenuFlyout

  behind the row's overflow button.

- Filter out cron-spawned sessions (key slot == 'cron') so the

  conversation list mirrors the macOS companion.

- Deep-link from a session row to the Chat surface: SessionsPage

  stores the session key on HubWindow.PendingChatSessionKey;

  ChatPage.ShowFunctionalSurface consumes it and remounts the

  OpenClawChatRoot with initialThreadId, so both the timeline and

  the composer's session dropdown render the requested session.

- Robust MenuFlyout click routing via ResolveSessionKey, which

  walks back to MenuFlyout.Target when DataContext propagation into

  popup items fails (canonical WinUI binding quirk).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
_mountedThreadId only records what we asked for, not what the user

later picked inside OpenClawChatRoot's composer dropdown. Comparing

against it could leave the chat root selected on a different session

than the row the user just clicked, sending the next message to the

wrong thread (raised in PR openclaw#514 review).

Treat any pending session key from SessionsPage as an unconditional

remount so the deep-link always wins.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@shanselman shanselman merged commit 07b1910 into openclaw:master May 22, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 message-delivery 🚨 Merging this PR could drop, duplicate, misroute, suppress, or wrongly target messages. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P2 Normal priority bug or improvement with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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.

2 participants