feat(web): show login wall on duplicated chat for askgh#939
Open
feat(web): show login wall on duplicated chat for askgh#939
Conversation
…cated chat When EXPERIMENT_ASK_GH_ENABLED is true, anonymous users who duplicate a public chat and try to send a follow-up message are now shown a login modal instead of sending the message. After OAuth redirect, the pending message is automatically restored and submitted. Also fixes vi.Mock type errors in listCommitsApi.test.ts by importing Mock type directly from vitest. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
msukkari
added a commit
that referenced
this pull request
Feb 25, 2026
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
WalkthroughAdds an AskGH "login wall" flow: unauthenticated send attempts store a pending message, fetch identity-provider info, open a LoginModal for OAuth, and restore/resend the pending message after authentication. Also updates hook and component signatures and test mocks; adds a changelog entry. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatThread
participant Server
participant SessionStorage
participant LoginModal
participant OAuthProvider
User->>ChatThread: Submit message (unauthenticated)
ChatThread->>Server: getAskGhLoginWallData()
Server-->>ChatThread: { isEnabled, providers }
ChatThread->>SessionStorage: store pending message
ChatThread->>LoginModal: open(with providers)
LoginModal->>OAuthProvider: redirect user for auth
OAuthProvider-->>ChatThread: redirect back (authenticated)
ChatThread->>SessionStorage: retrieve pending message
ChatThread->>ChatThread: restore UI message
ChatThread->>ChatThread: createNewChatThread (authenticated) / send
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 |
Contributor
There was a problem hiding this comment.
we can remove this from the landing page now, right?
Contributor
Author
There was a problem hiding this comment.
asked claude to refactor the guard logic to be down in the actual chat component
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 `@packages/web/src/features/chat/components/chatThread/chatThread.tsx`:
- Around line 219-226: The restore logic currently marks
hasRestoredPendingMessage and removes PENDING_MESSAGE_STORAGE_KEY before
validating the stored payload and chatId, and it rehydrates search scopes from
current selectedSearchScopes instead of the saved draft; change the flow in the
restore functions that reference PENDING_MESSAGE_STORAGE_KEY (including the
blocks around hasRestoredPendingMessage and the later restore paths at the
228-238 and 335-337 areas) to: read and JSON.parse the stored payload, validate
stored.chatId matches the currently selected chat id (or the chat being opened),
and only when the chatId matches apply the draft into your draft state
(including restoring saved selectedSearchScopes from the payload), then set
hasRestoredPendingMessage.current = true and finally call
sessionStorage.removeItem(PENDING_MESSAGE_STORAGE_KEY); if it does not match, do
not clear storage or mark restored so the draft survives for the correct chat.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.mdpackages/web/src/app/[domain]/askgh/[owner]/[repo]/components/landingPage.tsxpackages/web/src/app/[domain]/chat/[id]/components/chatThreadPanel.tsxpackages/web/src/app/[domain]/chat/[id]/page.tsxpackages/web/src/app/components/loginModal.tsxpackages/web/src/features/chat/components/chatThread/chatThread.tsxpackages/web/src/features/git/listCommitsApi.test.ts
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
msukkari
force-pushed
the
michael/askgh-duplicate-login-wall-SOU-568
branch
from
314b573 to
825781f
Compare
…rror The test was failing because importing utils.ts transitively pulled in server-side environment variables via createLogger, which crashes in the jsdom test environment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/web/src/features/chat/components/chatThread/chatThread.tsx (1)
211-241: Pending message is lost if user lands on a different chat after OAuth redirect.Lines 222–223 clear
sessionStorageand mark restore as done before thestoredChatId !== chatIdcheck on line 229. If the redirect lands on a different chat first, the draft is irreversibly deleted. Move the cleanup to after a successful match and send.Additionally,
selectedSearchScopeson line 235 uses the current runtime value, not the scopes active when the message was drafted. Consider persisting and restoring them alongsidechildren.Proposed fix
- hasRestoredPendingMessage.current = true; - sessionStorage.removeItem(PENDING_MESSAGE_STORAGE_KEY); - try { - const { chatId: storedChatId, children } = JSON.parse(stored) as { chatId: string; children: Descendant[] }; + const { chatId: storedChatId, children, selectedSearchScopes: storedScopes } = + JSON.parse(stored) as { chatId: string; children: Descendant[]; selectedSearchScopes?: SearchScope[] }; // Only restore if we're on the same chat that stored the pending message if (storedChatId !== chatId) { return; } const text = slateContentToString(children); const mentions = getAllMentionElements(children); - const message = createUIMessage(text, mentions.map(({ data }) => data), selectedSearchScopes); + const message = createUIMessage(text, mentions.map(({ data }) => data), storedScopes ?? selectedSearchScopes); sendMessage(message); setIsAutoScrollEnabled(true); + hasRestoredPendingMessage.current = true; + sessionStorage.removeItem(PENDING_MESSAGE_STORAGE_KEY); } catch (error) { + hasRestoredPendingMessage.current = true; + sessionStorage.removeItem(PENDING_MESSAGE_STORAGE_KEY); console.error('Failed to restore pending message:', error); }And in the storage call (line 335):
-sessionStorage.setItem(PENDING_MESSAGE_STORAGE_KEY, JSON.stringify({ chatId, children })); +sessionStorage.setItem(PENDING_MESSAGE_STORAGE_KEY, JSON.stringify({ chatId, children, selectedSearchScopes }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx` around lines 211 - 241, The restore logic clears sessionStorage and marks hasRestoredPendingMessage.current=true before verifying the stored chat matches the current chat and before sending, which causes drafts to be lost if the user lands on a different chat; move sessionStorage.removeItem(PENDING_MESSAGE_STORAGE_KEY) and setting hasRestoredPendingMessage.current=true to after the storedChatId === chatId check and after sendMessage succeeds. Also persist the search scopes with the stored payload and, when restoring, read the saved scopes instead of using the current selectedSearchScopes (update the JSON shape saved to PENDING_MESSAGE_STORAGE_KEY and use those restored scopes when calling createUIMessage); adjust slateContentToString, getAllMentionElements, createUIMessage, and sendMessage usage accordingly.
🧹 Nitpick comments (2)
packages/web/src/features/chat/components/chatThread/chatThread.tsx (2)
330-351: Server call on every unauthenticated submit — consider caching the result.
getAskGhLoginWallData()is a server action called on every submit attempt by an unauthenticated user. Since the result (feature flag + providers) is unlikely to change during a session, consider fetching it once (e.g., on mount or on first submit) and caching in state, rather than making a round-trip on every keystroke→submit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx` around lines 330 - 351, The onSubmit handler calls getAskGhLoginWallData() on every unauthenticated submit; cache that result in component state instead (e.g., add askGhLoginWallData / askGhLoginLoaded state) and populate it either on mount (useEffect) or on first unauthenticated submit, then read from that cached state in onSubmit (replace direct getAskGhLoginWallData() call). Update onSubmit to only call the server when cached data is missing, setLoginWallProviders from the cached response, and ensure dependencies (sendMessage, selectedSearchScopes, isAuthenticated, captureEvent, chatId, askGhLoginWallData) are kept correct to avoid stale closures.
503-509:callbackUrlfallback is fine for a client component, but the empty-string fallback is a silent no-op.Since this is a
'use client'component andLoginModalonly renders whenisLoginModalOpenis true (which requires user interaction),windowwill always be defined here. Thetypeof window !== 'undefined'guard is unnecessary but harmless. However, if it ever did fall through to'', the OAuth redirect would break silently. A minor nit — you could simplify to justwindow.location.href.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx` around lines 503 - 509, The callbackUrl currently uses a typeof window check that can fall back to an empty string; update the LoginModal invocation in chatThread.tsx so callbackUrl is set directly to window.location.href (since this is a 'use client' component and LoginModal only opens after user interaction), i.e., replace the ternary expression passed to the callbackUrl prop for LoginModal (the code referencing isLoginModalOpen/loginWallProviders) with window.location.href to avoid a silent empty-string fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx`:
- Around line 211-241: The restore logic clears sessionStorage and marks
hasRestoredPendingMessage.current=true before verifying the stored chat matches
the current chat and before sending, which causes drafts to be lost if the user
lands on a different chat; move
sessionStorage.removeItem(PENDING_MESSAGE_STORAGE_KEY) and setting
hasRestoredPendingMessage.current=true to after the storedChatId === chatId
check and after sendMessage succeeds. Also persist the search scopes with the
stored payload and, when restoring, read the saved scopes instead of using the
current selectedSearchScopes (update the JSON shape saved to
PENDING_MESSAGE_STORAGE_KEY and use those restored scopes when calling
createUIMessage); adjust slateContentToString, getAllMentionElements,
createUIMessage, and sendMessage usage accordingly.
---
Nitpick comments:
In `@packages/web/src/features/chat/components/chatThread/chatThread.tsx`:
- Around line 330-351: The onSubmit handler calls getAskGhLoginWallData() on
every unauthenticated submit; cache that result in component state instead
(e.g., add askGhLoginWallData / askGhLoginLoaded state) and populate it either
on mount (useEffect) or on first unauthenticated submit, then read from that
cached state in onSubmit (replace direct getAskGhLoginWallData() call). Update
onSubmit to only call the server when cached data is missing,
setLoginWallProviders from the cached response, and ensure dependencies
(sendMessage, selectedSearchScopes, isAuthenticated, captureEvent, chatId,
askGhLoginWallData) are kept correct to avoid stale closures.
- Around line 503-509: The callbackUrl currently uses a typeof window check that
can fall back to an empty string; update the LoginModal invocation in
chatThread.tsx so callbackUrl is set directly to window.location.href (since
this is a 'use client' component and LoginModal only opens after user
interaction), i.e., replace the ternary expression passed to the callbackUrl
prop for LoginModal (the code referencing isLoginModalOpen/loginWallProviders)
with window.location.href to avoid a silent empty-string fallback.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.mdpackages/web/src/app/[domain]/chat/[id]/page.tsxpackages/web/src/features/chat/actions.tspackages/web/src/features/chat/components/chatThread/chatThread.tsxpackages/web/src/features/git/utils.test.ts
💤 Files with no reviewable changes (1)
- packages/web/src/app/[domain]/chat/[id]/page.tsx
Centralizes the login wall logic in the hook so all chat creation entry points are guarded automatically. This adds the login wall to the general /chat landing page and removes the duplicate manual logic from the askgh landing page. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
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)
packages/web/src/features/chat/useCreateNewChatThread.ts (1)
33-47:⚠️ Potential issue | 🟠 MajorGuard async failures to avoid unhandled rejections and stuck UX state.
Line 40 (
createChat) and Line 62 (getAskGhLoginWallData) can throw; they are not wrapped. Also Line 71 and Line 93 invokedoCreateChatwithout controlled error propagation. This can cause unhandled rejections and inconsistent loading/error behavior.💡 Proposed fix
@@ - const doCreateChat = useCallback(async (children: Descendant[], selectedSearchScopes: SearchScope[]) => { + const doCreateChat = useCallback(async (children: Descendant[], selectedSearchScopes: SearchScope[]) => { const text = slateContentToString(children); const mentions = getAllMentionElements(children); @@ - setIsLoading(true); - const response = await createChat(); + setIsLoading(true); + let response; + try { + response = await createChat(); + } catch (error) { + toast({ + description: "❌ Failed to create chat. Reason: unexpected error" + }); + setIsLoading(false); + return; + } @@ - const createNewChatThread = useCallback(async (children: Descendant[], selectedSearchScopes: SearchScope[]) => { + const createNewChatThread = useCallback(async (children: Descendant[], selectedSearchScopes: SearchScope[]) => { if (!isAuthenticated) { - const result = await getAskGhLoginWallData(); + const result = await getAskGhLoginWallData(); if (!isServiceError(result) && result.isEnabled) { @@ - doCreateChat(children, selectedSearchScopes); + await doCreateChat(children, selectedSearchScopes); }, [isAuthenticated, captureEvent, doCreateChat]); @@ - doCreateChat(children, selectedSearchScopes); + void doCreateChat(children, selectedSearchScopes); } catch (error) { console.error('Failed to restore pending message:', error); }#!/bin/bash # Inspect async calls and call-sites that should be handled explicitly. rg -n -C3 'await createChat\(|await getAskGhLoginWallData\(|doCreateChat\(' packages/web/src/features/chat/useCreateNewChatThread.tsAlso applies to: 60-72, 88-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/chat/useCreateNewChatThread.ts` around lines 33 - 47, The createChat and getAskGhLoginWallData calls (and the doCreateChat callers) can throw and are not guarded; wrap the async body of doCreateChat in try/catch/finally so any thrown error is caught, always call setIsLoading(false) in finally, and surface a controlled error to callers (either return a Result or rethrow after cleanup). Update call sites that invoke doCreateChat to await it and handle rejections (or inspect the returned Result) instead of letting unhandled promises propagate. Specifically modify the doCreateChat function around the createChat call and the code that calls getAskGhLoginWallData, ensure toast shows errors with the caught error message, and adjust callers that call doCreateChat so they handle the returned error state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/features/chat/useCreateNewChatThread.ts`:
- Around line 65-67: The pending payload saved with
sessionStorage.setItem(PENDING_NEW_CHAT_KEY, ...) should be removed when the
login modal is dismissed; update the login modal dismiss path (where
setLoginWallState is called with isOpen: false / the modal close handler) to
call sessionStorage.removeItem(PENDING_NEW_CHAT_KEY) so the stale payload can’t
be restored later, and ensure any local flags used for restoration (e.g.,
hasRestoredPendingMessage) remain consistent with this removal.
---
Outside diff comments:
In `@packages/web/src/features/chat/useCreateNewChatThread.ts`:
- Around line 33-47: The createChat and getAskGhLoginWallData calls (and the
doCreateChat callers) can throw and are not guarded; wrap the async body of
doCreateChat in try/catch/finally so any thrown error is caught, always call
setIsLoading(false) in finally, and surface a controlled error to callers
(either return a Result or rethrow after cleanup). Update call sites that invoke
doCreateChat to await it and handle rejections (or inspect the returned Result)
instead of letting unhandled promises propagate. Specifically modify the
doCreateChat function around the createChat call and the code that calls
getAskGhLoginWallData, ensure toast shows errors with the caught error message,
and adjust callers that call doCreateChat so they handle the returned error
state.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/web/src/app/[domain]/askgh/[owner]/[repo]/components/landingPage.tsxpackages/web/src/app/[domain]/askgh/[owner]/[repo]/page.tsxpackages/web/src/app/[domain]/chat/components/landingPageChatBox.tsxpackages/web/src/app/[domain]/chat/page.tsxpackages/web/src/features/chat/useCreateNewChatThread.ts
💤 Files with no reviewable changes (1)
- packages/web/src/app/[domain]/askgh/[owner]/[repo]/page.tsx
Comment on lines
+65
to
+67
| sessionStorage.setItem(PENDING_NEW_CHAT_KEY, JSON.stringify({ children, selectedSearchScopes })); | ||
| setLoginWallState({ isOpen: true, providers: result.providers }); | ||
| return; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/web/src/features/chat && wc -l useCreateNewChatThread.tsRepository: sourcebot-dev/sourcebot
Length of output: 96
🏁 Script executed:
cat -n packages/web/src/features/chat/useCreateNewChatThread.tsRepository: sourcebot-dev/sourcebot
Length of output: 5108
Clear the pending payload when users dismiss the login modal to prevent unintended restoration after later authentication.
When a user dismisses the login modal at line 105, the pending message stored at line 65 is not cleared. If they authenticate from another tab and return to the app, the restoration effect at lines 80–93 will replay the stale pending message. While the code does remove the stored message immediately after restoration (line 86) and uses hasRestoredPendingMessage to prevent repeated restorations within the same component lifecycle, a single unintended restoration can still occur if users dismiss the modal without logging in and then authenticate elsewhere.
💡 Proposed fix
- onOpenChange: (open: boolean) => setLoginWallState(prev => ({ ...prev, isOpen: open })),
+ onOpenChange: (open: boolean) => {
+ if (!open) {
+ sessionStorage.removeItem(PENDING_NEW_CHAT_KEY);
+ }
+ setLoginWallState(prev => ({ ...prev, isOpen: open }));
+ },Also applies to: 80–87, 89–93, 102–106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/web/src/features/chat/useCreateNewChatThread.ts` around lines 65 -
67, The pending payload saved with sessionStorage.setItem(PENDING_NEW_CHAT_KEY,
...) should be removed when the login modal is dismissed; update the login modal
dismiss path (where setLoginWallState is called with isOpen: false / the modal
close handler) to call sessionStorage.removeItem(PENDING_NEW_CHAT_KEY) so the
stale payload can’t be restored later, and ensure any local flags used for
restoration (e.g., hasRestoredPendingMessage) remain consistent with this
removal.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EXPERIMENT_ASK_GH_ENABLED=true, shows a login modal when an anonymous user tries to send a message on a duplicated chatLoginModalcomponent to a shared location (app/components/loginModal.tsx) for reuse across askgh landing page and chat threadvi.Mocktype errors inlistCommitsApi.test.tsTest plan
EXPERIMENT_ASK_GH_ENABLED=true, open a public chat as anonymous user → duplicate → try to send message → login modal should appear🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores