Skip to content

fix(ui): disable session selection outside chat page to prevent confusion#97257

Closed
forrest0x59 wants to merge 1 commit into
openclaw:mainfrom
forrest0x59:fix/disable-session-selection-outside-chat-page
Closed

fix(ui): disable session selection outside chat page to prevent confusion#97257
forrest0x59 wants to merge 1 commit into
openclaw:mainfrom
forrest0x59:fix/disable-session-selection-outside-chat-page

Conversation

@forrest0x59

Copy link
Copy Markdown
Contributor

Summary

After commit 2b30951 (landed in 2026.5.31-beta.4), from a UI interaction perspective, the chat session selection component was changed from a local control to a global one. While this works fine within the chat page, it causes confusion on other pages – especially the "Sessions" page.

Currently, the session selection component is still visible and interactive on non-chat pages, but its behavior isn't fully consistent with those pages. Some actions on the component affect the Sessions page, while others don't, leading to unexpected UI interactions.

In my test environment, there are two agents: "main" and "opencode". The component currently has the agent set to "main". When I navigate to the Sessions page and select "opencode" in the component, the sessions' store changes from ".*/main/.*" to "(multiple)" — not to ".*/opencode/.*" as I expected. Then, when I click the refresh button on the right side of the Sessions page, the store switches back to ".*/main/.*", but still not to ".*/opencode/.*". This is confusing.

There are two long-term directions we could take:

  1. Keep it local​ – Only show the session selection component on the chat page. On other pages, either hide it entirely or disable it.

  2. Make it truly global​ – Fully bind the component to every page so that changing the selection updates each page accordingly (e.g., the Sessions page). This would require two-way binding and careful synchronization.

For now, I propose we go with a quick fix that aligns with option 1: disable the session selection component on all non-chat pages. This prevents users from accidentally interacting with it in places where it doesn't behave correctly. It's a safe, low-risk change that eliminates the immediate confusion.

If the team later decides to adopt option 2 (global component with full two-way binding), I'll submit a follow-up PR to properly integrate it with the Sessions page and other relevant views.

This PR simply disables the component outside the chat context. No behavioral changes inside the chat page itself.

Real Behavior Proof

Before Fix

There are two agents: "main" and "opencode".

2026-06-27-21-02-28-image

Navigate to the Sessions page. The store is ".*/main/.*".

2026-06-27-21-04-59-image

Select "opencode" in the component, the sessions' store changes from ".*/main/.*" to "(multiple)"

2026-06-27-21-08-36-image

After refreshing, the store switches back to ".*/main/.*".

2026-06-27-21-13-52-image

After Fix

Selection is disabled on pages other than the chat page.

2026-06-27-21-31-36-image

Selection is available on the chat page.

2026-06-27-21-31-59-image

…sion

The component was made global but behaves inconsistently on other pages.
Temporarily disable it until proper two-way binding is implemented.
@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 27, 2026
@clawsweeper

clawsweeper Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed June 27, 2026, 10:07 AM ET / 14:07 UTC.

Summary
The PR disables the Control UI session picker and agent selector whenever state.tab is not "chat".

PR surface: Source 0. Total 0 across 1 file.

Reproducibility: yes. source-level reproduction is high-confidence: the PR's screenshots show the user-facing confusion, and CI confirms the patch currently disables controls in existing chat-control tests because the fixture lacks tab: "chat".

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: ui/src/ui/chat/session-controls.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #97257
Summary: This PR is the only open fix candidate found for the session selector outside chat; the related merged calm-composer PR is provenance, not a competing canonical fix.

Members:

  • adjacent_distinct: feat: calm composer controls #88772 - This merged PR moved session selection into the sidebar and made the current confusing non-chat behavior visible, but it is not an open duplicate or replacement fix.

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

  • [P2] Set the shared chat-control test fixture to tab: "chat" and add a non-chat disabled-state assertion.
  • Rerun node scripts/run-vitest.mjs ui/src/ui/views/chat.test.ts on the repaired head.

Risk before merge

  • [P1] The current PR head has a failing unit-ui shard because existing chat-control tests render the selector with an undefined tab, so merge should wait for fixture and focused coverage updates.

Maintainer options:

  1. Decide the mitigation before merge
    Keep the non-chat disablement mitigation, but merge it only after chat-control fixtures explicitly run in the chat tab and a focused non-chat disabled-state assertion is added.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] A narrow automated repair can update the chat-control fixture and add focused disabled-state coverage without changing the UX direction.

Security
Cleared: The diff only changes Lit disabled conditions in UI rendering and does not touch dependencies, workflows, credentials, package resolution, or code-execution surfaces.

Review findings

  • [P2] Set chat-control tests to the chat tab — ui/src/ui/chat/session-controls.ts:125
Review details

Best possible solution:

Keep the non-chat disablement mitigation, but merge it only after chat-control fixtures explicitly run in the chat tab and a focused non-chat disabled-state assertion is added.

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

Yes, source-level reproduction is high-confidence: the PR's screenshots show the user-facing confusion, and CI confirms the patch currently disables controls in existing chat-control tests because the fixture lacks tab: "chat".

Is this the best way to solve the issue?

No, not as-is. Disabling the non-chat controls is an acceptable mitigation, but the branch should make chat versus non-chat context explicit in tests before merge.

Full review comments:

  • [P2] Set chat-control tests to the chat tab — ui/src/ui/chat/session-controls.ts:125
    This new guard treats the shared createChatHeaderState() fixture's missing tab as non-chat, so the PR head disables controls that existing chat session-control tests expect to open. CI already fails this unit-ui shard with null picker inputs and disabled-trigger assertions; please set the fixture to tab: "chat" and add a focused non-chat disabled-state assertion.
    Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5ccfc97b319e.

Label changes

Label justifications:

  • P2: This is a bounded Control UI bug fix with visible UX impact and a narrow test blocker, not a release-blocking runtime, security, or data-loss issue.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (screenshot): The PR body includes before/after screenshots that directly show the non-chat controls disabled and the chat selector still usable after the fix.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes before/after screenshots that directly show the non-chat controls disabled and the chat selector still usable after the fix.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body includes before/after screenshots that directly show the non-chat controls disabled and the chat selector still usable after the fix.
Evidence reviewed

PR surface:

Source 0. Total 0 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 2 2 0
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 2 2 0

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs ui/src/ui/views/chat.test.ts.
  • [P1] git diff --check.

What I checked:

  • Repository policy read: Root and scoped UI review policy were read; the review applied the repo's read-beyond-diff, UI test, and PR proof requirements. (AGENTS.md:1, 5ccfc97b319e)
  • Scoped UI policy read: The scoped UI guide was read; no i18n/generated-output rule was implicated by this one-file TypeScript diff. (ui/AGENTS.md:1, 5ccfc97b319e)
  • Changed disabled gates: The PR head adds state.tab !== "chat" to the session picker disabled condition and the agent selector disabled condition. (ui/src/ui/chat/session-controls.ts:125, 81aadb0d868d)
  • Global sidebar caller: Current main renders renderChatSessionSelect in the sidebar with sessionSwitcherOnly: true, which explains why the control is visible outside the chat page today. (ui/src/ui/app-render.ts:587, 5ccfc97b319e)
  • Test fixture conflict: The shared createChatHeaderState() fixture constructs an AppViewState without setting tab, so the PR's new state.tab !== "chat" guard treats many existing chat-control tests as non-chat. (ui/src/ui/views/chat.test.ts:349, 5ccfc97b319e)
  • CI failure confirms the blocker: The PR CI unit-ui shard fails 11 chat session controls tests, including null picker inputs and a disabled session trigger assertion, matching the source-level fixture issue. (ui/src/ui/views/chat.test.ts:2941, 81aadb0d868d)

Likely related people:

  • steipete: GitHub PR metadata shows @steipete authored the merged calm-composer PR that moved session selection into the sidebar, and git history shows Peter split the chat session controls into the current module earlier. (role: introduced behavior and adjacent area contributor; confidence: high; commits: 2b30951b8090, 1fad8efa12cf; files: ui/src/ui/chat/session-controls.ts, ui/src/ui/app-render.ts, ui/src/ui/app-render.helpers.ts)
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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. labels Jun 27, 2026
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Thanks @forrest0x59. Closing because merged #101017 removed this obsolete global-selector path: sessions now live in the sidebar, navigate directly to Chat, and active-session state is scoped to Chat. This conflicting head also no longer passes the required UI proof against current main.

@steipete steipete closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui P2 Normal backlog priority with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: XS status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants