fix(frontend): Keep the agent session streaming when you navigate away - #5862
fix(frontend): Keep the agent session streaming when you navigate away#5862ashrafchowdury wants to merge 5 commits into
Conversation
…c to maintain chat instances across navigation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds a session-scoped chat registry. ChangesSession chat persistence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AgentChatSession
participant SessionChatRegistry
participant SessionState
participant Chat
AgentChatSession->>SessionChatRegistry: acquireSessionChat(sessionId, hooks)
SessionChatRegistry->>Chat: create or reuse session chat
Chat-->>AgentChatSession: provide shared chat instance
AgentChatSession->>SessionChatRegistry: releaseSessionChat(sessionId, stillOpen)
SessionState->>SessionChatRegistry: dropSessionChat(sessionId) when session closes
SessionChatRegistry->>Chat: stop and remove closed chat
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dda9c5a8-2ea1-43be-ba15-ba583abd5e42
📒 Files selected for processing (3)
web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/state/chatRegistry.test.tsweb/oss/src/components/AgentChatSlice/state/chatRegistry.ts
Railway Preview Environment
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/oss/src/components/AgentChatSlice/state/chatRegistry.ts (1)
47-65: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftDo not publish a new registry entry during render.
useAgentChatSessioncallsacquireSessionChatduring render (the suppliedweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tssnippet, Lines 65-224). Lines 49-65 create and register aChatbefore any commit. If React abandons that first render, no effect cleanup runs. A later committed mount reuses the entry at Line 47, ignores its owninitialMessages, and can retain the abandoned chat indefinitely.Make registry ownership commit-aware. Keep a new entry provisional until a committed mount claims it, or discard uncommitted entries with a tokenized protocol. Add a regression test for an abandoned first acquisition with different
initialMessages.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 23cb2404-9619-4daa-89e6-4d3d055919e5
📒 Files selected for processing (4)
web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/state/chatRegistry.test.tsweb/oss/src/components/AgentChatSlice/state/chatRegistry.tsweb/oss/src/components/AgentChatSlice/state/sessions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.ts
…sposal logic and auto-resume prevention
Context
Start an agent run in the playground, switch to another page while it is still producing, and come back: the answer had stopped streaming. The turn was still alive on the runner, but the browser was no longer following it. The tab fell back to the 15s durable-log catch-up, so the rest of the answer arrived in jumps instead of live.
The cause was ownership.
useChatcreated itsChatinside the conversation component, so the SSE read lived and died with the mount, and the D9 teardown effect calledstop()on every unmount. A route change is an unmount, so it looked exactly like closing the tab. Fixes #5724.Changes
The
Chatinstance now lives in a small module-scoped registry keyed by session id (state/chatRegistry.ts), and the component borrows it instead of owning it.Before: unmount always aborted the stream.
After: unmount asks whether the session itself is gone. A route change leaves the tab open, so the chat stays and the stream keeps running; re-entering the route re-binds to the same instance mid-turn. Closing, deleting, or archiving the session removes it from the open-tab set first, so that path still stops the stream and drops the instance.
Because the chat now outlives the mount, its callbacks (
prepareRequest,sendAutomaticallyWhen,onFinish) are rebound on every acquire. That is what keeps a long-lived chat from running stale closures, and it is why a run still follows a revision switch or a self-commit rather than sticking to the revision the session first mounted on.One subtlety is load-bearing and worth knowing while reading the diff: the registry must never hand
useChata fresh instance under a session id it already rendered.useChatswaps its internal ref on identity change but keys its message subscription on the chat id, which does not change, so it would keep listening to the dropped instance and the transcript would freeze. Keeping the entry alive for as long as the tab is open is what guarantees that. The trade-off is one idleChatper open tab until that tab is closed or the page reloads.Tests
chatRegistry.test.tscovers the acquire/release policy in 6 cases: re-bind on remount, preserve astreamingand asubmittedchat across a navigation, keep an idle chat while its tab is open, tear down when the session is no longer open, and forward a settled turn to the current mount'sonFinish.vitest run src/components/AgentChatSlice, 18 files, 132 tests).tsc --noEmitand eslint clean on@agenta/oss.state/sessions.tsremoves the id fromopenIdsByAppAtombefore React runs the cleanup. If a new teardown path is ever added, it has to follow the same order.What to QA