fix: prevent infinite re-render loop in reattach effect

[AlexPlum405]

Summary

• Fixed "Maximum update depth exceeded" error caused by the attachRecoverableRuns useEffect in ProjectView.tsx
• The effect had messages in its dependency array while also calling updateMessageById inside the effect body, creating a state update → re-render → effect re-fire loop
• Replaced direct messages dependency with a messagesRef pattern so the effect reads current messages without re-triggering on every state change

Root Cause

The useEffect (line ~563) that reattaches recoverable daemon runs depended on messages. Inside the effect, updateMessageById is called to patch message run status (e.g., setting runId, marking as failed). Each call updates messages state → triggers re-render → effect re-fires → calls updateMessageById again → infinite loop.

While ref-based guards (reattachControllersRef, completedReattachRunsRef) prevent duplicate reattach operations, they cannot prevent the effect function itself from being re-invoked and re-executing updateMessageById for messages that match the initial conditions before the async guards take effect.

Fix

1. Added messagesRef (useRef) that always holds the latest messages value
2. The effect reads messagesRef.current instead of closing over messages
3. Removed messages from the dependency array

The effect now only re-runs when daemonLive, activeConversationId, or streaming state changes — which are the actual conditions that should trigger reattachment logic.

Test plan

• Verified no TypeScript errors
• Open a project with an active/recoverable agent run → no console error, no infinite loop
• Send a new message → streaming works normally, no "Maximum update depth exceeded"

[lefarcen]

Hi @AlexPlum405! 🎉
Thanks for the PR — this infinite reattach loop fix looks like exactly the right pattern.
I will run a deep review and get back to you within 24h.

Thanks for making open-design better!
— open-design team

[lefarcen]

Hey @AlexPlum405 — thanks for tackling this reattach loop issue! The ref pattern approach is sound, but I found a critical timing issue that needs fixing before merge.

Core problem

Removing messages from deps creates a hydration race:

1. On project/conversation load, activeConversationId changes → effect runs
2. Effect reads messagesRef.current (still empty or from prev conversation)
3. Later setMessages(list) doesn't retrigger effect → active messages never reattach

P1 — Deps array (line ~754)

Issue: Removing messages from deps breaks initial reattachment. On load, the effect fires before messages hydrate, reads stale/empty ref, then never runs again when real messages arrive.

Fix: Don't rely solely on removing messages from deps. Instead:

• Add a "hydration complete" flag/effect that calls reattach after listMessages() completes
• Keep duplicate-run guards separate from the hydration trigger
• Or: trigger reattach directly with the freshly loaded list instead of reading stale ref

P2 — Snapshot staleness (line ~570)

Issue: currentMessages is a snapshot at effect start. If messages change during listActiveChatRuns() / fetchChatRunStatus() (e.g., user presses Stop), later updates can revive/clobber a message that's no longer active.

Fix: Re-read messagesRef.current after each await and before starting reattach. Make updaters no-op if latest message state is no longer active for that run.

P2 — Fallback update (line ~596)

Issue: Fallback update blindly writes runId/runStatus from stale captured message. If latest state already has different runId or is terminal, this clobbers newer state.

Fix: Updater should validate prev.runStatus and prev.runId before writing. Skip launching reattach if validation fails.

P2 — Test coverage

This critical reattach path appears untested at component/state level. Existing provider SSE tests don't cover the timing regression introduced here.

Fix: Add automated test that:

1. Loads conversation with active assistant message
2. Verifies reattach starts after messages hydrate
3. Verifies status/message updates don't cause repeated loops

---

Once the P1 hydration race is fixed, the other P2s become much easier to validate. Let me know if you'd like to discuss approach!

[mrcfps]

@AlexPlum405 Thanks for tackling the render loop here. I found one lifecycle issue that can keep the recoverable-run reattach path from running after messages finish loading; the suggested fix is to trigger the effect from message-load completion without depending on every message update. 🙂

_{Generated by Looper 0.5.4 · runner=reviewer · agent=opencode}

[mrcfps]

This now snapshots messagesRef.current only when the effect runs, but the effect is still triggered by activeConversationId before the [project.id, activeConversationId] loader finishes its async listMessages(...)/setMessages(list) work. At that point the ref can still be [] (or the previous conversation's messages), and because messages was removed from the dependency list below, the reattach pass won't run again when the persisted active assistant message actually arrives. That means opening a project with a recoverable run can silently skip reattachDaemonRun, which breaks the bugfix's main recovery path.

Could you keep the loop prevention but add a dependency that represents message-load completion—for example a messagesLoadedConversationId/revision set after listMessages resolves, or move the reattach trigger into the load completion path—and add a regression test that loads an active assistant message asynchronously and verifies reattach starts? 🙂