[#6042] fix(frontend): Stop losing a new session's first message on rapid create - #6048
[#6042] fix(frontend): Stop losing a new session's first message on rapid create#6048mmabrouk wants to merge 1 commit into
Conversation
…avigate Three holes in the first-message dispatch pipeline, all silent (#6042): the create/seed handoff slots were single globals a second create overwrote; the request build could fail instantly (invocation URL not loaded) or park forever (hung auth-header fetch) with no error; and a transcript stranded by any of these looked busy-forever on every reopen. - Park create/open handoffs and first-run seeds in per-session LISTS; a seed leaves the list only once its conversation carries a message. - Bound the run-request build: retry while the workflow is loading, reject on hang/deadline so the failure renders as an error bubble. - On reopen, stamp a transcript whose unanswered user tail has a confirmed-EMPTY durable record log with a visible 'never reached the agent' error instead of an eternal spinner.
dosubot
Bot
added
the
size:L
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAgent session handoffs now support concurrent pending opens and first-run seeds. Agent request preparation retries within a deadline. Session hydration marks stranded sends with a retryable error. ChangesAgent session reliability
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to The change improves rapid session creation and makes failed first sends visible, but a transient records lookup failure can still leave an unanswered message without the intended recovery notice until the session is revisited. Merge should wait for that bounded recovery issue to be fixed or explicitly accepted. 🚥 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 |
Railway Preview Environment
|
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: b9326cc2-c54b-4c76-ad89-eafb59764ccf
📒 Files selected for processing (14)
web/oss/src/components/AgentChatSlice/AgentChatPanel.tsxweb/oss/src/components/AgentChatSlice/assets/boundedRequest.test.tsweb/oss/src/components/AgentChatSlice/assets/boundedRequest.tsweb/oss/src/components/AgentChatSlice/hooks/useAgentChatSession.tsweb/oss/src/components/AgentChatSlice/hooks/useFirstRunSeed.tsweb/oss/src/components/AgentChatSlice/hooks/useOpenAgentSession.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.test.tsweb/oss/src/components/AgentChatSlice/hooks/useSessionHydration.tsweb/oss/src/components/AgentChatSlice/hooks/useStartAgentSession.tsweb/oss/src/components/AgentChatSlice/state/firstRunSeed.tsweb/oss/src/components/AgentChatSlice/state/pendingHandoffs.test.tsweb/oss/src/components/AgentChatSlice/state/pendingSessionOpen.tsweb/oss/src/components/Sidebar/dynamic/registry.tsweb/oss/src/components/pages/agent-home/hooks/useCreateAgent.ts
| timer = setTimeout(() => reject(new Error(PREPARE_HUNG_MESSAGE)), remaining) | ||
| }), | ||
| ]) | ||
| if (result) return result |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return every non-null build result.
null is the only unavailable-result sentinel in the function type. A valid falsy T, such as 0 or "", retries until it reports PREPARE_NOT_READY_MESSAGE.
Proposed fix
- if (result) return result
+ if (result !== null) return result📝 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 (result) return result | |
| if (result !== null) return result |
| } finally { | ||
| clearTimeout(timer) | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, retryMs)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Limit the retry sleep to the remaining deadline.
If a null build resolves shortly before the deadline, this line waits another full retryMs. The function can then fail up to 300 ms after its configured 15-second deadline.
Proposed fix
- await new Promise((resolve) => setTimeout(resolve, retryMs))
+ const delay = Math.min(retryMs, Math.max(0, deadline - Date.now()))
+ if (delay > 0) await new Promise((resolve) => setTimeout(resolve, delay))📝 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.
| await new Promise((resolve) => setTimeout(resolve, retryMs)) | |
| const delay = Math.min(retryMs, Math.max(0, deadline - Date.now())) | |
| if (delay > 0) await new Promise((resolve) => setTimeout(resolve, delay)) |
| useEffect(() => { | ||
| if (strandedCheckedRef.current || isHydrating || busy) return | ||
| if (liveness.isLoading || liveness.nest.isRunning) return | ||
| if (!hasStrandedTail(messagesRef.current)) return | ||
| strandedCheckedRef.current = true | ||
| let cancelled = false | ||
| void getDefaultStore() | ||
| .set(fetchSessionRecordsAtom, sessionId) | ||
| .then(({records}) => { | ||
| if (cancelled || busyRef.current) return | ||
| if (!records || records.length > 0) return | ||
| const current = messagesRef.current | ||
| if (!hasStrandedTail(current)) return | ||
| const stamped = [...current, strandedRunErrorCarrier()] | ||
| setMessages(stamped) | ||
| persistMessages({id: sessionId, messages: stamped}) | ||
| }) | ||
| return () => { | ||
| cancelled = true | ||
| } | ||
| }, [isHydrating, busy, liveness, sessionId, messagesRef, busyRef, setMessages, persistMessages]) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Retry an inconclusive record fetch.
Line 472 sets strandedCheckedRef.current before the fetch completes. If the fetch returns records: null, Line 478 exits without stamping an error, but every later effect run exits at Line 469. The stranded message then remains unresolved for the mounted session after a transient records failure.
Set the one-shot guard only after a conclusive non-empty or empty result. Schedule a bounded retry when records is unavailable.
1 participant
Context
Creating sessions quickly from the Home composer could silently lose the first message. The session either never came into existence (reproduced live on the 8180 dev stack: "Repro test 1" left no session row, no run, no error, while a second send worked), or it was created and named but its message never dispatched, so it read as busy forever on every reopen (Mahmoud's sessions
183cbbec…and37cda04e…on bighetzner: named stream rows, zero turns, zero records, no runner log lines).Three holes lined up, all silent:
pendingSessionOpenAtom,agentFirstRunSeedAtom). A second create before the first was consumed overwrote it, and a consumed seed died with the pane if you navigated away during the model/overlay wait.prepareSendMessagesRequesthad no bounds: a workflow whose invocation URL had not loaded yet failed instantly, and a hung await inside the build (the auth-header fetch) parked the send in "submitted" forever with no error and no network request.Changes
Both handoff carriers are now lists with add/remove writers.
AgentChatPanelconsumes every queued open for its scope (two rapid creates become two sessions), and a claimed seed leaves the list only once its conversation actually carries a message, so it survives an unmount mid-wait and retries on remount.The request build now runs through
buildRequestWithinDeadline(newassets/boundedRequest.ts): it retries whilebuildAgentRequestreturns null (workflow still loading) and rejects on hang or deadline (15s), so a failed send surfaces through the existing red error bubble instead of an eternal spinner.On reopen,
useSessionHydrationruns a one-shot stranded-send check: if the transcript tail is an unanswered user turn, nothing is running, and the durable record log is confirmed empty (records: []; a failed fetch returnsnulland never stamps), it appends the same run-error carrier the live error path uses: "This message never reached the agent — the run was not started. Send it again."Tests
pendingHandoffs.test.ts: list semantics for both carriers (no overwrite across sessions, replace within a session, identity-based removal).boundedRequest.test.ts: resolves immediately, retries null builds, not-ready error at deadline, timeout error on hang (fake timers).hasStrandedTailcases added touseSessionHydration.test.ts.tscclean.What to QA
Closes #6042