fix: gate ?q= auto-send on the provisioned sessionId (chat#1847)#1848
Conversation
The deep-link initial message (AGENTS cards route to /chat?q=...) fired as soon as the user was authenticated and useChat was idle - before the new-chat bootstrap minted a sessionId - so the transport POSTed /api/chat without a sessionId, the api validator 400'd (missing_fields: [sessionId]), and the user saw the generic error toast. The manual Send button was already gated on provisioning; the auto-send path skipped that gate and lost the race every time for logged-in users. Extract the gate into lib/chat/shouldAutoSendInitialMessage (SRP, unit-tested) mirroring the manual-send conditions plus sessionId. The effect re-runs when the bootstrap lands, so the ?q= message queues and sends instead of failing. TDD: 6-case suite RED->GREEN (incl. the provisioning race and the busy-transport double-send guard). Full suite 80 passed; tsc no new errors vs main (22 pre-existing); prettier clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds ChangesAuto-send gating fix
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/chat/shouldAutoSendInitialMessage.ts (2)
1-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten
statusto the actual union type instead ofstring.
status: stringaccepts any value, so a typo like"redy"would silently fail the=== "ready"check with no compile-time warning. TheuseChathook exposesstatusas a literal union ("submitted" | "streaming" | "ready" | "error"); importing/reusing that type here would give compile-time safety for this critical gate.As per path instructions,
lib/**/*.tsshould "Use TypeScript for type safety."♻️ Suggested fix
+import type { ChatStatus } from "ai"; // verify exact export name/version + interface ShouldAutoSendInitialMessageParams { hasInitialMessages: boolean; /** useChat transport status — only "ready" may send. */ - status: string; + status: ChatStatus;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/chat/shouldAutoSendInitialMessage.ts` around lines 1 - 16, The ShouldAutoSendInitialMessageParams status field is too broad and should match the actual useChat status union instead of string. Update the type used by shouldAutoSendInitialMessage to reuse the useChat status literal union ("submitted" | "streaming" | "ready" | "error"), so the ready check is compile-time safe and typos are rejected. Keep the change focused on the ShouldAutoSendInitialMessageParams interface and any related type import or alias needed in shouldAutoSendInitialMessage.Source: Path instructions
32-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGate has no memory of "already attempted" — possible re-fire after a retry truncates
messages.The gate only inspects instantaneous
messagesLength/status; it doesn't know whether the initial message was already sent once. InuseVercelChat.ts, a failed auto-sent message can be truncated back out ofmessagesviadeleteTrailingMessagesduring a manual retry (seeearliestFailedUserMessageIdflow,hooks/useVercelChat.tslines 232-250, 353-358). IfmessagesLengthdrops back to 0 whilestatustransiently reads"ready", this gate would returntrueagain and the effect would re-fireinitialMessages[0], duplicating the send alongside the user's manual retry.Official
useChatdocs state status only becomes"ready"once "the full response has been received and processed," which limits — but doesn't fully rule out — this race depending on exact retry timing. Worth confirming with a regression test around the retry-after-auto-send-failure path, since the PR already adds tests for the provisioning race.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/chat/shouldAutoSendInitialMessage.ts` around lines 32 - 37, The shouldAutoSendInitialMessage gate is stateless and can re-fire the initial send after a retry truncates messages back to empty. Update shouldAutoSendInitialMessage to also consult a persisted “already attempted/sent” signal rather than only messagesLength, status, authenticated, userId, and sessionId, and wire that state through useVercelChat so deleteTrailingMessages and the earliestFailedUserMessageId retry flow cannot trigger a duplicate initialMessages[0] send. Add a regression test covering the auto-send-failure retry path to confirm the initial message is not sent twice.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/chat/shouldAutoSendInitialMessage.ts`:
- Around line 1-16: The ShouldAutoSendInitialMessageParams status field is too
broad and should match the actual useChat status union instead of string. Update
the type used by shouldAutoSendInitialMessage to reuse the useChat status
literal union ("submitted" | "streaming" | "ready" | "error"), so the ready
check is compile-time safe and typos are rejected. Keep the change focused on
the ShouldAutoSendInitialMessageParams interface and any related type import or
alias needed in shouldAutoSendInitialMessage.
- Around line 32-37: The shouldAutoSendInitialMessage gate is stateless and can
re-fire the initial send after a retry truncates messages back to empty. Update
shouldAutoSendInitialMessage to also consult a persisted “already
attempted/sent” signal rather than only messagesLength, status, authenticated,
userId, and sessionId, and wire that state through useVercelChat so
deleteTrailingMessages and the earliestFailedUserMessageId retry flow cannot
trigger a duplicate initialMessages[0] send. Add a regression test covering the
auto-send-failure retry path to confirm the initial message is not sent twice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8fb20e91-e80c-4d5f-90ae-4f55a22a9e00
⛔ Files ignored due to path filters (1)
lib/chat/__tests__/shouldAutoSendInitialMessage.test.tsis excluded by!**/*.test.*and included bylib/**
📒 Files selected for processing (2)
hooks/useVercelChat.tslib/chat/shouldAutoSendInitialMessage.ts
Preview verification — 2026-07-04 ✅Preview
Also exercised the no-double-send guard implicitly: the effect re-ran across several provisioning re-renders and sent exactly once. Environment note for future preview testing: chat previews point at Unit suite: 6 cases RED→GREEN incl. the provisioning race; full repo suite 80 passed; tsc no new errors vs main; prettier clean. Ready for review/merge (docs→none needed; client-only fix). |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 3/5
lib/chat/shouldAutoSendInitialMessage.tscan re-trigger the?q=auto-send when the first attempt leaves only the user message in state, which risks duplicate initial messages and repeated downstream processing in chat sessions; tighten the gate to treat an existing user-only initial message as already handled (or persist an explicit "initial message sent" flag) before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/chat/shouldAutoSendInitialMessage.ts">
<violation number="1" location="lib/chat/shouldAutoSendInitialMessage.ts:34">
P2: The `?q=` message can be sent again after a first attempt leaves only the user message in state, because this gate only blocks conversations with more than one message. Since `useChat` is not initialized with `initialMessages`, consider treating any existing message as “already sent” (`messagesLength > 0`).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| }: ShouldAutoSendInitialMessageParams): boolean { | ||
| if (!hasInitialMessages) return false; | ||
| if (status !== "ready") return false; | ||
| if (messagesLength > 1) return false; |
There was a problem hiding this comment.
P2: The ?q= message can be sent again after a first attempt leaves only the user message in state, because this gate only blocks conversations with more than one message. Since useChat is not initialized with initialMessages, consider treating any existing message as “already sent” (messagesLength > 0).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/chat/shouldAutoSendInitialMessage.ts, line 34:
<comment>The `?q=` message can be sent again after a first attempt leaves only the user message in state, because this gate only blocks conversations with more than one message. Since `useChat` is not initialized with `initialMessages`, consider treating any existing message as “already sent” (`messagesLength > 0`).</comment>
<file context>
@@ -0,0 +1,38 @@
+}: ShouldAutoSendInitialMessageParams): boolean {
+ if (!hasInitialMessages) return false;
+ if (status !== "ready") return false;
+ if (messagesLength > 1) return false;
+ if (!userId || !authenticated) return false;
+ if (!sessionId) return false;
</file context>
| if (messagesLength > 1) return false; | |
| if (messagesLength > 0) return false; |
…n extension hook (OCP) Review feedback on #1848/#1849: useVercelChat grew in both PRs, being modified for every deep-link change instead of extended. Move the whole ?q= behavior (prefill + provisioning-gated auto-fire + one-shot guards) into hooks/useInitialMessageAutoSend, composed like the existing extension hooks (useChatTransport, usePersistSelectedModel, useMessageLoader). useVercelChat now carries one hook call and is smaller than it was before #1848 (422 lines vs 424); future deep-link changes touch only the dedicated hook. No behavior change: the hook is a verbatim move of the two effects over the tested pure helpers (shouldAutoSendInitialMessage, getInitialMessageText). Full suite 83 passed; tsc no new errors vs main; prettier clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…1848 follow-up) (#1849) * feat: prefill the input with the ?q= prompt while provisioning (chat#1848 follow-up) After #1848 the deep-link message correctly waits for the session, but the wait is dead space: nothing on screen tells the customer their AGENTS click worked. Prefill the input with the prompt immediately, then auto-fire when the workspace goes ready — sending whatever is in the input at that moment, so an edit made while waiting sends the edited text and a cleared input sends nothing, exactly like pressing Send. - lib/chat/getInitialMessageText.ts (SRP, unit-tested): first text part of the deep-link message, undefined when nothing meaningful. - useVercelChat: one-shot prefill effect (never clobbers an existing conversation or typed text) + auto-fire builds the message from the live input; one-shot ref prevents double-sends. TDD RED->GREEN (3-case extractor suite); full suite 83 passed; tsc no new errors vs main; prettier clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: extract useInitialMessageAutoSend — deep-link behavior as an extension hook (OCP) Review feedback on #1848/#1849: useVercelChat grew in both PRs, being modified for every deep-link change instead of extended. Move the whole ?q= behavior (prefill + provisioning-gated auto-fire + one-shot guards) into hooks/useInitialMessageAutoSend, composed like the existing extension hooks (useChatTransport, usePersistSelectedModel, useMessageLoader). useVercelChat now carries one hook call and is smaller than it was before #1848 (422 lines vs 424); future deep-link changes touch only the dedicated hook. No behavior change: the hook is a verbatim move of the two effects over the tested pure helpers (shouldAutoSendInitialMessage, getInitialMessageText). Full suite 83 passed; tsc no new errors vs main; prettier clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: source auth state inside useInitialMessageAutoSend (KISS) authenticated/userId come from usePrivy/useUserProvider directly instead of riding through params — the hook's params now carry only what is chat-instance-scoped (initialMessages, status, messagesLength, sessionId, input/setInput, send). Suite 83 passed; tsc clean vs main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Fixes #1847 — customer-reported (2026-07-04): clicking any AGENTS card as a logged-in user showed the prefilled message, then "An error occurred, please try again!".
Root cause
The
?q=deep-link auto-send effect inuseVercelChat.tsgated onauthenticated + useChat readybut not on the new-chat bootstrap — it fired whilesessionIdwas stillundefined, so the transport POSTed/api/chatwith nosessionIdand the api validator 400'd (missing_fields: ["sessionId"]) → generic error toast. The manual Send button was already correctly gated on provisioning (#1747/#1767); the auto-send path skipped that gate and lost the race every time for logged-in users. Captured live in the issue: the failingPOST /api/chatlands beforePOST /api/sessionsreturns.What
lib/chat/shouldAutoSendInitialMessage.ts— the gate extracted as a pure function (SRP), mirroring the manual-send conditions plussessionId.hooks/useVercelChat.ts— the auto-send effect uses it, withsessionIdadded to the dep array: when the bootstrap lands, the effect re-runs and the queued?q=message sends. No UX change beyond "it works" — the message simply waits for the workspace dot to go ready, same as manual send.Tests (TDD, RED→GREEN)
6-case unit suite for the gate: happy path, provisioning race (the #1847 case), no initial message, busy transport (double-send guard), already-has-messages, pre-auth. Full repo suite 80 passed;
tsc --noEmitno new errors vs main (22 pre-existing); prettier clean.Verification
Preview verification (real login →
/chat?q=…→ assistant responds,POST /api/chatonly afterPOST /api/sessions) to follow as a PR comment.🤖 Generated with Claude Code
Summary by cubic
Fixes #1847: the /chat?q= auto-send now waits for a provisioned sessionId, preventing 400s and the generic error toast for logged-in users. The initial message queues and sends once the new chat session is ready.
Bug Fixes
shouldAutoSendInitialMessagecheck that mirrors manual-send and requires a validsessionId.sessionIdto the effect dependencies so it re-runs after bootstrap; send occurs only when authenticated, status is ready, and no prior messages exist.Refactors
lib/chat/shouldAutoSendInitialMessage(pure function).Written for commit bbef98d. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes