feat(chat): add /sessions/[sid]/chats/[cid] route + session-scoped loader#1752
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 57 minutes and 8 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThreads sessionId through routes, Chat props, providers, hooks, and APIs: adds session-scoped chat page, redirects legacy /chat/:roomId, updates Home bootstrap, requires sessionId in Chat and VercelChatProvider, rewrites message loader and getChatMessages, and adapts conversations/listing paths. ChangesSession-scoped chat routing and loader
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1fbcde025
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <Chat | ||
| id={chatId} | ||
| sessionId={sessionId} | ||
| initialMessages={data.messages} |
There was a problem hiding this comment.
For the canonical route, data.messages are persisted history, but in this codebase initialMessages is not passed into useChat; it is only consumed by the effect in useVercelChat that calls sendMessage(initialMessages[0]). As a result, opening or reloading any existing /sessions/{sid}/chats/{cid} chat renders with an empty transcript and then re-posts the first saved message to /api/chat/workflow once auth is ready, instead of restoring the full history.
Useful? React with 👍 / 👎.
| <Chat | ||
| id={chatId} | ||
| sessionId={sessionId} |
There was a problem hiding this comment.
Mounting Chat on this new route leaves useParams().roomId undefined, so useVercelChat's handleSendMessage takes its existing !roomId branch and calls window.history.replaceState({}, '', '/chat/${id}'). That means any message sent from /sessions/{sessionId}/chats/{chatId} is treated like a brand-new non-room chat and immediately navigates back to the legacy /chat/{chatId} URL, so the canonical workflow URL cannot survive normal use.
Useful? React with 👍 / 👎.
| const token = authenticated ? await getAccessToken() : null; | ||
| const result = await getSessionChat(sessionId, chatId, token); |
There was a problem hiding this comment.
When Privy is ready but the user is not yet authenticated, this effect still calls getSessionChat with a null token. For direct visits to a protected canonical chat, the API will return an auth error and SessionChatBootstrap stays on the error screen; because <Chat> never mounts in that state, the existing useAutoLogin() path in ChatContent never runs to prompt sign-in and retry the load.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
lib/sessions/getSessionChat.ts (1)
63-63: 💤 Low valueUnvalidated cast trusts the API shape entirely.
as SessionChatsilences the compiler without provingmessagesis actually aUIMessage[]. A malformed payload flows straight into<Chat initialMessages={...}>. Acceptable for a trusted first-party endpoint, but worth a runtime guard if this contract is still settling.🤖 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/sessions/getSessionChat.ts` at line 63, The current unconditional cast "return (await response.json()) as SessionChat" in getSessionChat trusts the API shape; replace it with a runtime validation: parse the JSON into a local variable (e.g., const data = await response.json()), verify data is an object, ensure data.messages is Array.isArray and that every element conforms to UIMessage minimal shape (check required fields like id/role/content or whatever UIMessage requires), and if validation fails throw/return a clear error; only then return data typed as SessionChat. Use the function name getSessionChat, the response variable, and the SessionChat/UIMessage type names to locate and implement the guard.
🤖 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.
Inline comments:
In `@components/VercelChat/SessionChatBootstrap.tsx`:
- Around line 25-39: The error and loading branches in SessionChatBootstrap (the
blocks checking status === "error" and status !== "ready" || !data and using
error?.message and Loader) are not announced to assistive tech; add role="alert"
to the error message container so screen readers announce the error (keep the
existing text fallback), and add role="status" to the loading container with an
accessible, visually-hidden label (e.g., an sr-only span saying "Loading chat…")
adjacent to the Loader component so the spinner is announced; ensure you use the
same className utility (sr-only or visually-hidden) used by the project for
hidden text.
In `@lib/sessions/getSessionChat.ts`:
- Around line 45-48: The fetch call in getSessionChat is not protected by a
timeout; wrap it with an AbortSignal-based timeout using feature-detection: if
AbortSignal.timeout exists use it to create a signal for the desired timeout,
otherwise create an AbortController, call setTimeout to controller.abort() after
the timeout, pass the resulting signal into fetch alongside headers, and ensure
any fallback timeout is cleared (clearTimeout) after fetch completes; update the
call site where response = await fetch(...) in getSessionChat (use sessionId,
chatId, headers, getClientApiBaseUrl) to include this timeout-safe signal.
---
Nitpick comments:
In `@lib/sessions/getSessionChat.ts`:
- Line 63: The current unconditional cast "return (await response.json()) as
SessionChat" in getSessionChat trusts the API shape; replace it with a runtime
validation: parse the JSON into a local variable (e.g., const data = await
response.json()), verify data is an object, ensure data.messages is
Array.isArray and that every element conforms to UIMessage minimal shape (check
required fields like id/role/content or whatever UIMessage requires), and if
validation fails throw/return a clear error; only then return data typed as
SessionChat. Use the function name getSessionChat, the response variable, and
the SessionChat/UIMessage type names to locate and implement the guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36a3ba63-4e9e-41f8-b90c-d6bcb66dc191
📒 Files selected for processing (5)
app/sessions/[sessionId]/chats/[chatId]/page.tsxcomponents/VercelChat/SessionChatBootstrap.tsxhooks/useSessionChat.tshooks/useVercelChat.tslib/sessions/getSessionChat.ts
| if (status === "error") { | ||
| return ( | ||
| <div className="flex size-full items-center justify-center text-sm text-red-600"> | ||
| {error?.message ?? "Failed to load chat"} | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (status !== "ready" || !data) { | ||
| return ( | ||
| <div className="flex size-full items-center justify-center"> | ||
| <Loader className="size-5 animate-spin text-muted-foreground" /> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Announce loading and error states to assistive tech.
The error message and the spinning Loader are purely visual — a screen-reader user gets silence on both. Adding role="alert" to the error and role="status" with an sr-only label to the spinner makes the states perceivable without changing the visuals.
♿ Proposed accessibility additions
if (status === "error") {
return (
- <div className="flex size-full items-center justify-center text-sm text-red-600">
+ <div
+ role="alert"
+ className="flex size-full items-center justify-center text-sm text-red-600"
+ >
{error?.message ?? "Failed to load chat"}
</div>
);
}
if (status !== "ready" || !data) {
return (
- <div className="flex size-full items-center justify-center">
+ <div role="status" className="flex size-full items-center justify-center">
<Loader className="size-5 animate-spin text-muted-foreground" />
+ <span className="sr-only">Loading chat…</span>
</div>
);
}📝 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 (status === "error") { | |
| return ( | |
| <div className="flex size-full items-center justify-center text-sm text-red-600"> | |
| {error?.message ?? "Failed to load chat"} | |
| </div> | |
| ); | |
| } | |
| if (status !== "ready" || !data) { | |
| return ( | |
| <div className="flex size-full items-center justify-center"> | |
| <Loader className="size-5 animate-spin text-muted-foreground" /> | |
| </div> | |
| ); | |
| } | |
| if (status === "error") { | |
| return ( | |
| <div | |
| role="alert" | |
| className="flex size-full items-center justify-center text-sm text-red-600" | |
| > | |
| {error?.message ?? "Failed to load chat"} | |
| </div> | |
| ); | |
| } | |
| if (status !== "ready" || !data) { | |
| return ( | |
| <div role="status" className="flex size-full items-center justify-center"> | |
| <Loader className="size-5 animate-spin text-muted-foreground" /> | |
| <span className="sr-only">Loading chat…</span> | |
| </div> | |
| ); | |
| } |
🤖 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 `@components/VercelChat/SessionChatBootstrap.tsx` around lines 25 - 39, The
error and loading branches in SessionChatBootstrap (the blocks checking status
=== "error" and status !== "ready" || !data and using error?.message and Loader)
are not announced to assistive tech; add role="alert" to the error message
container so screen readers announce the error (keep the existing text
fallback), and add role="status" to the loading container with an accessible,
visually-hidden label (e.g., an sr-only span saying "Loading chat…") adjacent to
the Loader component so the spinner is announced; ensure you use the same
className utility (sr-only or visually-hidden) used by the project for hidden
text.
| const response = await fetch( | ||
| `${getClientApiBaseUrl()}/api/sessions/${encodeURIComponent(sessionId)}/chats/${encodeURIComponent(chatId)}`, | ||
| { headers }, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is AbortSignal.timeout supported across the browsers targeted by this project?
💡 Result:
Yes—AbortSignal.timeout is supported by all major modern browsers, but not by very old versions. According to Can I use’s compatibility table, support is available in: - Chrome: supported from 124+ [1] - Edge: supported from 124+ [1] - Firefox: supported from 100+ [1] - Safari: supported from 16+ [1] MDN also describes AbortSignal.timeout and notes it may not work in older devices/browsers (and that unsupported implementations can lead to a TypeError) [2]. If your project targets “all major browsers” on current versions, you can generally rely on AbortSignal.timeout. If you need to support older browser versions (e.g., pre-124 Chrome/Edge, pre-100 Firefox, pre-16 Safari/iOS Safari), you should feature-detect and/or provide a fallback (e.g., using AbortController + setTimeout). MDN even points out that you may need to handle the case where the method is not supported [2].
Citations:
- 1: https://caniuse.com/wf-abortsignal-timeout
- 2: https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout_static
Guard the session chat bootstrap fetch with a timeout (feature-detect or fallback)
AbortSignal.timeout is supported in major modern browsers (Chrome/Edge 124+, Firefox 100+, Safari 16+), but unsupported implementations can throw (e.g., TypeError). Add a timeout to prevent the page loader from hanging indefinitely when the upstream API stalls.
File: lib/sessions/getSessionChat.ts (lines 45-48)
♻️ Proposed timeout via AbortSignal
const response = await fetch(
`${getClientApiBaseUrl()}/api/sessions/${encodeURIComponent(sessionId)}/chats/${encodeURIComponent(chatId)}`,
- { headers },
+ { headers, signal: AbortSignal.timeout(15000) },
);🤖 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/sessions/getSessionChat.ts` around lines 45 - 48, The fetch call in
getSessionChat is not protected by a timeout; wrap it with an AbortSignal-based
timeout using feature-detection: if AbortSignal.timeout exists use it to create
a signal for the desired timeout, otherwise create an AbortController, call
setTimeout to controller.abort() after the timeout, pass the resulting signal
into fetch alongside headers, and ensure any fallback timeout is cleared
(clearTimeout) after fetch completes; update the call site where response =
await fetch(...) in getSessionChat (use sessionId, chatId, headers,
getClientApiBaseUrl) to include this timeout-safe signal.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 5 files
Confidence score: 2/5
- Merge risk is high because there are multiple user-facing flow risks at severity 6–8/10, including chat initialization and routing behavior that can fail on first load.
- Most severe: in
components/VercelChat/SessionChatBootstrap.tsx, ifinitialMessagesis handled viasendMessageinstead of true initial seeding inuseVercelChat, the first message path can be incorrect and bootstrap behavior may regress. components/VercelChat/SessionChatBootstrap.tsxandhooks/useSessionChat.tstogether suggest a fragile startup path: undefinedroomIdcan triggerreplaceStateaway from the intended session, andgetSessionChatmay be called with a null token whenreadyis true but auth is not complete, leading to visible auth errors.- Pay close attention to
components/VercelChat/SessionChatBootstrap.tsx,hooks/useSessionChat.ts- initialization/routing/auth timing issues can break session chat entry and expose backend error text.
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="components/VercelChat/SessionChatBootstrap.tsx">
<violation number="1" location="components/VercelChat/SessionChatBootstrap.tsx:28">
P2: Avoid rendering raw backend error messages in the chat bootstrap UI; show a generic user-safe message instead.</violation>
<violation number="2" location="components/VercelChat/SessionChatBootstrap.tsx:44">
P1: On this route `useParams().roomId` is undefined, so `useVercelChat`'s `handleSendMessage` will take the `!roomId` branch and call `window.history.replaceState` to navigate to `/chat/{id}` — immediately abandoning the canonical workflow URL after the first message is sent. The send handler needs to be aware of the session-based route to preserve it.</violation>
<violation number="3" location="components/VercelChat/SessionChatBootstrap.tsx:45">
P1: Verify that `initialMessages` actually seeds the message list in `useVercelChat` rather than triggering a `sendMessage` call. If the hook has an effect that calls `sendMessage(initialMessages[0])` instead of passing `messages: initialMessages` to `useChat`, reloading this page will re-post the first message to the workflow API rather than restoring the full persisted history.</violation>
</file>
<file name="hooks/useSessionChat.ts">
<violation number="1" location="hooks/useSessionChat.ts:37">
P2: When `ready` is true but `authenticated` is false (e.g. direct navigation before auto-login completes), this proceeds to call `getSessionChat` with a null token. The API will return an auth error, the bootstrap shows the error screen, and since `<Chat>` never mounts, the existing `useAutoLogin()` flow in `ChatContent` never triggers to prompt sign-in. Consider waiting for `authenticated` to be true before fetching, or handling the unauthenticated case explicitly.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return ( | ||
| <Chat | ||
| id={chatId} | ||
| sessionId={sessionId} |
There was a problem hiding this comment.
P1: On this route useParams().roomId is undefined, so useVercelChat's handleSendMessage will take the !roomId branch and call window.history.replaceState to navigate to /chat/{id} — immediately abandoning the canonical workflow URL after the first message is sent. The send handler needs to be aware of the session-based route to preserve it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/VercelChat/SessionChatBootstrap.tsx, line 44:
<comment>On this route `useParams().roomId` is undefined, so `useVercelChat`'s `handleSendMessage` will take the `!roomId` branch and call `window.history.replaceState` to navigate to `/chat/{id}` — immediately abandoning the canonical workflow URL after the first message is sent. The send handler needs to be aware of the session-based route to preserve it.</comment>
<file context>
@@ -0,0 +1,48 @@
+ return (
+ <Chat
+ id={chatId}
+ sessionId={sessionId}
+ initialMessages={data.messages}
+ />
</file context>
| <Chat | ||
| id={chatId} | ||
| sessionId={sessionId} | ||
| initialMessages={data.messages} |
There was a problem hiding this comment.
P1: Verify that initialMessages actually seeds the message list in useVercelChat rather than triggering a sendMessage call. If the hook has an effect that calls sendMessage(initialMessages[0]) instead of passing messages: initialMessages to useChat, reloading this page will re-post the first message to the workflow API rather than restoring the full persisted history.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/VercelChat/SessionChatBootstrap.tsx, line 45:
<comment>Verify that `initialMessages` actually seeds the message list in `useVercelChat` rather than triggering a `sendMessage` call. If the hook has an effect that calls `sendMessage(initialMessages[0])` instead of passing `messages: initialMessages` to `useChat`, reloading this page will re-post the first message to the workflow API rather than restoring the full persisted history.</comment>
<file context>
@@ -0,0 +1,48 @@
+ <Chat
+ id={chatId}
+ sessionId={sessionId}
+ initialMessages={data.messages}
+ />
+ );
</file context>
| if (status === "error") { | ||
| return ( | ||
| <div className="flex size-full items-center justify-center text-sm text-red-600"> | ||
| {error?.message ?? "Failed to load chat"} |
There was a problem hiding this comment.
P2: Avoid rendering raw backend error messages in the chat bootstrap UI; show a generic user-safe message instead.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At components/VercelChat/SessionChatBootstrap.tsx, line 28:
<comment>Avoid rendering raw backend error messages in the chat bootstrap UI; show a generic user-safe message instead.</comment>
<file context>
@@ -0,0 +1,48 @@
+ if (status === "error") {
+ return (
+ <div className="flex size-full items-center justify-center text-sm text-red-600">
+ {error?.message ?? "Failed to load chat"}
+ </div>
+ );
</file context>
| setStatus("loading"); | ||
| setError(null); | ||
| try { | ||
| const token = authenticated ? await getAccessToken() : null; |
There was a problem hiding this comment.
P2: When ready is true but authenticated is false (e.g. direct navigation before auto-login completes), this proceeds to call getSessionChat with a null token. The API will return an auth error, the bootstrap shows the error screen, and since <Chat> never mounts, the existing useAutoLogin() flow in ChatContent never triggers to prompt sign-in. Consider waiting for authenticated to be true before fetching, or handling the unauthenticated case explicitly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At hooks/useSessionChat.ts, line 37:
<comment>When `ready` is true but `authenticated` is false (e.g. direct navigation before auto-login completes), this proceeds to call `getSessionChat` with a null token. The API will return an auth error, the bootstrap shows the error screen, and since `<Chat>` never mounts, the existing `useAutoLogin()` flow in `ChatContent` never triggers to prompt sign-in. Consider waiting for `authenticated` to be true before fetching, or handling the unauthenticated case explicitly.</comment>
<file context>
@@ -0,0 +1,57 @@
+ setStatus("loading");
+ setError(null);
+ try {
+ const token = authenticated ? await getAccessToken() : null;
+ const result = await getSessionChat(sessionId, chatId, token);
+ if (cancelled) return;
</file context>
arpitgupta1214
force-pushed
the
feat/sessions-chat-page
branch
from
f1fbcde to
e97370a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@app/sessions/`[sessionId]/chats/[chatId]/page.tsx:
- Around line 12-16: This route renders <Chat id={chatId} sessionId={sessionId}
/> without initialMessages causing useVercelChat/useMessageLoader to fetch
legacy chat-id messages; change the page to bootstrap session-scoped messages
and pass them as initialMessages to Chat (or use the SessionChatBootstrap/fetch
for GET /api/sessions/{sessionId}/chats/{chatId}) so hydration uses session
workflow; specifically, call the session-scoped loader (or getChatMessages
variant that accepts sessionId) to fetch messages for the given sessionId and
chatId and supply that result to the Chat component via its initialMessages
prop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4d48970b-6a03-42c2-82bd-ca5794be7302
📒 Files selected for processing (1)
app/sessions/[sessionId]/chats/[chatId]/page.tsx
arpitgupta1214
changed the title
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
arpitgupta1214
changed the title
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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="hooks/useSessionChat.ts">
<violation number="1" location="hooks/useSessionChat.ts:37">
P2: When `ready` is true but `authenticated` is false (e.g. direct navigation before auto-login completes), this proceeds to call `getSessionChat` with a null token. The API will return an auth error, the bootstrap shows the error screen, and since `<Chat>` never mounts, the existing `useAutoLogin()` flow in `ChatContent` never triggers to prompt sign-in. Consider waiting for `authenticated` to be true before fetching, or handling the unauthenticated case explicitly.</violation>
</file>
<file name="components/VercelChat/SessionChatBootstrap.tsx">
<violation number="1" location="components/VercelChat/SessionChatBootstrap.tsx:28">
P2: Avoid rendering raw backend error messages in the chat bootstrap UI; show a generic user-safe message instead.</violation>
<violation number="2" location="components/VercelChat/SessionChatBootstrap.tsx:44">
P1: On this route `useParams().roomId` is undefined, so `useVercelChat`'s `handleSendMessage` will take the `!roomId` branch and call `window.history.replaceState` to navigate to `/chat/{id}` — immediately abandoning the canonical workflow URL after the first message is sent. The send handler needs to be aware of the session-based route to preserve it.</violation>
<violation number="3" location="components/VercelChat/SessionChatBootstrap.tsx:45">
P1: Verify that `initialMessages` actually seeds the message list in `useVercelChat` rather than triggering a `sendMessage` call. If the hook has an effect that calls `sendMessage(initialMessages[0])` instead of passing `messages: initialMessages` to `useChat`, reloading this page will re-post the first message to the workflow API rather than restoring the full persisted history.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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="providers/UserProvder.tsx">
<violation number="1" location="providers/UserProvder.tsx:17">
P1: Calling `useAutoLogin()` inside `UserProvider` uses context before the provider exists, so auto-login checks run against the default context and can incorrectly force a login modal.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // Open the Privy login modal once when there's no signed-in user. | ||
| // Owning auto-login at the provider layer means page components | ||
| // don't have to remember to call `useAutoLogin()` themselves. | ||
| useAutoLogin(); |
There was a problem hiding this comment.
P1: Calling useAutoLogin() inside UserProvider uses context before the provider exists, so auto-login checks run against the default context and can incorrectly force a login modal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At providers/UserProvder.tsx, line 17:
<comment>Calling `useAutoLogin()` inside `UserProvider` uses context before the provider exists, so auto-login checks run against the default context and can incorrectly force a login modal.</comment>
<file context>
@@ -10,6 +11,11 @@ const UserContext = createContext<ReturnType<typeof useUser>>(
+ // Open the Privy login modal once when there's no signed-in user.
+ // Owning auto-login at the provider layer means page components
+ // don't have to remember to call `useAutoLogin()` themselves.
+ useAutoLogin();
+
const value = useMemo(() => ({ ...user }), [user]);
</file context>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hooks/useMessageLoader.ts (1)
22-55:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard against stale/out-of-order loads overwriting live messages in
useMessageLoader
getAccessTokenfromusePrivyhas no documented stable/memoized function-identity guarantee, so this effect can re-run and race an in-flightgetChatMessagescall. Since there’s no cancellation/cleanup, an older request can resolve later and clobber live/streamed conversation state.🛡️ Proposed cancellation guard
useEffect(() => { if (!userId) { setIsLoading(true); return; } + let cancelled = false; const loadMessages = async () => { setIsLoading(true); setError(null); try { const accessToken = await getAccessToken(); if (!accessToken) return; const initialMessages = await getChatMessages( sessionId, chatId, accessToken, ); - if (initialMessages.length > 0) { + if (!cancelled && initialMessages.length > 0) { setMessages(initialMessages); } } catch (err) { console.error("Error loading messages:", err); - setError( - err instanceof Error ? err : new Error("Failed to load messages"), - ); + if (!cancelled) + setError( + err instanceof Error ? err : new Error("Failed to load messages"), + ); } finally { - setIsLoading(false); + if (!cancelled) setIsLoading(false); } }; loadMessages(); + return () => { + cancelled = true; + }; }, [userId, sessionId, chatId, getAccessToken, setMessages]);🤖 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 `@hooks/useMessageLoader.ts` around lines 22 - 55, The effect in useMessageLoader can race because getAccessToken/getChatMessages may resolve after a later run; modify the loadMessages logic to prevent stale responses from overwriting live state by introducing a cancellation guard: create a local token (e.g., runId or cancelled boolean/AbortController) captured when the effect starts, pass an AbortSignal to getChatMessages if it accepts one, and in the finally/after-await blocks check the token (or signal) before calling setMessages, setError, or setIsLoading; also set the token as cancelled in the effect cleanup so any in-flight promise knows not to mutate state. Ensure you update loadMessages, the useEffect cleanup, and the checks around setMessages/setIsLoading to reference this guard.hooks/useVercelChat.ts (1)
321-326:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard optimistic “New Chat” creation by whether this chat has prior messages (not
roomId)On the canonical page
app/sessions/[sessionId]/chats/[chatId]/page.tsx,useParams()has noroomIdparam, and the legacyapp/chat/[roomId]/page.tsxjust redirects to/chat—soroomIdis effectively alwaysundefinedon canonical routes. That makes theif (!roomId)branch fire on every send (including follow-ups), repeatedly callingaddOptimisticConversation("New Chat", id, ...). Also,addOptimisticConversationprepends a temp conversation to the react-query cache without deduping bychatId(it only skips when the pathname matches/chat/<...>), so duplicates in Recent Chats are possible until the refetch catches up.🐛 Gate on new-chat state instead of the stale `roomId` param
// Capture the input value before it's cleared by handleSubmit const messageContent = input; + // Brand-new chats have no prior messages. The canonical route no longer + // exposes a `roomId` param, so message count is the reliable signal. + const isNewChat = messages.length === 0; // Submit the message handleSubmit(event); - if (!roomId) { + if (isNewChat) { // Optimistically append a temporary conversation so it appears in Recent Chats // It will be replaced by the real conversation after the updates/refetch addOptimisticConversation("New Chat", id, messageContent); silentlyUpdateUrl(); }After this change,
roomId/useParamsinuseVercelChatmay become unused in the hook.🤖 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 `@hooks/useVercelChat.ts` around lines 321 - 326, The optimistic "New Chat" creation is currently gated by roomId which is often undefined; update useVercelChat to gate calling addOptimisticConversation("New Chat", id, messageContent) by whether this chat truly has no prior messages (e.g., check the messages array or a conversation.messages.length for the chat id) so it only runs for genuinely new conversations, and keep silentlyUpdateUrl() in the same guarded branch; also remove/unused the roomId usage in the hook if it becomes unnecessary and ensure addOptimisticConversation is not called for follow-up sends (or add a quick dedupe check by id before prepending).
🧹 Nitpick comments (1)
lib/messages/getChatMessages.ts (1)
22-25: ⚡ Quick winNon-OK responses are silently flattened to an empty chat.
Returning
[]on!response.okmeans a 401/403/5xx is indistinguishable from a genuinely empty conversation, so thehasErrorbranch inchat.tsx("Failed to load messages") never fires for server failures — the user just sees the empty greeting. Consider throwing on non-OK souseMessageLoader's catch can surface the existing error UI.♻️ Surface server errors instead of masking them
- if (!response.ok) return []; + if (!response.ok) { + throw new Error(`Failed to load chat messages (${response.status})`); + } const data = (await response.json()) as { messages?: UIMessage[] }; return data.messages ?? [];This depends on the recoup-api contract for
GET /api/sessions/{sid}/chats/{cid}— confirm it returns a non-2xx (not200with emptymessages) for auth/lookup failures.🤖 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/messages/getChatMessages.ts` around lines 22 - 25, The current getChatMessages function masks server errors by returning [] when response.ok is false; update getChatMessages to throw an Error (including response.status and a brief response body or statusText) when !response.ok so callers like useMessageLoader can catch and surface the "Failed to load messages" UI; ensure the thrown error includes identifying info (status/statusText/body) and preserve the existing return of data.messages ?? [] only for successful responses.
🤖 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.
Inline comments:
In `@app/page.tsx`:
- Line 11: The code currently asserts searchParams.q as a string for
initialMessage which is unsafe; update the logic around initialMessage to handle
searchParams.q being string | string[] | undefined by checking
Array.isArray(searchParams?.q) and extracting a single string (e.g., take the
first element) or handling undefined (e.g., default to empty string or
undefined) before passing it to getMessages; modify the usage of initialMessage
so getMessages receives a properly typed string/undefined rather than relying on
a type assertion.
---
Outside diff comments:
In `@hooks/useMessageLoader.ts`:
- Around line 22-55: The effect in useMessageLoader can race because
getAccessToken/getChatMessages may resolve after a later run; modify the
loadMessages logic to prevent stale responses from overwriting live state by
introducing a cancellation guard: create a local token (e.g., runId or cancelled
boolean/AbortController) captured when the effect starts, pass an AbortSignal to
getChatMessages if it accepts one, and in the finally/after-await blocks check
the token (or signal) before calling setMessages, setError, or setIsLoading;
also set the token as cancelled in the effect cleanup so any in-flight promise
knows not to mutate state. Ensure you update loadMessages, the useEffect
cleanup, and the checks around setMessages/setIsLoading to reference this guard.
In `@hooks/useVercelChat.ts`:
- Around line 321-326: The optimistic "New Chat" creation is currently gated by
roomId which is often undefined; update useVercelChat to gate calling
addOptimisticConversation("New Chat", id, messageContent) by whether this chat
truly has no prior messages (e.g., check the messages array or a
conversation.messages.length for the chat id) so it only runs for genuinely new
conversations, and keep silentlyUpdateUrl() in the same guarded branch; also
remove/unused the roomId usage in the hook if it becomes unnecessary and ensure
addOptimisticConversation is not called for follow-up sends (or add a quick
dedupe check by id before prepending).
---
Nitpick comments:
In `@lib/messages/getChatMessages.ts`:
- Around line 22-25: The current getChatMessages function masks server errors by
returning [] when response.ok is false; update getChatMessages to throw an Error
(including response.status and a brief response body or statusText) when
!response.ok so callers like useMessageLoader can catch and surface the "Failed
to load messages" UI; ensure the thrown error includes identifying info
(status/statusText/body) and preserve the existing return of data.messages ?? []
only for successful responses.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca5e6a98-f78b-4b5e-9756-8bb0ea13e887
📒 Files selected for processing (9)
app/chat/[roomId]/page.tsxapp/page.tsxcomponents/Home/HomePage.tsxcomponents/VercelChat/chat.tsxhooks/useMessageLoader.tshooks/useVercelChat.tslib/messages/getChatMessages.tsproviders/UserProvder.tsxproviders/VercelChatProvider.tsx
|
|
||
| export default async function Home({ searchParams }: ChatPageProps) { | ||
| const id = generateUUID(); | ||
| const initialMessage = (await searchParams)?.q as string; |
There was a problem hiding this comment.
Handle searchParams.q type safely instead of using type assertion.
The q parameter can be string | string[] | undefined, but the code asserts it as string. If a user provides multiple q parameters (e.g., /?q=hello&q=world), searchParams.q will be an array, which could cause unexpected behavior in getMessages.
🛡️ Proposed fix to handle all possible types
- const initialMessage = (await searchParams)?.q as string;
+ const q = (await searchParams)?.q;
+ const initialMessage = Array.isArray(q) ? q[0] : q;
const initialMessages = getMessages(initialMessage);As per coding guidelines: "Use strict TypeScript types with zero 'any' types" — type assertions bypass type safety and should be avoided when possible.
🤖 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 `@app/page.tsx` at line 11, The code currently asserts searchParams.q as a
string for initialMessage which is unsafe; update the logic around
initialMessage to handle searchParams.q being string | string[] | undefined by
checking Array.isArray(searchParams?.q) and extracting a single string (e.g.,
take the first element) or handling undefined (e.g., default to empty string or
undefined) before passing it to getMessages; modify the usage of initialMessage
so getMessages receives a properly typed string/undefined rather than relying on
a type assertion.
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 5 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
hooks/useConversations.tsx (1)
56-67:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix misleading “RecentChats filter” comment for optimistic
memories(hooks/useConversations.tsx ~56-67): RecentChats doesn’t filter the conversation list based onmemories;chatRoom.memoriesis used byChatItem/useCreateChatto detect an optimistic chat item (pending/disabled UI and extracting the first message to create the chat). Update the inline comment to reflect that behavior instead of implying it’s a filtering mechanism.🤖 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 `@hooks/useConversations.tsx` around lines 56 - 67, Update the misleading comment above the optimistic `memories` array in useConversations (the block that sets id: `${chatId}-m1` and content.optimistic) so it states that `chatRoom.memories` is used by `ChatItem`/`useCreateChat` to mark an optimistic/pending chat item and to extract the first message for creation, rather than implying RecentChats filters conversations by `memories`; keep the existing optimistic memory structure but change the comment to accurately describe detection/extraction behavior.
🧹 Nitpick comments (1)
hooks/useClickChat.tsx (1)
9-12: ⚡ Quick winDon’t add a missing-
sessionIdguard inuseClickChat; enforce/normalize upstream instead.
Conversation.sessionIdisstring(non-optional) intypes/Chat.tsx, so the/sessions/undefined/...outcome shouldn’t happen for correctly typed data. If the PR regression is truly from API rows omittingsessionId, fix/validate the mapping whereConversationobjects are constructed (or add a dev-time invariant) rather than silently returning insideuseClickChat.🤖 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 `@hooks/useClickChat.tsx` around lines 9 - 12, The review flags that handleClick in useClickChat silently guards against a missing sessionId even though Conversation.sessionId is typed as non-optional; instead of adding a runtime early-return here, find and fix the upstream mapping/creation of Conversation objects so sessionId is always populated (or add a dev-only invariant check where Conversations are constructed). Specifically, remove any added missing-session guard in useClickChat/handleClick, and normalize/validate sessionId at the producer side (where Conversation objects are built or fetched) — if needed add a development-time assert (e.g., throw or console.error in development) at the Conversation factory/mapping code to surface missing sessionId bugs early.
🤖 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.
Inline comments:
In `@components/VercelChat/tools/chats/GetChatsResult.tsx`:
- Around line 48-64: The Link href currently interpolates chat.sessionId without
a runtime check, which can produce /sessions/undefined/chats/{id}; update the
render in GetChatsResult.tsx to guard chat.sessionId before building the href
used in the Link (the element that creates
`/sessions/${chat.sessionId}/chats/${chat.id}`), e.g. skip rendering that list
item or render a non-clickable fallback (no href/disabled link) when
chat.sessionId is falsy; ensure the check is performed where displayTitle is
computed and before the Link is returned so rows with missing sessionId are not
rendered as broken links.
---
Outside diff comments:
In `@hooks/useConversations.tsx`:
- Around line 56-67: Update the misleading comment above the optimistic
`memories` array in useConversations (the block that sets id: `${chatId}-m1` and
content.optimistic) so it states that `chatRoom.memories` is used by
`ChatItem`/`useCreateChat` to mark an optimistic/pending chat item and to
extract the first message for creation, rather than implying RecentChats filters
conversations by `memories`; keep the existing optimistic memory structure but
change the comment to accurately describe detection/extraction behavior.
---
Nitpick comments:
In `@hooks/useClickChat.tsx`:
- Around line 9-12: The review flags that handleClick in useClickChat silently
guards against a missing sessionId even though Conversation.sessionId is typed
as non-optional; instead of adding a runtime early-return here, find and fix the
upstream mapping/creation of Conversation objects so sessionId is always
populated (or add a dev-only invariant check where Conversations are
constructed). Specifically, remove any added missing-session guard in
useClickChat/handleClick, and normalize/validate sessionId at the producer side
(where Conversation objects are built or fetched) — if needed add a
development-time assert (e.g., throw or console.error in development) at the
Conversation factory/mapping code to surface missing sessionId bugs early.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08fdd416-8f65-48b3-8420-3a5fdd783754
⛔ Files ignored due to path filters (2)
lib/__tests__/getConversations.test.tsis excluded by!**/*.test.*and included bylib/**types/Chat.tsxis excluded by none and included by none
📒 Files selected for processing (6)
app/chat/[roomId]/page.tsxcomponents/VercelChat/tools/chats/GetChatsResult.tsxhooks/useClickChat.tsxhooks/useConversations.tsxhooks/useVercelChat.tslib/getConversations.tsx
💤 Files with no reviewable changes (1)
- app/chat/[roomId]/page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/useVercelChat.ts
| const displayTitle = | ||
| chat.title && chat.title.trim().length > 0 | ||
| ? chat.title | ||
| : "Untitled Chat"; | ||
|
|
||
| return ( | ||
| <li key={chat.id}> | ||
| <Link | ||
| href={`/chat/${chat.id}`} | ||
| href={`/sessions/${chat.sessionId}/chats/${chat.id}`} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="flex items-center gap-2 p-2 rounded-md hover:bg-muted transition-colors group" | ||
| > | ||
| <MessageSquare className="h-4 w-4 text-muted-foreground shrink-0" /> | ||
| <span className="text-sm text-foreground truncate flex-1"> | ||
| {displayTopic} | ||
| {displayTitle} | ||
| </span> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the get_chats MCP tool projection includes sessionId per row
rg -nP -C4 'get_chats|GetChatsResult' --type=ts --type=tsx
rg -nP -C3 '\bsessionId\b' -g '*hats*'Repository: recoupable/chat
Length of output: 1897
🏁 Script executed:
#!/bin/bash
# Inspect how GetChatsResult gets its data (and whether sessionId is runtime-enforced)
sed -n '1,220p' components/VercelChat/tools/chats/GetChatsResult.tsxRepository: recoupable/chat
Length of output: 2879
🏁 Script executed:
#!/bin/bash
# Find where/if get_chats is parsed/cast into GetChatsResultRow[]
rg -n "get_chats|GetChatsResultType|GetChatsResultRow" components/VercelChat/tools/chats -SRepository: recoupable/chat
Length of output: 529
🏁 Script executed:
#!/bin/bash
# Search for the MCP tool client/handler that defines get_chats projection
rg -n "get_chats" components/VercelChat -SRepository: recoupable/chat
Length of output: 349
🏁 Script executed:
#!/bin/bash
sed -n '150,260p' components/VercelChat/ToolComponents.tsxRepository: recoupable/chat
Length of output: 3071
🏁 Script executed:
#!/bin/bash
sed -n '340,460p' components/VercelChat/ToolComponents.tsxRepository: recoupable/chat
Length of output: 2260
🏁 Script executed:
#!/bin/bash
# Locate any server/tool code that constructs the get_chats response (and possibly validates sessionId)
rg -n "get_chats" -S .Repository: recoupable/chat
Length of output: 355
🏁 Script executed:
#!/bin/bash
sed -n '330,430p' components/VercelChat/ToolComponents.tsxRepository: recoupable/chat
Length of output: 2523
🏁 Script executed:
#!/bin/bash
sed -n '1,120p' components/VercelChat/ToolComponents.tsxRepository: recoupable/chat
Length of output: 5461
🏁 Script executed:
#!/bin/bash
rg -n "JSON\.parse\\(\\(output as CallToolResult\\)" -n components/VercelChat/ToolComponents.tsx
sed -n '240,360p' components/VercelChat/ToolComponents.tsxRepository: recoupable/chat
Length of output: 3593
🏁 Script executed:
#!/bin/bash
# Show the MCP parsing/cast path used for all dynamic tools (including get_chats)
sed -n '200,260p' components/VercelChat/ToolComponents.tsxRepository: recoupable/chat
Length of output: 1898
Guard sessionId before building the chat href
displayTitle fallback is solid, but the Link URL trusts chat.sessionId—get_chats is decoded via JSON.parse(...) and then only type-cast to GetChatsResultType (no runtime validation). If any MCP row ever lacks sessionId, you’ll generate /sessions/undefined/chats/{id} and 404 in a new tab; skip/disable those rows or add a runtime guard in components/VercelChat/tools/chats/GetChatsResult.tsx.
🤖 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 `@components/VercelChat/tools/chats/GetChatsResult.tsx` around lines 48 - 64,
The Link href currently interpolates chat.sessionId without a runtime check,
which can produce /sessions/undefined/chats/{id}; update the render in
GetChatsResult.tsx to guard chat.sessionId before building the href used in the
Link (the element that creates `/sessions/${chat.sessionId}/chats/${chat.id}`),
e.g. skip rendering that list item or render a non-clickable fallback (no
href/disabled link) when chat.sessionId is falsy; ensure the check is performed
where displayTitle is computed and before the Link is returned so rows with
missing sessionId are not rendered as broken links.
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
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/getConversations.tsx">
<violation number="1" location="lib/getConversations.tsx:7">
P1: Custom agent: **Flag AI Slop and Fabricated Changes**
Comment and interface claim `/api/chats` rows always include `sessionId`, but PR says the listing endpoint is still rooms-shaped and lacks `sessionId` per row</violation>
<violation number="2" location="lib/getConversations.tsx:62">
P1: `sessionId` is mapped without a runtime guard even though `/api/chats` may still omit it, so conversation items can carry `undefined` and break canonical chat linking.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| * The account is automatically inferred from the authentication token. | ||
| * Wire shape of each row returned by recoup-api `GET /api/chats`. | ||
| * Mirrors the session-scoped projection landed in api H1: every chat | ||
| * is joined to its parent session, so `sessionId` is always present. |
There was a problem hiding this comment.
P1: Custom agent: Flag AI Slop and Fabricated Changes
Comment and interface claim /api/chats rows always include sessionId, but PR says the listing endpoint is still rooms-shaped and lacks sessionId per row
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/getConversations.tsx, line 7:
<comment>Comment and interface claim `/api/chats` rows always include `sessionId`, but PR says the listing endpoint is still rooms-shaped and lacks `sessionId` per row</comment>
<file context>
@@ -2,22 +2,32 @@ import type { Conversation } from "@/types/Chat";
- * The account is automatically inferred from the authentication token.
+ * Wire shape of each row returned by recoup-api `GET /api/chats`.
+ * Mirrors the session-scoped projection landed in api H1: every chat
+ * is joined to its parent session, so `sessionId` is always present.
+ */
+interface ApiChatRow {
</file context>
| id: row.id, | ||
| topic: row.title, | ||
| account_id: row.accountId, | ||
| sessionId: row.sessionId, |
There was a problem hiding this comment.
P1: sessionId is mapped without a runtime guard even though /api/chats may still omit it, so conversation items can carry undefined and break canonical chat linking.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/getConversations.tsx, line 62:
<comment>`sessionId` is mapped without a runtime guard even though `/api/chats` may still omit it, so conversation items can carry `undefined` and break canonical chat linking.</comment>
<file context>
@@ -38,13 +48,21 @@ const getConversations = async (
+ id: row.id,
+ topic: row.title,
+ account_id: row.accountId,
+ sessionId: row.sessionId,
+ updated_at: row.updatedAt,
+ }),
</file context>
4a18682 to
0426309
Compare
Mounts <Chat id sessionId> for the canonical workflow URL. Loader
plumbing (useMessageLoader → /api/sessions/{sid}/chats/{cid}) ships
in E2; this PR is route-only.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
useMessageLoader takes (sessionId, chatId) and fetches from
GET /api/sessions/{sid}/chats/{cid}; legacy memories path deleted.
Skips when either id is missing — chats opened without a sessionId
(legacy /chat/{roomId} route) render an empty transcript instead of
falling back to memories.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
arpitgupta1214
force-pushed
the
feat/sessions-chat-page
branch
from
0426309 to
1efc22a
Compare
arpitgupta1214
changed the title
Manual test results — preview ✅Tested against the preview deployment Tests
Notes
🤖 Tested with Claude Code |
2 participants
Adds the canonical workflow URL
/sessions/{sessionId}/chats/{chatId}and points the message loader at the matching session-scoped api endpoint.app/sessions/[sessionId]/chats/[chatId]/page.tsxmounts<Chat id={chatId} sessionId={sessionId} />.useMessageLoadertakes(sessionId, chatId, ...)and fetches fromGET /api/sessions/{sid}/chats/{cid}; legacymemoriesfetcher deleted./chat/{roomId}route still renders, just with an empty transcript (no fallback to memories).The sidebar PR (chat#1756) emits canonical URLs that this route receives. Merge order: chat#1756 → chat#1752 (back-to-back; sidebar links 404 between them).
silentlyUpdateUrlrewrite + HomePage refactor live in a follow-up PR.Test plan
/sessions/{sid}/chats/{cid}for a workflow chat → messages load → reload preserves them./chat/{roomId}still resolves but shows an empty transcript.