fix(frontend): resolve agent response buffering due to stale EventSou…

[psav]

…rce closure

Fixes intermittent message buffering where agent responses don't display until the user sends another message. The EventSource onmessage handler was capturing a stale processEvent closure, causing events to be processed with outdated setState callbacks when component callbacks changed (e.g., during agent team operations). Use a ref to ensure the handler always calls the latest processEvent, guaranteeing immediate UI updates.

[coderabbitai]

Walkthrough

Replaced direct processEvent usage in the SSE handler with a ref-backed callback in use-agui-stream to avoid stale-closure behavior; added processEventRef and a syncing useEffect, and removed processEvent from the connect hook dependency list.

Changes

Cohort / File(s)	Summary
EventSource stale-closure fix components/frontend/src/hooks/use-agui-stream.ts	Added processEventRef ref and a useEffect to keep it synchronized with processEvent. Updated eventSource.onmessage to call processEventRef.current?.(event) and removed processEvent from the connect hook dependency list.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main fix: resolving a stale EventSource closure causing message buffering in the frontend.
Description check	✅ Passed	The description is directly related to the changeset, explaining the stale closure bug and how the ref-based solution addresses it.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

components/frontend/src/hooks/use-agui-stream.ts (1)

133-141: 🛠️ Refactor suggestion | 🟠 Major

Remove processEvent from connect's dependency array.

Since the handler now accesses processEvent through the ref rather than capturing it directly, processEvent should be removed from the dependency array at line 188. Keeping it causes unnecessary recreations of connect whenever processEvent changes, which defeats part of the purpose of using the ref pattern.

Proposed fix

     },
-    [projectName, sessionName, processEvent, onConnected, onError, onDisconnected],
+    [projectName, sessionName, onConnected, onError, onDisconnected],
   )

Also applies to: 188-188

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/hooks/use-agui-stream.ts` around lines 133 - 141, The
connect function's dependency array still includes processEvent even though the
SSE handler uses processEventRef.current, causing unnecessary recreations;
remove processEvent from the dependency array of connect in use-agui-stream.ts
(leave other deps intact), and ensure processEventRef is kept in sync elsewhere
(the existing ref assignment to processEventRef.current should remain) so the
handler continues to call the latest processEvent via processEventRef.current.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@components/frontend/src/hooks/use-agui-stream.ts`:
- Around line 133-141: The connect function's dependency array still includes
processEvent even though the SSE handler uses processEventRef.current, causing
unnecessary recreations; remove processEvent from the dependency array of
connect in use-agui-stream.ts (leave other deps intact), and ensure
processEventRef is kept in sync elsewhere (the existing ref assignment to
processEventRef.current should remain) so the handler continues to call the
latest processEvent via processEventRef.current.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6d39a692-ce50-4045-8cc2-5d38f907590c

📥 Commits

Reviewing files that changed from the base of the PR and between 6b21778 and 60ca537.

📒 Files selected for processing (1)

• components/frontend/src/hooks/use-agui-stream.ts

[Gkrumbach07]

@psav can u look at the code rabbit comment. is it accurate

Remove processEvent from connect's dependency array.

Since the handler now accesses processEvent through the ref rather than capturing it directly, processEvent should be removed from the dependency array at line 188. Keeping it causes unnecessary recreations of connect whenever processEvent changes, which defeats part of the purpose of using the ref pattern.****

[jeremyeder]

I rechecked the current head, and these points are already addressed:

• processEventRef is synchronized with useLayoutEffect
• processEvent has been removed from connect’s dependency array
• there is a regression test covering rerender without reconnecting and asserting the latest callback is used

So my earlier concern was accurate for an earlier revision, but it’s stale against the current PR state.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@components/frontend/src/hooks/use-agui-stream.ts`:
- Around line 48-49: Replace the async ref update with a synchronous one:
initialize processEventRef as a non-null no-op (useRef<(event: PlatformEvent) =>
void>(() => {})) and change the effect that assigns processEventRef.current =
processEvent from useEffect to useLayoutEffect so the SSE handler
(processEventRef.current?.(event)) sees the updated callback immediately on
mount; keep the EventSource handler logic unchanged but ensure it calls
processEventRef.current(event).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8eb185d7-4603-4572-877e-5ca19e6f48de

📥 Commits

Reviewing files that changed from the base of the PR and between 27e55a7 and e1f5bbc.

📒 Files selected for processing (1)

• components/frontend/src/hooks/use-agui-stream.ts

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In React 19 client components, does useEffectrun after paint whileuseLayoutEffectruns synchronously after commit, and isuseLayoutEffect the recommended pattern for keeping callback refs fresh for external event listeners (like EventSource)?

💡 Result:

Yes, in React 19 client components, useEffect runs asynchronously after paint (or before paint if triggered by user interaction), while useLayoutEffect runs synchronously after the commit phase but before the browser paints. useLayoutEffect is a recommended pattern for scenarios requiring the latest ref values before paint, such as measurements to avoid flicker. For keeping callback refs fresh specifically for external event listeners like EventSource (to avoid stale closures), the common pattern uses useRef updated in useLayoutEffect (the "latest ref pattern"). While not exclusively for EventSource, this ensures the ref holds the most current callback when setting up listeners in useEffect, preventing bugs from closures capturing outdated values. React 19 introduces useEffectEvent as a cleaner alternative for event callbacks inside effects that need latest values without deps.

Citations:

• 1: https://beta.reactjs.org/reference/react/useLayoutEffect
• 2: https://lqtiendev.com/difference-between-useeffect-and-uselayouteffect-updates-for-react-19-e8cc9c93e439
• 3: https://react.dev/reference/react/useLayoutEffect
• 4: https://kentcdodds.com/blog/useeffect-vs-uselayouteffect
• 5: https://react.dev/reference/react/useEffectEvent
• 6: https://www.epicreact.dev/the-latest-ref-pattern-in-react

---

🏁 Script executed:

head -200 components/frontend/src/hooks/use-agui-stream.ts | tail -160

Repository: ambient-code/platform

Length of output: 5027

---

Use useLayoutEffect with a non-null ref to synchronously update the SSE callback, closing the race condition window.

The current useEffect at lines 94–97 runs asynchronously after paint. If an EventSource message arrives before that update completes, processEventRef.current?.(event) at line 137 will hit a stale or null handler, potentially causing buffering or lost events on initial mount.

Proposed fix

-import { useCallback, useEffect, useRef, useState } from 'react'
+import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react'

-  const processEventRef = useRef<((event: PlatformEvent) => void) | null>(null)
+  const processEventRef = useRef<(event: PlatformEvent) => void>(() => {})

-  useEffect(() => {
+  useLayoutEffect(() => {
     processEventRef.current = processEvent
   }, [processEvent])

-          processEventRef.current?.(event)
+          processEventRef.current(event)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/frontend/src/hooks/use-agui-stream.ts` around lines 48 - 49,
Replace the async ref update with a synchronous one: initialize processEventRef
as a non-null no-op (useRef<(event: PlatformEvent) => void>(() => {})) and
change the effect that assigns processEventRef.current = processEvent from
useEffect to useLayoutEffect so the SSE handler
(processEventRef.current?.(event)) sees the updated callback immediately on
mount; keep the EventSource handler logic unchanged but ensure it calls
processEventRef.current(event).