[codex] Fix web chat refresh active thread - #2330
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the frontend to automatically reopen the server's active thread upon initial load if no thread is specified in the URL hash. It also refines the logic for retrieving thread metadata to correctly handle the assistant thread's read-only status. Additionally, an end-to-end test has been added to verify that the active thread is preserved after a page refresh without a URL hash. I have no feedback to provide.
ilblackdragon
left a comment
There was a problem hiding this comment.
Code Review
Overview
Fixes #2285 where browser refresh always fell back to the assistant thread instead of restoring the server's active thread. loadThreads() now consults data.active_thread before falling back. Includes an E2E regression test.
Analysis
The fix is correct. Priority ordering: URL hash > server active_thread > assistant fallback. The _pendingThreadRestore path still runs first, so explicit URL hashes win.
Minor Issue
The three new return; statements skip the read-only channel detection block below. If the active thread is a read-only channel (e.g., Telegram), chat input won't be disabled on first load. It self-corrects after the next loadThreads() cycle, so it's a transient bug. Pre-existing (the _pendingThreadRestore path had the same issue), but this PR widens it to more code paths. Consider whether switchThread() / switchToAssistant() should internally handle the read-only state, or remove the early returns and fall through to the existing read-only check.
E2E Test
Well-structured: creates thread, sends message, strips URL hash, reloads, verifies active thread restoration and message history visibility.
Verdict: Approve — Clean, focused fix with good regression test. The read-only transient issue is pre-existing and minor.
There was a problem hiding this comment.
Pull request overview
Fixes the web gateway chat’s refresh behavior by restoring the server-reported active thread (when the URL lacks a thread hash) instead of always falling back to the pinned assistant thread, and adds an E2E regression test to prevent the issue from returning.
Changes:
- Update
loadThreads()to preferdata.active_threadwhencurrentThreadIdis unset, with assistant-thread fallback preserved. - Adjust channel/read-only detection to correctly resolve the assistant thread object when it’s the selected thread.
- Add an E2E test covering refresh-without-hash restoring a non-assistant active thread’s history.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/ironclaw_gateway/static/app.js |
Uses /api/chat/threads’ active_thread to restore the correct visible thread after refresh; keeps assistant fallback. |
tests/e2e/scenarios/test_sse_reconnect.py |
Adds a regression test ensuring refresh without a hash reopens the active non-assistant thread and preserves its messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Root Cause
On page refresh, the frontend loaded
/api/chat/threadsbut ignored theactive_threadfield whencurrentThreadIdwas empty. It selected the pinned assistant thread by default, so an in-flight non-assistant thread disappeared from the browser while the backend continued processing it.Fixes #2285.
Validation
node --check crates/ironclaw_gateway/static/app.jsCARGO_TARGET_DIR=/Volumes/NVME/ironclaw-upstream/target /Volumes/NVME/ironclaw-upstream/tests/e2e/.venv/bin/pytest scenarios/test_sse_reconnect.py::test_refresh_without_hash_reopens_active_thread_history -q --timeout=300