fix(webui-v2): redirect invalid chat thread routes

[flyagents]

Summary

• Redirect reserved or invalid /chat/:threadId routes back to /chat.
• Wait for the thread list to settle before deciding a deep-linked thread is missing.
• Keep locally created/selected threads active while the thread list refetch catches up.
• Add caller-level ChatPage regression coverage and regenerate the WebUI v2 bundle.

Change Type

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

Linked Issue

Fixes #5077

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/pages/chat/chat-page.test.mjs crates/ironclaw_webui_v2_static/static/js/pages/chat/lib/useHistory.test.mjs crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useThreads.test.mjs
  • node --check crates/ironclaw_webui_v2_static/static/js/pages/chat/chat-page.test.mjs
  • cargo test -p ironclaw_webui_v2_static
• cargo test --features integration if database-backed or integration behavior changed
• Manual testing
• If a coding agent was used and supports it, review-pr or pr-shepherd --fix was run before requesting review

Security Impact

None.

Reborn Trust-Boundary Checklist

N/A. This is WebUI route-state handling only; it does not change Reborn runtime, trust-bearing types, prompts, sandboxing, command execution, auth, or persistence.

Database Impact

None.

Blast Radius

WebUI v2 chat routing and thread selection state only. Possible impact is limited to /chat, /chat/:threadId, chat deep links, and active chat rendering while the thread list loads/refetches.

Rollback Plan

Revert this commit to restore the previous ChatPage behavior and regenerated bundle.

Review Follow-Through

No known follow-up. Reviewer attention requested on the loading/refetch edge case: a locally selected newly created thread should not be cleared before the thread list catches up.

---

Review track: B

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 510a68ee-cd10-4f8c-9298-c856b010bb15

📥 Commits

Reviewing files that changed from the base of the PR and between f127ebb and a3d4c17.

⛔ Files ignored due to path filters (1)

• crates/ironclaw_webui_v2_static/static/dist/app.js is excluded by !**/dist/**

📒 Files selected for processing (1)

• crates/ironclaw_webui_v2_static/static/js/pages/chat/chat-page.js

💤 Files with no reviewable changes (1)

• crates/ironclaw_webui_v2_static/static/js/pages/chat/chat-page.js

---

📝 Walkthrough

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Improved thread navigation behavior to properly handle reserved route names, malformed routes, and empty states. Thread state now syncs correctly with URL navigation.

• Tests

  • Added comprehensive test coverage for thread routing, state synchronization, and loading states.

Walkthrough

useThreads exposes React Query status fields. ChatPage replaces a trivial URL-to-state mirror with logic that detects reserved (newchat), malformed, missing, and non-openable thread IDs, clears state, and issues replace-mode redirects to /chat. Chat receives derived props. A Node.js vm-based test suite covers all routing cases.

Changes

Chat URL routing hardening

Layer / File(s)	Summary
useThreads status flags crates/ironclaw_webui_v2_static/static/js/pages/chat/hooks/useThreads.js	Exposes isLoaded (from query.isSuccess) and isFetching (from query.isFetching) in the hook return object.
ChatPage derived routing and redirect effect crates/ironclaw_webui_v2_static/static/js/pages/chat/chat-page.js	Derives threads, reserved/malformed route booleans, list settled/fetching status, and routeActiveThreadId; replaces simple useEffect with effect that clears threadsState.activeThreadId and navigates to /chat with { replace: true } for missing, reserved, malformed, or non-openable thread IDs; passes derived values to Chat.
vm-based ChatPage routing tests crates/ironclaw_webui_v2_static/static/js/pages/chat/chat-page.test.mjs	Loads and transforms chat-page.js source in vm context with stubbed React/router hooks; asserts redirect behavior for reserved route, missing threads, and stale active threads; asserts state preservation for valid loaded routes; asserts hold during fetch and before isLoaded; asserts bare /chat route clears state.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• nearai/ironclaw#5021: Adjusts Chat's activeThreadId-driven clearThreadState(...) reset timing to prevent flicker/bleed when routing changes the active thread, complementing this PR's shift of thread-ID derivation to the router layer.

Suggested reviewers

• think-in-universe

Poem

Route lands on newchat? Hard redirect fires. 🔀
Thread not in list? Clear and retire.
isLoaded false — hold still, don't stir.
Settled at last, set the ID just so.
No more red errors left to inquire. ✅

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title follows Conventional Commits style with scope and summary; clearly describes the main fix of redirecting invalid chat thread routes.
Description check	✅ Passed	Description includes all required sections: Summary, Change Type, Linked Issue, Validation results, Security Impact, Reborn checklist, Database Impact, Blast Radius, Rollback Plan, and Review track.
Linked Issues check	✅ Passed	PR fully addresses #5077 acceptance criteria: redirects reserved routes, waits for thread list settlement, handles edge cases, and adds regression coverage as required.
Out of Scope Changes check	✅ Passed	All changes are scoped to WebUI v2 chat routing and state management; no extraneous modifications detected outside linked issue objectives.

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

---

_{Comment @coderabbitai help to get the list of available commands.}

[gemini-code-assist]

Code Review

This pull request refactors the routing and thread selection logic in ChatPage to handle loading states, deep links, and malformed or reserved routes, supported by a new suite of unit tests and additional state flags in the useThreads hook. The review feedback highlights a potential UX issue where deep-linking to a valid thread briefly displays a blank composer while loading, and suggests preserving the thread ID during this state. Additionally, the feedback recommends removing a redundant dependency from the useEffect dependency array and updating the corresponding test assertion to align with the improved loading behavior.

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.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/ironclaw_webui_v2_static/static/js/pages/chat/chat-page.test.mjs`:
- Line 162: In the test case for the "wait for thread list" scenario in
chat-page.test.mjs, the assertion on line 162 for
result.chatProps.activeThreadId currently expects "thread-1", but this
contradicts the test scenario where isLoaded is false and no local selection
exists. Update the assertion to expect an unset value (such as undefined, null,
or empty string depending on the implementation) instead of "thread-1", as
activeThreadId should remain unset until the thread list finishes loading and
resolves the route-sync contract.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 621cde3a-9cb6-4e11-b7c0-84479a517265

📥 Commits

Reviewing files that changed from the base of the PR and between dc26dfb and f127ebb.

📒 Files selected for processing (1)

• crates/ironclaw_webui_v2_static/static/js/pages/chat/chat-page.test.mjs

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Expected value is inverted for the “wait for thread list” case.

Line 162 currently asserts "thread-1", but this scenario (isLoaded: false, no local selection) should keep activeThreadId unset until the list settles. This assertion contradicts the route-sync contract and the test intent.

Proposed fix

-  assert.equal(result.chatProps.activeThreadId, "thread-1");
+  assert.equal(result.chatProps.activeThreadId, null);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	assert.equal(result.chatProps.activeThreadId, "thread-1");
	assert.equal(result.chatProps.activeThreadId, null);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ironclaw_webui_v2_static/static/js/pages/chat/chat-page.test.mjs` at
line 162, In the test case for the "wait for thread list" scenario in
chat-page.test.mjs, the assertion on line 162 for
result.chatProps.activeThreadId currently expects "thread-1", but this
contradicts the test scenario where isLoaded is false and no local selection
exists. Update the assertion to expect an unset value (such as undefined, null,
or empty string depending on the implementation) instead of "thread-1", as
activeThreadId should remain unset until the thread list finishes loading and
resolves the route-sync contract.

[ilblackdragon]

Review — REQUEST CHANGES (needs a port onto current main)

The behavior is genuinely still missing on main (the chat page blindly does setActiveThreadId(urlThreadId) with no existence check or redirect), and the fix itself is good: the redirect target is the constant /chat (no open-redirect — the user-controlled urlThreadId is never reflected into a navigation target), and the load-race is handled correctly (it waits for the thread list to settle before treating a deep link as missing, and preserves a locally-created thread during refetch). Tests are non-vacuous and drive the real effect.

But it can't merge as-is — it's mechanically incompatible with current main:

1. Wrong/dead crate path (blocking). It targets crates/ironclaw_webui_v2_static/…, which refactor: fold storage/backend sub-crates into their parent crates #5540 folded into crates/ironclaw_webui_v2/. The source edits and the new chat-page.test.mjs land at a path that no longer exists. → Port both files to crates/ironclaw_webui_v2/static/js/pages/chat/.
2. Dropped prop (blocking). main's ChatPage gained globalAutoApproveEnabled (outlet-context destructure + Chat prop). This PR rewrites those exact lines from an older base, so a naive rebase silently drops it → auto-approve regression. → Preserve globalAutoApproveEnabled when porting.
3. Committed build artifact. The minified static/dist/app.js is hand-committed at the old (gitignored) path. → Drop it from the diff; regenerate via crates/ironclaw_webui_v2/frontend/build.mjs.

Nits after the port: the isReservedThreadRoute === "newchat" branch guards a URL nothing in the codebase produces (dead — drop it), and the urlThreadId && !isReserved && !isMalformed guard is recomputed three times (extract one isCandidateThreadRoute). One behavior note: on a query error the route stays in perpetual "pending validation" and never redirects — acceptable fail-open, worth a comment.

Once ported onto crates/ironclaw_webui_v2/ with globalAutoApproveEnabled preserved and the bundle dropped, this is APPROVE-quality. (Same crate-split issue hit #5130 — the ironclaw_webui_v2_static → ironclaw_webui_v2 fold from #5540 strands any webui PR opened before it.)

[ilblackdragon]

Review — VERDICT: REQUEST CHANGES (sound idea, must be recreated on the current tree) (batch review pass)

The redirect logic itself is sound — no redirect-loop risk (post-redirect urlThreadId is undefined → no-op branch), no open-redirect (every navigate() target is a constant or an internal id, never the raw route param), and unauthorized/nonexistent ids degrade safely to a redirect.

Relevance: still relevant — main's chat-page.js retains the naive effect with no thread-id validation, so the bug (deep link to a nonexistent/malformed id leaves an "active but empty" UI) is unfixed, and the chat-page.test.mjs coverage is net-new.

Conflict is structural, not a rebuildable bundle:

1. Crate renamed after this PR's merge-base: crates/ironclaw_webui_v2_static/ → crates/ironclaw_webui_v2/. Every path this PR edits is gone on main.
2. Committed bundle deleted; build system replaced. main has no committed static/dist/app.js — the frontend now bundles via frontend/build.mjs (esbuild) into OUT_DIR. The 5-line bundle edit is obsolete; drop it entirely.
3. Genuine source drift: main's chat-page.js added globalAutoApproveEnabled; main's useThreads.js swapped invalidateQueries for upsertThreadInCache.

Resolution (recreate on crates/ironclaw_webui_v2/): re-apply the redirect effect on main's current chat-page.js (keep globalAutoApproveEnabled), add isLoaded/isFetching to main's useThreads.js (and update the existing useThreads.test.mjs), place chat-page.test.mjs in the new path, and drop the bundle edit.

Findings:

1. Medium [chat-page.js:16,26-33] — post-settle race: on the PR's invalidateQueries-based create, if the backend list lags, a just-created thread can be ejected via navigate("/chat"). main's upsertThreadInCache already mitigates this — another reason to rebuild on main's useThreads.
2. Low [chat-page.js:18] — isMalformedThreadRoute only catches whitespace; other junk ids fall through to the safe "not in list → redirect" path (naming nit, not a hole).
3. Low — tests are non-vacuous (would fail pre-fix); only gap is no case asserting a valid thread stays open during a background refetch (isFetching:true, id present).

🤖 batch PR-review pass via parallel-pr-review