fix(web): Prompt anonymous users to login if they want to use connectors in Ask - #1274

Merged
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors
Jun 4, 2026
Merged

fix(web): Prompt anonymous users to login if they want to use connectors in Ask#1274
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors

Conversation

@jsourcebot

@jsourcebotjsourcebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

https://linear.app/sourcebot/issue/SOU-1268/show-login-prompt-for-configuring-connectors

Summary by CodeRabbit

  • New Features

    • Toolbar and chat controls now adapt to authentication state, enabling appropriate connector actions and login prompts.
  • Improvements

    • Connectors menu shows a “Log in to use connectors” prompt for unauthenticated users instead of the full list.
    • Connector loading now uses smarter error handling and retry behavior that avoids retrying on authentication failures.

@coderabbitai

coderabbitaiBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Threads authentication state into chat toolbar components and gates ConnectorsMenu's MCP servers query on that state. ConnectorsMenu now treats authentication failures as non-retriable and conditionally shows a login prompt for unauthenticated users instead of the connectors list.

Changes

Authentication State Threading and ConnectorsMenu Gating

Layer / File(s)Summary
Authentication state prop threading
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx, packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx, packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx, packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
ChatBoxToolbarProps and ChatBoxPlusButtonProps gain isAuthenticated: boolean. The prop is destructured and forwarded through ChatBoxToolbar → ChatBoxPlusButton → ConnectorsMenu; three UI entry points now pass isAuthenticated.
ConnectorsMenu authentication-aware error handling and UI gating
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
Adds exported McpServersLoadError and shouldRetryMcpServersLoad to skip retries for ErrorCode.NOT_AUTHENTICATED and limit other retries. The MCP servers react-query is enabled only when isAuthenticated is true. UI computes shouldShowLoginConnectorItem to render a login item when appropriate.
Tests for retry logic
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
New tests verify shouldRetryMcpServersLoad behavior for authentication vs non-authentication failures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • brendan-kellam
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title accurately describes the main change: adding a login prompt for anonymous users attempting to use connectors in the Ask feature. It is specific, concise, and directly reflects the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jminnetian/fix-anon-connectors

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

This comment has been minimized.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx`:
- Around line 97-100: The current loginHref uses usePathname() which drops the
query string, so preserve the full current URL by appending the search/query
string when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef24737f-2b49-4dc0-b19d-0e837deb4ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 358d481 and f104e77.

📒 Files selected for processing (7)
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx
  • packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx
  • packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx

Comment on lines +97 to +100
const pathname = usePathname();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve the full current URL in callbackUrl.

usePathname() drops the current query string, so after login we redirect back to the route but not necessarily the same Ask state. Build loginHref from pathname + searchParams here, the same way handleConnect() preserves window.location.search for the OAuth return path.

Suggested fix
-import { usePathname, useRouter } from "next/navigation";+import { usePathname, useRouter, useSearchParams } from "next/navigation";
...
const router = useRouter();
const pathname = usePathname();
+ const searchParams = useSearchParams();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
- const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;+ const callbackUrl = `${pathname}${searchParams.size > 0 ? `?${searchParams.toString()}` : ''}`;+ const loginHref = `/login?callbackUrl=${encodeURIComponent(callbackUrl)}`;
🤖 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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx` around
lines 97 - 100, The current loginHref uses usePathname() which drops the query
string, so preserve the full current URL by appending the search/query string
when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.

@brendan-kellambrendan-kellam changed the title Prompt anonymous users to login if they want to use connectors in Askfix(web): Prompt anonymous users to login if they want to use connectors in AskJun 4, 2026
@jsourcebot
jsourcebot merged commit bcbb5e0 into mainJun 4, 2026
9 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Jun 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jsourcebot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(web): Prompt anonymous users to login if they want to use connectors in Ask - #1274

Merged
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors
Jun 4, 2026
Merged

fix(web): Prompt anonymous users to login if they want to use connectors in Ask#1274
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors

Conversation

@jsourcebot

@jsourcebotjsourcebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

https://linear.app/sourcebot/issue/SOU-1268/show-login-prompt-for-configuring-connectors

Summary by CodeRabbit

  • New Features

    • Toolbar and chat controls now adapt to authentication state, enabling appropriate connector actions and login prompts.
  • Improvements

    • Connectors menu shows a “Log in to use connectors” prompt for unauthenticated users instead of the full list.
    • Connector loading now uses smarter error handling and retry behavior that avoids retrying on authentication failures.

@coderabbitai

coderabbitaiBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Threads authentication state into chat toolbar components and gates ConnectorsMenu's MCP servers query on that state. ConnectorsMenu now treats authentication failures as non-retriable and conditionally shows a login prompt for unauthenticated users instead of the connectors list.

Changes

Authentication State Threading and ConnectorsMenu Gating

Layer / File(s)Summary
Authentication state prop threading
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx, packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx, packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx, packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
ChatBoxToolbarProps and ChatBoxPlusButtonProps gain isAuthenticated: boolean. The prop is destructured and forwarded through ChatBoxToolbar → ChatBoxPlusButton → ConnectorsMenu; three UI entry points now pass isAuthenticated.
ConnectorsMenu authentication-aware error handling and UI gating
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
Adds exported McpServersLoadError and shouldRetryMcpServersLoad to skip retries for ErrorCode.NOT_AUTHENTICATED and limit other retries. The MCP servers react-query is enabled only when isAuthenticated is true. UI computes shouldShowLoginConnectorItem to render a login item when appropriate.
Tests for retry logic
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
New tests verify shouldRetryMcpServersLoad behavior for authentication vs non-authentication failures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • brendan-kellam
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title accurately describes the main change: adding a login prompt for anonymous users attempting to use connectors in the Ask feature. It is specific, concise, and directly reflects the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jminnetian/fix-anon-connectors

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

This comment has been minimized.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx`:
- Around line 97-100: The current loginHref uses usePathname() which drops the
query string, so preserve the full current URL by appending the search/query
string when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef24737f-2b49-4dc0-b19d-0e837deb4ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 358d481 and f104e77.

📒 Files selected for processing (7)
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx
  • packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx
  • packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx

Comment on lines +97 to +100
const pathname = usePathname();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve the full current URL in callbackUrl.

usePathname() drops the current query string, so after login we redirect back to the route but not necessarily the same Ask state. Build loginHref from pathname + searchParams here, the same way handleConnect() preserves window.location.search for the OAuth return path.

Suggested fix
-import { usePathname, useRouter } from "next/navigation";+import { usePathname, useRouter, useSearchParams } from "next/navigation";
...
const router = useRouter();
const pathname = usePathname();
+ const searchParams = useSearchParams();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
- const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;+ const callbackUrl = `${pathname}${searchParams.size > 0 ? `?${searchParams.toString()}` : ''}`;+ const loginHref = `/login?callbackUrl=${encodeURIComponent(callbackUrl)}`;
🤖 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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx` around
lines 97 - 100, The current loginHref uses usePathname() which drops the query
string, so preserve the full current URL by appending the search/query string
when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.

@brendan-kellambrendan-kellam changed the title Prompt anonymous users to login if they want to use connectors in Askfix(web): Prompt anonymous users to login if they want to use connectors in AskJun 4, 2026
@jsourcebot
jsourcebot merged commit bcbb5e0 into mainJun 4, 2026
9 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Jun 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jsourcebot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(web): Prompt anonymous users to login if they want to use connectors in Ask - #1274

Merged
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors
Jun 4, 2026
Merged

fix(web): Prompt anonymous users to login if they want to use connectors in Ask#1274
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors

Conversation

@jsourcebot

@jsourcebotjsourcebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

https://linear.app/sourcebot/issue/SOU-1268/show-login-prompt-for-configuring-connectors

Summary by CodeRabbit

  • New Features

    • Toolbar and chat controls now adapt to authentication state, enabling appropriate connector actions and login prompts.
  • Improvements

    • Connectors menu shows a “Log in to use connectors” prompt for unauthenticated users instead of the full list.
    • Connector loading now uses smarter error handling and retry behavior that avoids retrying on authentication failures.

@coderabbitai

coderabbitaiBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Threads authentication state into chat toolbar components and gates ConnectorsMenu's MCP servers query on that state. ConnectorsMenu now treats authentication failures as non-retriable and conditionally shows a login prompt for unauthenticated users instead of the connectors list.

Changes

Authentication State Threading and ConnectorsMenu Gating

Layer / File(s)Summary
Authentication state prop threading
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx, packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx, packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx, packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
ChatBoxToolbarProps and ChatBoxPlusButtonProps gain isAuthenticated: boolean. The prop is destructured and forwarded through ChatBoxToolbar → ChatBoxPlusButton → ConnectorsMenu; three UI entry points now pass isAuthenticated.
ConnectorsMenu authentication-aware error handling and UI gating
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
Adds exported McpServersLoadError and shouldRetryMcpServersLoad to skip retries for ErrorCode.NOT_AUTHENTICATED and limit other retries. The MCP servers react-query is enabled only when isAuthenticated is true. UI computes shouldShowLoginConnectorItem to render a login item when appropriate.
Tests for retry logic
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
New tests verify shouldRetryMcpServersLoad behavior for authentication vs non-authentication failures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • brendan-kellam
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title accurately describes the main change: adding a login prompt for anonymous users attempting to use connectors in the Ask feature. It is specific, concise, and directly reflects the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jminnetian/fix-anon-connectors

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

This comment has been minimized.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx`:
- Around line 97-100: The current loginHref uses usePathname() which drops the
query string, so preserve the full current URL by appending the search/query
string when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef24737f-2b49-4dc0-b19d-0e837deb4ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 358d481 and f104e77.

📒 Files selected for processing (7)
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx
  • packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx
  • packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx

Comment on lines +97 to +100
const pathname = usePathname();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve the full current URL in callbackUrl.

usePathname() drops the current query string, so after login we redirect back to the route but not necessarily the same Ask state. Build loginHref from pathname + searchParams here, the same way handleConnect() preserves window.location.search for the OAuth return path.

Suggested fix
-import { usePathname, useRouter } from "next/navigation";+import { usePathname, useRouter, useSearchParams } from "next/navigation";
...
const router = useRouter();
const pathname = usePathname();
+ const searchParams = useSearchParams();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
- const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;+ const callbackUrl = `${pathname}${searchParams.size > 0 ? `?${searchParams.toString()}` : ''}`;+ const loginHref = `/login?callbackUrl=${encodeURIComponent(callbackUrl)}`;
🤖 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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx` around
lines 97 - 100, The current loginHref uses usePathname() which drops the query
string, so preserve the full current URL by appending the search/query string
when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.

@brendan-kellambrendan-kellam changed the title Prompt anonymous users to login if they want to use connectors in Askfix(web): Prompt anonymous users to login if they want to use connectors in AskJun 4, 2026
@jsourcebot
jsourcebot merged commit bcbb5e0 into mainJun 4, 2026
9 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Jun 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jsourcebot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(web): Prompt anonymous users to login if they want to use connectors in Ask - #1274

Merged
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors
Jun 4, 2026
Merged

fix(web): Prompt anonymous users to login if they want to use connectors in Ask#1274
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors

Conversation

@jsourcebot

@jsourcebotjsourcebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

https://linear.app/sourcebot/issue/SOU-1268/show-login-prompt-for-configuring-connectors

Summary by CodeRabbit

  • New Features

    • Toolbar and chat controls now adapt to authentication state, enabling appropriate connector actions and login prompts.
  • Improvements

    • Connectors menu shows a “Log in to use connectors” prompt for unauthenticated users instead of the full list.
    • Connector loading now uses smarter error handling and retry behavior that avoids retrying on authentication failures.

@coderabbitai

coderabbitaiBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Threads authentication state into chat toolbar components and gates ConnectorsMenu's MCP servers query on that state. ConnectorsMenu now treats authentication failures as non-retriable and conditionally shows a login prompt for unauthenticated users instead of the connectors list.

Changes

Authentication State Threading and ConnectorsMenu Gating

Layer / File(s)Summary
Authentication state prop threading
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx, packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx, packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx, packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
ChatBoxToolbarProps and ChatBoxPlusButtonProps gain isAuthenticated: boolean. The prop is destructured and forwarded through ChatBoxToolbar → ChatBoxPlusButton → ConnectorsMenu; three UI entry points now pass isAuthenticated.
ConnectorsMenu authentication-aware error handling and UI gating
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
Adds exported McpServersLoadError and shouldRetryMcpServersLoad to skip retries for ErrorCode.NOT_AUTHENTICATED and limit other retries. The MCP servers react-query is enabled only when isAuthenticated is true. UI computes shouldShowLoginConnectorItem to render a login item when appropriate.
Tests for retry logic
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
New tests verify shouldRetryMcpServersLoad behavior for authentication vs non-authentication failures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • brendan-kellam
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title accurately describes the main change: adding a login prompt for anonymous users attempting to use connectors in the Ask feature. It is specific, concise, and directly reflects the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jminnetian/fix-anon-connectors

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

This comment has been minimized.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx`:
- Around line 97-100: The current loginHref uses usePathname() which drops the
query string, so preserve the full current URL by appending the search/query
string when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef24737f-2b49-4dc0-b19d-0e837deb4ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 358d481 and f104e77.

📒 Files selected for processing (7)
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx
  • packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx
  • packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx

Comment on lines +97 to +100
const pathname = usePathname();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve the full current URL in callbackUrl.

usePathname() drops the current query string, so after login we redirect back to the route but not necessarily the same Ask state. Build loginHref from pathname + searchParams here, the same way handleConnect() preserves window.location.search for the OAuth return path.

Suggested fix
-import { usePathname, useRouter } from "next/navigation";+import { usePathname, useRouter, useSearchParams } from "next/navigation";
...
const router = useRouter();
const pathname = usePathname();
+ const searchParams = useSearchParams();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
- const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;+ const callbackUrl = `${pathname}${searchParams.size > 0 ? `?${searchParams.toString()}` : ''}`;+ const loginHref = `/login?callbackUrl=${encodeURIComponent(callbackUrl)}`;
🤖 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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx` around
lines 97 - 100, The current loginHref uses usePathname() which drops the query
string, so preserve the full current URL by appending the search/query string
when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.

@brendan-kellambrendan-kellam changed the title Prompt anonymous users to login if they want to use connectors in Askfix(web): Prompt anonymous users to login if they want to use connectors in AskJun 4, 2026
@jsourcebot
jsourcebot merged commit bcbb5e0 into mainJun 4, 2026
9 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Jun 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jsourcebot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(web): Prompt anonymous users to login if they want to use connectors in Ask - #1274

Merged
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors
Jun 4, 2026
Merged

fix(web): Prompt anonymous users to login if they want to use connectors in Ask#1274
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors

Conversation

@jsourcebot

@jsourcebotjsourcebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

https://linear.app/sourcebot/issue/SOU-1268/show-login-prompt-for-configuring-connectors

Summary by CodeRabbit

  • New Features

    • Toolbar and chat controls now adapt to authentication state, enabling appropriate connector actions and login prompts.
  • Improvements

    • Connectors menu shows a “Log in to use connectors” prompt for unauthenticated users instead of the full list.
    • Connector loading now uses smarter error handling and retry behavior that avoids retrying on authentication failures.

@coderabbitai

coderabbitaiBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Threads authentication state into chat toolbar components and gates ConnectorsMenu's MCP servers query on that state. ConnectorsMenu now treats authentication failures as non-retriable and conditionally shows a login prompt for unauthenticated users instead of the connectors list.

Changes

Authentication State Threading and ConnectorsMenu Gating

Layer / File(s)Summary
Authentication state prop threading
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx, packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx, packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx, packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
ChatBoxToolbarProps and ChatBoxPlusButtonProps gain isAuthenticated: boolean. The prop is destructured and forwarded through ChatBoxToolbar → ChatBoxPlusButton → ConnectorsMenu; three UI entry points now pass isAuthenticated.
ConnectorsMenu authentication-aware error handling and UI gating
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
Adds exported McpServersLoadError and shouldRetryMcpServersLoad to skip retries for ErrorCode.NOT_AUTHENTICATED and limit other retries. The MCP servers react-query is enabled only when isAuthenticated is true. UI computes shouldShowLoginConnectorItem to render a login item when appropriate.
Tests for retry logic
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
New tests verify shouldRetryMcpServersLoad behavior for authentication vs non-authentication failures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • brendan-kellam
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title accurately describes the main change: adding a login prompt for anonymous users attempting to use connectors in the Ask feature. It is specific, concise, and directly reflects the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jminnetian/fix-anon-connectors

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

This comment has been minimized.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx`:
- Around line 97-100: The current loginHref uses usePathname() which drops the
query string, so preserve the full current URL by appending the search/query
string when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef24737f-2b49-4dc0-b19d-0e837deb4ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 358d481 and f104e77.

📒 Files selected for processing (7)
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx
  • packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx
  • packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx

Comment on lines +97 to +100
const pathname = usePathname();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve the full current URL in callbackUrl.

usePathname() drops the current query string, so after login we redirect back to the route but not necessarily the same Ask state. Build loginHref from pathname + searchParams here, the same way handleConnect() preserves window.location.search for the OAuth return path.

Suggested fix
-import { usePathname, useRouter } from "next/navigation";+import { usePathname, useRouter, useSearchParams } from "next/navigation";
...
const router = useRouter();
const pathname = usePathname();
+ const searchParams = useSearchParams();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
- const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;+ const callbackUrl = `${pathname}${searchParams.size > 0 ? `?${searchParams.toString()}` : ''}`;+ const loginHref = `/login?callbackUrl=${encodeURIComponent(callbackUrl)}`;
🤖 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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx` around
lines 97 - 100, The current loginHref uses usePathname() which drops the query
string, so preserve the full current URL by appending the search/query string
when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.

@brendan-kellambrendan-kellam changed the title Prompt anonymous users to login if they want to use connectors in Askfix(web): Prompt anonymous users to login if they want to use connectors in AskJun 4, 2026
@jsourcebot
jsourcebot merged commit bcbb5e0 into mainJun 4, 2026
9 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Jun 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jsourcebot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(web): Prompt anonymous users to login if they want to use connectors in Ask - #1274

Merged
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors
Jun 4, 2026
Merged

fix(web): Prompt anonymous users to login if they want to use connectors in Ask#1274
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors

Conversation

@jsourcebot

@jsourcebotjsourcebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

https://linear.app/sourcebot/issue/SOU-1268/show-login-prompt-for-configuring-connectors

Summary by CodeRabbit

  • New Features

    • Toolbar and chat controls now adapt to authentication state, enabling appropriate connector actions and login prompts.
  • Improvements

    • Connectors menu shows a “Log in to use connectors” prompt for unauthenticated users instead of the full list.
    • Connector loading now uses smarter error handling and retry behavior that avoids retrying on authentication failures.

@coderabbitai

coderabbitaiBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Threads authentication state into chat toolbar components and gates ConnectorsMenu's MCP servers query on that state. ConnectorsMenu now treats authentication failures as non-retriable and conditionally shows a login prompt for unauthenticated users instead of the connectors list.

Changes

Authentication State Threading and ConnectorsMenu Gating

Layer / File(s)Summary
Authentication state prop threading
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx, packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx, packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx, packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
ChatBoxToolbarProps and ChatBoxPlusButtonProps gain isAuthenticated: boolean. The prop is destructured and forwarded through ChatBoxToolbar → ChatBoxPlusButton → ConnectorsMenu; three UI entry points now pass isAuthenticated.
ConnectorsMenu authentication-aware error handling and UI gating
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
Adds exported McpServersLoadError and shouldRetryMcpServersLoad to skip retries for ErrorCode.NOT_AUTHENTICATED and limit other retries. The MCP servers react-query is enabled only when isAuthenticated is true. UI computes shouldShowLoginConnectorItem to render a login item when appropriate.
Tests for retry logic
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
New tests verify shouldRetryMcpServersLoad behavior for authentication vs non-authentication failures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • brendan-kellam
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title accurately describes the main change: adding a login prompt for anonymous users attempting to use connectors in the Ask feature. It is specific, concise, and directly reflects the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jminnetian/fix-anon-connectors

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

This comment has been minimized.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx`:
- Around line 97-100: The current loginHref uses usePathname() which drops the
query string, so preserve the full current URL by appending the search/query
string when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef24737f-2b49-4dc0-b19d-0e837deb4ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 358d481 and f104e77.

📒 Files selected for processing (7)
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx
  • packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx
  • packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx

Comment on lines +97 to +100
const pathname = usePathname();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve the full current URL in callbackUrl.

usePathname() drops the current query string, so after login we redirect back to the route but not necessarily the same Ask state. Build loginHref from pathname + searchParams here, the same way handleConnect() preserves window.location.search for the OAuth return path.

Suggested fix
-import { usePathname, useRouter } from "next/navigation";+import { usePathname, useRouter, useSearchParams } from "next/navigation";
...
const router = useRouter();
const pathname = usePathname();
+ const searchParams = useSearchParams();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
- const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;+ const callbackUrl = `${pathname}${searchParams.size > 0 ? `?${searchParams.toString()}` : ''}`;+ const loginHref = `/login?callbackUrl=${encodeURIComponent(callbackUrl)}`;
🤖 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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx` around
lines 97 - 100, The current loginHref uses usePathname() which drops the query
string, so preserve the full current URL by appending the search/query string
when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.

@brendan-kellambrendan-kellam changed the title Prompt anonymous users to login if they want to use connectors in Askfix(web): Prompt anonymous users to login if they want to use connectors in AskJun 4, 2026
@jsourcebot
jsourcebot merged commit bcbb5e0 into mainJun 4, 2026
9 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Jun 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jsourcebot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(web): Prompt anonymous users to login if they want to use connectors in Ask - #1274

Merged
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors
Jun 4, 2026
Merged

fix(web): Prompt anonymous users to login if they want to use connectors in Ask#1274
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors

Conversation

@jsourcebot

@jsourcebotjsourcebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

https://linear.app/sourcebot/issue/SOU-1268/show-login-prompt-for-configuring-connectors

Summary by CodeRabbit

  • New Features

    • Toolbar and chat controls now adapt to authentication state, enabling appropriate connector actions and login prompts.
  • Improvements

    • Connectors menu shows a “Log in to use connectors” prompt for unauthenticated users instead of the full list.
    • Connector loading now uses smarter error handling and retry behavior that avoids retrying on authentication failures.

@coderabbitai

coderabbitaiBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Threads authentication state into chat toolbar components and gates ConnectorsMenu's MCP servers query on that state. ConnectorsMenu now treats authentication failures as non-retriable and conditionally shows a login prompt for unauthenticated users instead of the connectors list.

Changes

Authentication State Threading and ConnectorsMenu Gating

Layer / File(s)Summary
Authentication state prop threading
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx, packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx, packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx, packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
ChatBoxToolbarProps and ChatBoxPlusButtonProps gain isAuthenticated: boolean. The prop is destructured and forwarded through ChatBoxToolbar → ChatBoxPlusButton → ConnectorsMenu; three UI entry points now pass isAuthenticated.
ConnectorsMenu authentication-aware error handling and UI gating
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
Adds exported McpServersLoadError and shouldRetryMcpServersLoad to skip retries for ErrorCode.NOT_AUTHENTICATED and limit other retries. The MCP servers react-query is enabled only when isAuthenticated is true. UI computes shouldShowLoginConnectorItem to render a login item when appropriate.
Tests for retry logic
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
New tests verify shouldRetryMcpServersLoad behavior for authentication vs non-authentication failures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • brendan-kellam
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title accurately describes the main change: adding a login prompt for anonymous users attempting to use connectors in the Ask feature. It is specific, concise, and directly reflects the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jminnetian/fix-anon-connectors

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

This comment has been minimized.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx`:
- Around line 97-100: The current loginHref uses usePathname() which drops the
query string, so preserve the full current URL by appending the search/query
string when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef24737f-2b49-4dc0-b19d-0e837deb4ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 358d481 and f104e77.

📒 Files selected for processing (7)
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx
  • packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx
  • packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx

Comment on lines +97 to +100
const pathname = usePathname();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve the full current URL in callbackUrl.

usePathname() drops the current query string, so after login we redirect back to the route but not necessarily the same Ask state. Build loginHref from pathname + searchParams here, the same way handleConnect() preserves window.location.search for the OAuth return path.

Suggested fix
-import { usePathname, useRouter } from "next/navigation";+import { usePathname, useRouter, useSearchParams } from "next/navigation";
...
const router = useRouter();
const pathname = usePathname();
+ const searchParams = useSearchParams();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
- const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;+ const callbackUrl = `${pathname}${searchParams.size > 0 ? `?${searchParams.toString()}` : ''}`;+ const loginHref = `/login?callbackUrl=${encodeURIComponent(callbackUrl)}`;
🤖 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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx` around
lines 97 - 100, The current loginHref uses usePathname() which drops the query
string, so preserve the full current URL by appending the search/query string
when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.

@brendan-kellambrendan-kellam changed the title Prompt anonymous users to login if they want to use connectors in Askfix(web): Prompt anonymous users to login if they want to use connectors in AskJun 4, 2026
@jsourcebot
jsourcebot merged commit bcbb5e0 into mainJun 4, 2026
9 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Jun 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jsourcebot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(web): Prompt anonymous users to login if they want to use connectors in Ask - #1274

Merged
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors
Jun 4, 2026
Merged

fix(web): Prompt anonymous users to login if they want to use connectors in Ask#1274
jsourcebot merged 2 commits into
mainfrom
jminnetian/fix-anon-connectors

Conversation

@jsourcebot

@jsourcebotjsourcebot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

https://linear.app/sourcebot/issue/SOU-1268/show-login-prompt-for-configuring-connectors

Summary by CodeRabbit

  • New Features

    • Toolbar and chat controls now adapt to authentication state, enabling appropriate connector actions and login prompts.
  • Improvements

    • Connectors menu shows a “Log in to use connectors” prompt for unauthenticated users instead of the full list.
    • Connector loading now uses smarter error handling and retry behavior that avoids retrying on authentication failures.

@coderabbitai

coderabbitaiBot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Threads authentication state into chat toolbar components and gates ConnectorsMenu's MCP servers query on that state. ConnectorsMenu now treats authentication failures as non-retriable and conditionally shows a login prompt for unauthenticated users instead of the connectors list.

Changes

Authentication State Threading and ConnectorsMenu Gating

Layer / File(s)Summary
Authentication state prop threading
packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx, packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx, packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx, packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx, packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
ChatBoxToolbarProps and ChatBoxPlusButtonProps gain isAuthenticated: boolean. The prop is destructured and forwarded through ChatBoxToolbar → ChatBoxPlusButton → ConnectorsMenu; three UI entry points now pass isAuthenticated.
ConnectorsMenu authentication-aware error handling and UI gating
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
Adds exported McpServersLoadError and shouldRetryMcpServersLoad to skip retries for ErrorCode.NOT_AUTHENTICATED and limit other retries. The MCP servers react-query is enabled only when isAuthenticated is true. UI computes shouldShowLoginConnectorItem to render a login item when appropriate.
Tests for retry logic
packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
New tests verify shouldRetryMcpServersLoad behavior for authentication vs non-authentication failures.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • brendan-kellam
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title accurately describes the main change: adding a login prompt for anonymous users attempting to use connectors in the Ask feature. It is specific, concise, and directly reflects the primary objective of the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jminnetian/fix-anon-connectors

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

This comment has been minimized.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx`:
- Around line 97-100: The current loginHref uses usePathname() which drops the
query string, so preserve the full current URL by appending the search/query
string when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef24737f-2b49-4dc0-b19d-0e837deb4ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 358d481 and f104e77.

📒 Files selected for processing (7)
  • packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx
  • packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx
  • packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.test.ts
  • packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxPlusButton.tsx
  • packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx

Comment on lines +97 to +100
const pathname = usePathname();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preserve the full current URL in callbackUrl.

usePathname() drops the current query string, so after login we redirect back to the route but not necessarily the same Ask state. Build loginHref from pathname + searchParams here, the same way handleConnect() preserves window.location.search for the OAuth return path.

Suggested fix
-import { usePathname, useRouter } from "next/navigation";+import { usePathname, useRouter, useSearchParams } from "next/navigation";
...
const router = useRouter();
const pathname = usePathname();
+ const searchParams = useSearchParams();
const { toast } = useToast();
const isOwner = useRole() === OrgRole.OWNER;
- const loginHref = `/login?callbackUrl=${encodeURIComponent(pathname)}`;+ const callbackUrl = `${pathname}${searchParams.size > 0 ? `?${searchParams.toString()}` : ''}`;+ const loginHref = `/login?callbackUrl=${encodeURIComponent(callbackUrl)}`;
🤖 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 `@packages/web/src/ee/features/chat/mcp/components/connectorsMenu.tsx` around
lines 97 - 100, The current loginHref uses usePathname() which drops the query
string, so preserve the full current URL by appending the search/query string
when building loginHref (same approach as handleConnect which uses
window.location.search); update the loginHref construction in connectorsMenu.tsx
(where usePathname(), useToast(), useRole() are used and loginHref is defined)
to include the current search params (e.g., pathname + window.location.search or
via useSearchParams) before encodeURIComponent so the callbackUrl contains the
full route and query state.

@brendan-kellambrendan-kellam changed the title Prompt anonymous users to login if they want to use connectors in Askfix(web): Prompt anonymous users to login if they want to use connectors in AskJun 4, 2026
@jsourcebot
jsourcebot merged commit bcbb5e0 into mainJun 4, 2026
9 checks passed
@github-actionsgithub-actionsBot mentioned this pull request Jun 4, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jsourcebot