fix(webui): recover from stale thread 404s - #5928
serrrfirat wants to merge 2 commits into
Conversation
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
github-actions
Bot
added
size: M
📝 WalkthroughWalkthroughChangesThread Not-Found Recovery
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements robust handling for missing chat threads (404 errors) in the frontend, ensuring that stale threads are evicted from the cache and history, a toast notification is shown, and the user is redirected. The review feedback suggests two improvements: first, ensuring that removeThreadFromCache is imported and called in useHistory.ts to evict stale threads from the sidebar cache even if the user has navigated away before the 404 resolves; second, removing a redundant submitBusyRef.current = false assignment in useChat.ts as it is already handled in the finally block.
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.
| import React from "react"; | ||
| import { fetchTimeline } from "../../../lib/api"; | ||
| import { authScope } from "../../../lib/auth-scope"; | ||
| import { isThreadNotFoundError } from "../../../lib/thread-errors"; |
There was a problem hiding this comment.
Import removeThreadFromCache to allow evicting stale threads from the sidebar cache when a background thread load returns a 404.
| import { isThreadNotFoundError } from "../../../lib/thread-errors"; | |
| import { isThreadNotFoundError } from "../../../lib/thread-errors"; | |
| import { removeThreadFromCache } from "../lib/thread-cache"; |
| if (isThreadNotFoundError(err)) { | ||
| evictThreadHistory(threadId); |
There was a problem hiding this comment.
When a background thread load returns a 404, the thread should be evicted from the sidebar cache (removeThreadFromCache) even if it is not the currently active thread. Currently, removeThreadFromCache is only called inside handleThreadNotFound when threadIdRef.current === threadId, leaving stale threads in the sidebar if the user navigated away before the 404 resolved.
if (isThreadNotFoundError(err)) {
removeThreadFromCache(threadId);
evictThreadHistory(threadId);| updateCurrentRunState(() => setIsProcessing(false)); | ||
| submitBusyRef.current = false; | ||
| throw err; |
There was a problem hiding this comment.
The assignment submitBusyRef.current = false is redundant here because the finally block of the send function (on line 844) unconditionally resets submitBusyRef.current = false on any exit path.
| updateCurrentRunState(() => setIsProcessing(false)); | |
| submitBusyRef.current = false; | |
| throw err; | |
| updateCurrentRunState(() => setIsProcessing(false)); | |
| throw err; |
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | 983bffa7040a |
Head: 983bffa7040a3f247a19224c6223d13bc22d550e
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Found one blocking WebUI regression in the stale-thread 404 recovery path: sending a message to a deleted thread can discard the user's submitted draft.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Preserve submitted draft when redirecting after send 404
Location: crates/ironclaw_webui_v2/frontend/src/pages/chat/chat.tsx:75
The new missing-thread handler is also invoked from useChat.send after the composer has already cleared the submitted text/attachments. ChatInput restores failed sends to the old/current draftKey, but this redirect switches to /chat with the new-chat draft key and the 404 path skips the generic retryable error bubble, so the user's just-submitted message can disappear. Preserve the submitted payload into the new-chat draft/location state before navigating, or keep a retryable failure visible until the user can recover it.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/failed/superseded state while reviewers run.
| (missingThreadId) => { | ||
| clearThreadState(missingThreadId); | ||
| toast(t("chat.threadNoLongerExists"), { tone: "error", duration: 5000 }); | ||
| onSelectThread?.(null, { replace: true }); |
There was a problem hiding this comment.
This redirect is also reached from useChat.send after ChatInput has cleared the submitted draft. The failure restore writes back under the old thread draft key, but navigating to /chat switches to the new-chat key and this 404 path skips the retryable error bubble, so the user's just-submitted text/attachments can disappear. Please preserve the payload into the new-chat draft/location state before navigating, or keep a retryable failure visible.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/frontend/src/pages/chat/chat.tsx`:
- Around line 71-78: Preserve the composer draft in handleThreadNotFound when
redirecting from a missing thread: capture the active draft before
clearThreadState and pass it through the replace navigation to the new chat
route, or retain the prior composerDraftKey during the handoff so unsent text is
not stranded under the missing thread ID.
In `@crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts`:
- Around line 122-125: Reset missingThreadIdsRef whenever threadId changes so
stale 404 deduplication does not survive leaving and reopening a thread; update
the relevant useChat effect/logic near handleThreadNotFound and add a regression
test covering a thread becoming not found, switching away, then reopening it and
verifying recovery runs again.
- Around line 797-807: Only reset processing state and clear submitBusyRef
inside the active-target branch where threadIdRef.current is absent or matches
sendThreadId; leave the stale-send branch limited to removing the cached thread
and evicting its history. Update the thread-not-found handling around
isThreadNotFoundError and handleThreadNotFound accordingly.
🪄 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: 0ff46036-a436-4c23-b317-c890629bb093
📒 Files selected for processing (12)
crates/ironclaw_webui_v2/frontend/src/i18n/en.tscrates/ironclaw_webui_v2/frontend/src/lib/thread-errors.test.tscrates/ironclaw_webui_v2/frontend/src/lib/thread-errors.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/chat.tsxcrates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useHistory.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/chat.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/thread-cache.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/thread-cache.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChat-send.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useHistory.test.tscrates/ironclaw_webui_v2/src/static_assets/assets.rs
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.07% — 285397 / 335501 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
|
🚅 Deployed to the ironclaw-pr-5928 environment in ironclaw-ci-preview
|
|
Addressed review feedback in 65a424d:
Local validation after the review fixes:
Post-review @ironloopai review --agent reviewer |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/frontend/src/pages/chat/chat.tsx`:
- Around line 76-82: In the new-chat recovery callback, clear stale recovery
state by detecting when a recovery payload is supplied and unconditionally
writing both the draft and staged attachments stores for NEW_DRAFT_KEY,
including empty values. Update the logic around recovery.composerDraft and
recovery.stagedAttachments so a text-only retry removes prior attachments and an
attachment-only or empty recovery removes any prior draft.
In `@crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts`:
- Around line 798-804: Prevent stale thread recovery in the thread-not-found
handling around handleThreadNotFound: only treat a null threadIdRef.current as
valid when the send originated from the new-chat flow, otherwise require it to
match sendThreadId before restoring payload or clearing processing. Add a
regression test covering a send from thread A that returns 404 after navigation
to /chat.
- Line 279: Move the missingThreadIdsRef reset out of the render path in useChat
and into the committed effect that responds to threadId changes. Remove the
render-time clear near the thread-switch logic, and call
missingThreadIdsRef.current.clear() inside the existing threadId effect after
the change commits, preserving deduplication when renders are discarded.
🪄 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: 5d167265-64f4-4cd4-85d2-e6a7a7638c05
📒 Files selected for processing (6)
crates/ironclaw_webui_v2/frontend/src/pages/chat/chat.tsxcrates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useHistory.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/chat.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useChat-send.test.tscrates/ironclaw_webui_v2/frontend/src/pages/chat/lib/useHistory.test.ts
| (missingThreadId, recovery = {}) => { | ||
| const draft = recovery.composerDraft || ""; | ||
| const stagedAttachments = recovery.stagedAttachments || []; | ||
| if (draft) setDraft(NEW_DRAFT_KEY, draft); | ||
| if (stagedAttachments.length > 0) { | ||
| setStagedAttachments(NEW_DRAFT_KEY, stagedAttachments); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear prior new-chat recovery state when the recovered value is empty.
The conditional writes retain a previous NEW_DRAFT_KEY draft or attachments. Recovering a text-only send can therefore retain unrelated staged attachments and submit them on the retry. Detect a supplied recovery payload, then overwrite both stores unconditionally.
Proposed fix
const handleThreadNotFound = React.useCallback(
(missingThreadId, recovery = {}) => {
const draft = recovery.composerDraft || "";
const stagedAttachments = recovery.stagedAttachments || [];
- if (draft) setDraft(NEW_DRAFT_KEY, draft);
- if (stagedAttachments.length > 0) {
+ const hasRecovery =
+ Object.prototype.hasOwnProperty.call(recovery, "composerDraft") ||
+ Object.prototype.hasOwnProperty.call(recovery, "stagedAttachments");
+ if (hasRecovery) {
+ setDraft(NEW_DRAFT_KEY, draft);
setStagedAttachments(NEW_DRAFT_KEY, stagedAttachments);
}📝 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.
| (missingThreadId, recovery = {}) => { | |
| const draft = recovery.composerDraft || ""; | |
| const stagedAttachments = recovery.stagedAttachments || []; | |
| if (draft) setDraft(NEW_DRAFT_KEY, draft); | |
| if (stagedAttachments.length > 0) { | |
| setStagedAttachments(NEW_DRAFT_KEY, stagedAttachments); | |
| } | |
| (missingThreadId, recovery = {}) => { | |
| const draft = recovery.composerDraft || ""; | |
| const stagedAttachments = recovery.stagedAttachments || []; | |
| const hasRecovery = | |
| Object.prototype.hasOwnProperty.call(recovery, "composerDraft") || | |
| Object.prototype.hasOwnProperty.call(recovery, "stagedAttachments"); | |
| if (hasRecovery) { | |
| setDraft(NEW_DRAFT_KEY, draft); | |
| setStagedAttachments(NEW_DRAFT_KEY, stagedAttachments); | |
| } |
🤖 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/frontend/src/pages/chat/chat.tsx` around lines 76 -
82, In the new-chat recovery callback, clear stale recovery state by detecting
when a recovery payload is supplied and unconditionally writing both the draft
and staged attachments stores for NEW_DRAFT_KEY, including empty values. Update
the logic around recovery.composerDraft and recovery.stagedAttachments so a
text-only retry removes prior attachments and an attachment-only or empty
recovery removes any prior draft.
| // raw setActiveRunState rather than the activeRunRef-mutating wrapper. | ||
| if (stateThreadId !== threadId) { | ||
| setStateThreadId(threadId); | ||
| missingThreadIdsRef.current.clear(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts'
wc -l "$file"
printf '\n--- lines 240-320 ---\n'
sed -n '240,320p' "$file"
printf '\n--- grep missingThreadIdsRef ---\n'
rg -n "missingThreadIdsRef|threadIdRef|stateThreadId" "$file"Repository: nearai/ironclaw
Length of output: 4334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts'
sed -n '250,310p' "$file"
printf '\n---\n'
rg -n "missingThreadIdsRef|threadIdRef|stateThreadId" "$file"Repository: nearai/ironclaw
Length of output: 3500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='crates/ironclaw_webui_v2/frontend/src/pages/chat/hooks/useChat.ts'
printf '\n--- lines 150-175 ---\n'
sed -n '150,175p' "$file"
printf '\n--- lines 748-810 ---\n'
sed -n '748,810p' "$file"Repository: nearai/ironclaw
Length of output: 3464
Move missingThreadIdsRef reset out of render. useChat.ts:279 still writes a ref in the render branch, which this hook’s own invariant forbids. A discarded thread-switch render can clear the dedup set without committing the matching stateThreadId change, so stale 404 recovery for the old thread can fire again. Move the reset into the committed threadId effect.
🤖 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/frontend/src/pages/chat/hooks/useChat.ts` at line
279, Move the missingThreadIdsRef reset out of the render path in useChat and
into the committed effect that responds to threadId changes. Remove the
render-time clear near the thread-switch logic, and call
missingThreadIdsRef.current.clear() inside the existing threadId effect after
the change commits, preserving deduplication when renders are discarded.
| if (isThreadNotFoundError(err) && sendThreadId) { | ||
| if (!threadIdRef.current || threadIdRef.current === sendThreadId) { | ||
| handleThreadNotFound(sendThreadId, { | ||
| composerDraft: renderContent, | ||
| stagedAttachments, | ||
| }); | ||
| updateCurrentRunState(() => setIsProcessing(false)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not recover a stale send after navigation to new chat.
When a send from thread A fails after navigation to /chat, threadIdRef.current is null, so this branch restores A’s payload and clears processing in the new chat. Only treat null as active when this send originated from the new-chat flow.
Proposed fix
if (isThreadNotFoundError(err) && sendThreadId) {
- if (!threadIdRef.current || threadIdRef.current === sendThreadId) {
+ const shouldRecoverVisibleThread =
+ threadIdRef.current === sendThreadId ||
+ (!threadId && !targetThreadId);
+ if (shouldRecoverVisibleThread) {
handleThreadNotFound(sendThreadId, {
composerDraft: renderContent,
stagedAttachments,
});Add a regression test for a send from thread A that returns 404 after navigation to /chat.
📝 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.
| if (isThreadNotFoundError(err) && sendThreadId) { | |
| if (!threadIdRef.current || threadIdRef.current === sendThreadId) { | |
| handleThreadNotFound(sendThreadId, { | |
| composerDraft: renderContent, | |
| stagedAttachments, | |
| }); | |
| updateCurrentRunState(() => setIsProcessing(false)); | |
| if (isThreadNotFoundError(err) && sendThreadId) { | |
| const shouldRecoverVisibleThread = | |
| threadIdRef.current === sendThreadId || | |
| (!threadId && !targetThreadId); | |
| if (shouldRecoverVisibleThread) { | |
| handleThreadNotFound(sendThreadId, { | |
| composerDraft: renderContent, | |
| stagedAttachments, | |
| }); | |
| updateCurrentRunState(() => setIsProcessing(false)); |
🤖 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/frontend/src/pages/chat/hooks/useChat.ts` around
lines 798 - 804, Prevent stale thread recovery in the thread-not-found handling
around handleThreadNotFound: only treat a null threadIdRef.current as valid when
the send originated from the new-chat flow, otherwise require it to match
sendThreadId before restoring payload or clearing processing. Add a regression
test covering a send from thread A that returns 404 after navigation to /chat.
There was a problem hiding this comment.
✅ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ✅ Approved | 0 | 0 | 0 | 65a424da0ba8 |
Head: 65a424da0ba86d8092b57eaf1e7793e44d3bd47a
Next: No reviewer action needed.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
No concrete blocking issues found in the stale-thread 404 recovery changes. The PR adds focused handling and tests for active/background missing-thread cases and keeps the cache/history cleanup scoped.
Findings
None.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/failed/superseded state while reviewers run.
This branch was successfully deployed
1 participant
Summary
/chatand showThis thread no longer existsinstead of an inline genericNot founderrorChange Type
Linked Issue
None.
Validation
pnpm testpnpm typecheckpnpm lintpnpm buildcargo fmt --all -- --checkcargo test -p ironclaw_webui_v2 --features webui-v2-betapost-review rerun deferred to remote CI because local Rust compile was blocked under toolchain load after pushSecurity Impact
No auth, permission, token, secret, CORS, listener, or trust-boundary behavior changes. The change only handles existing 404 responses in the WebUI v2 client.
Trust-Boundary Impact
No new trust boundary. The browser still consumes same-origin WebUI v2 APIs and treats backend 404s as missing-thread state.
DB Impact
None. No schema, migration, persistence, or backend write behavior changes.
Blast Radius
Scoped to WebUI v2 chat thread history/send error handling and client-side caches/drafts.
Rollback
Revert this PR. The fallback behavior returns to the prior generic Not found/chat error path for stale thread 404s.
Review Follow-Through
Addressed Gemini and CodeRabbit feedback by evicting sidebar cache from
useHistory, preserving send drafts across missing-thread redirects, resetting missing-thread dedupe on thread changes, and keeping stale background send cleanup isolated from the active thread.