Skip to content

fix(webui-v2): clear sidebar thread highlight off chat routes - #5130

Closed
flyagents wants to merge 4 commits into
nearai:mainfrom
flyagents:fix/sidebar-active-thread-non-chat-pages
Closed

flyagents wants to merge 4 commits into
nearai:mainfrom
flyagents:fix/sidebar-active-thread-non-chat-pages

Conversation

@flyagents

@flyagents flyagents commented Jun 22, 2026 •

Copy link
Copy Markdown

Summary

  • Fixes the WebUI v2 sidebar so an old chat thread is not highlighted on non-chat pages.
  • Keeps the underlying active thread state unchanged; only the sidebar highlight is route-scoped.
  • Adds caller-level tests for GatewayLayout -> Sidebar and Sidebar -> SidebarThreads.
  • Regenerates the committed WebUI v2 bundle.

Change Type

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • CI/Infrastructure
  • Security
  • Dependencies

Linked Issue

Fixes #5076

Validation

  • cargo fmt --all -- --check
  • cargo clippy --all --benches --tests --examples --all-features -- -D warnings
  • cargo build
  • Relevant tests pass:
    • node --test crates/ironclaw_webui_v2_static/static/js/layout/gateway-layout.test.mjs crates/ironclaw_webui_v2_static/static/js/components/sidebar.test.mjs
    • node --check crates/ironclaw_webui_v2_static/static/js/layout/gateway-layout.test.mjs
    • node --check crates/ironclaw_webui_v2_static/static/js/components/sidebar.test.mjs
    • node build.mjs
    • git diff --check
  • cargo test --features integration if database-backed or integration behavior changed
  • Manual testing: not run locally
  • If a coding agent was used and supports it, review-pr or pr-shepherd --fix was run before requesting review

Rust checks were not run because cargo is not installed in this shell.

Security Impact

None.

Reborn Trust-Boundary Checklist

N/A. This is a WebUI-only presentational change that does not affect auth, secrets, prompts, runtime policy, sandboxing, DB, queues, or trust-bearing types.

Database Impact

None.

Blast Radius

WebUI v2 sidebar conversation highlighting only. The main risk is visual active-state regression in the sidebar on chat and non-chat routes.

Rollback Plan

Revert this PR/commit to restore the previous sidebar behavior.

Review Follow-Through

No known follow-up required. Local targeted tests passed; full Rust verification was unavailable because cargo is not installed in this shell.


Review track: B

Only show the active chat thread highlight when the current route is a chat route.

Changes:
- Compute a route-scoped sidebar active thread id in GatewayLayout.
- Pass null to Sidebar on non-chat routes so old conversations are not visually active.
- Let Sidebar accept an activeThreadId override while preserving the existing fallback.
- Add caller-level tests for GatewayLayout -> Sidebar and Sidebar -> SidebarThreads.
- Regenerate the committed WebUI v2 bundle.

Verification:
- node --test crates/ironclaw_webui_v2_static/static/js/layout/gateway-layout.test.mjs crates/ironclaw_webui_v2_static/static/js/components/sidebar.test.mjs
- node --check for both new test files
- node build.mjs
- git diff --check

Not run:
- cargo test -p ironclaw_webui_v2_static, because cargo is not installed in this shell.
@coderabbitai

coderabbitai Bot commented Jun 22, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Sidebar gains an activeThreadId prop defaulting to threadsState.activeThreadId. GatewayLayout computes sidebarActiveThreadId as null on non-chat routes and the real thread id on /chat routes, then passes it into Sidebar. Two new vm-sandbox test files cover both components.

Changes

Route-aware Sidebar active thread highlight

Layer / File(s) Summary
Sidebar activeThreadId prop and forwarding
crates/ironclaw_webui_v2_static/static/js/components/sidebar.js
Sidebar destructures activeThreadId with a default from threadsState.activeThreadId and forwards that resolved variable to SidebarThreads instead of reading state directly.
GatewayLayout route-gated activeThreadId
crates/ironclaw_webui_v2_static/static/js/layout/gateway-layout.js
Introduces isChatRoute check on pathname; computes sidebarActiveThreadId as threadsState.activeThreadId on chat routes or null elsewhere; wires it into Sidebar.
Sidebar vm-sandbox tests
crates/ironclaw_webui_v2_static/static/js/components/sidebar.test.mjs
Loads sidebar.js in a Node.js vm context with stripped imports and mocked globals; asserts SidebarThreads receives null on explicit override and falls back to state when no override is provided.
GatewayLayout vm-sandbox tests
crates/ironclaw_webui_v2_static/static/js/layout/gateway-layout.test.mjs
Loads gateway-layout.js in a vm context with fully mocked React/router/state; asserts Sidebar gets activeThreadId: null on /automations and "thread-1" on /chat/thread-1.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • think-in-universe

Poem

🗺️ Chat routes glow, all others go dark,
No ghost thread lingers past its park.
A prop defaults, a null is sent,
The sidebar rests — its highlight spent.
Route-aware at last, no false remark! ✨

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title follows Conventional Commits style (fix scope: summary) and accurately summarizes the main change: clearing sidebar thread highlight on non-chat routes.
Linked Issues check ✅ Passed Code changes directly address #5076 acceptance criteria: route-aware sidebar highlight cleared on non-chat routes [gateway-layout.js], activeThreadId prop added to Sidebar [sidebar.js], and comprehensive test coverage added for both GatewayLayout and Sidebar interactions.
Out of Scope Changes check ✅ Passed All changes are scoped to the stated objectives: route-aware activeThreadId propagation in gateway-layout.js, prop destructuring in sidebar.js, and new test files covering the interaction. No unrelated refactors or out-of-scope modifications detected.
Description check ✅ Passed PR description is complete and follows template structure with all required sections filled and validation tests documented.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added size: L 200-499 changed lines risk: low Changes to docs, tests, or low-risk modules contributor: new First-time contributor labels Jun 22, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the sidebar to only highlight the active thread when the user is on a chat-related route. It introduces an activeThreadId prop to the Sidebar component and conditionally passes it from GatewayLayout based on the current route. Additionally, comprehensive unit tests have been added for both components. The review feedback suggests using optional chaining (threadsState?.activeThreadId) in both files to defensively prevent potential runtime errors if threadsState is undefined.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread crates/ironclaw_webui_v2_static/static/js/components/sidebar.js Outdated
Comment thread crates/ironclaw_webui_v2_static/static/js/layout/gateway-layout.js Outdated
rafly-habibi and others added 3 commits June 22, 2026 15:40
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…t.js

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@ilblackdragon

Copy link
Copy Markdown
Member

Review — closing as superseded/duplicate of #5592

This fixes a real bug (the sidebar thread highlight persists on non-chat routes because it's driven by threadsState.activeThreadId, which isn't cleared on navigation). But it's superseded:

The bug is not yet fixed on main, so #5592 remains the one to land.

One thing worth carrying to #5592: this PR's tests are actually the better pattern — they render GatewayLayout/Sidebar and assert the wired activeThreadId prop (testing through the caller, per .claude/rules/testing.md), whereas #5592's tests only cover the pure helpers and never assert the wiring that regresses. Recommend #5592 adopt a GatewayLayout render assertion before merge.

Closing this in favor of #5592. Thanks for the fix — the diagnosis and the test approach were both good.

@ilblackdragon

Copy link
Copy Markdown
Member

Closing as duplicate of #5592 (same bug, fixed more coherently on the live crate path). This PR targets the deleted ironclaw_webui_v2_static crate (#5540) and hand-edits the minified bundle. #5592 is the one to land — recommend it adopt this PR's caller-level GatewayLayout render test. Thanks!

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

Labels

contributor: new First-time contributor risk: low Changes to docs, tests, or low-risk modules size: L 200-499 changed lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Reborn] Sidebar keeps chat thread highlighted on non-chat pages

3 participants