fix: missing connectors section #5387

Weves · 2025-09-10T19:02:19Z

Description

^

How Has This Been Tested?

[Describe the tests you ran to verify your changes]

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

This PR should be backported (make sure to check that the backport attempt succeeds)
[Optional] Override Linear Check

vercel · 2025-09-10T19:02:24Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
internal-search	Ready	Preview	Comment	Sep 11, 2025 2:29am

greptile-apps

Greptile Summary

This PR fixes a missing connectors section in the user settings modal by implementing federated OAuth connector support across multiple components. The changes add the ability for users to view and manage federated connectors (like Slack OAuth integrations) in their user settings, regardless of which page they access the settings from.

The core fix involves three main components:

SidebarWrapper.tsx - Adds the useFederatedOAuthStatus hook and passes federated connector data (federatedConnectors and refetchFederatedConnectors) along with ccPairs to the UserSettingsModal. This ensures the connectors section is visible when accessing settings from the assistants page.
ChatPage.tsx - Similarly integrates the federated OAuth status hook to provide complete connector information to the UserSettingsModal when accessed from the chat interface.
Comprehensive testing infrastructure - Adds a new end-to-end test (federated_slack.spec.ts) that validates the complete federated Slack connector workflow, from admin creation through user OAuth interaction to settings visibility. The test ensures that federated connectors properly appear in user settings after admin configuration.

The implementation follows existing patterns in the codebase where the UserSettingsModal conditionally shows a 'Connectors' tab based on the hasConnectors variable, which depends on both regular connectors (ccPairs) and federated connectors being provided as props. Without these changes, users would experience inconsistent behavior where the connectors section might be missing depending on their entry point to the settings modal.

Additionally, the PR includes necessary CI/CD updates to support federated connector testing by adding Slack OAuth credentials (SLACK_CLIENT_ID and SLACK_CLIENT_SECRET) to the Playwright test environment, enabling comprehensive end-to-end testing of the OAuth flow.

Confidence score: 4/5

This PR addresses a clear user interface inconsistency with well-structured changes that follow existing patterns in the codebase
Score reflects solid implementation approach but some concerns about test reliability due to hardcoded timeouts and potentially fragile selectors in the new e2e test
Pay close attention to web/tests/e2e/connectors/federated_slack.spec.ts for potential test stability issues with environment variables and timing-dependent assertions

_{5 files reviewed, 1 comment}

_{Edit Code Review Bot Settings | Greptile}

web/tests/e2e/connectors/federated_slack.spec.ts

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

cubic-dev-ai

8 issues found across 5 files

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

cubic-dev-ai · 2025-09-10T19:20:32Z

.github/workflows/pr-playwright-tests.yml

+
+  # for federated slack tests
+  SLACK_CLIENT_ID: ${{ secrets.SLACK_CLIENT_ID }}
+  SLACK_CLIENT_SECRET: ${{ secrets.SLACK_CLIENT_SECRET }}


Secrets are defined at workflow-level env, exposing them to every step and action; scope them to the Playwright test step to minimize exposure.

Prompt for AI agents

Address the following comment on .github/workflows/pr-playwright-tests.yml at line 15: <comment>Secrets are defined at workflow-level env, exposing them to every step and action; scope them to the Playwright test step to minimize exposure.</comment> <file context> @@ -9,6 +9,11 @@ env: + + # for federated slack tests + SLACK_CLIENT_ID: ${{ secrets.SLACK_CLIENT_ID }} + SLACK_CLIENT_SECRET: ${{ secrets.SLACK_CLIENT_SECRET }} + MOCK_LLM_RESPONSE: true </file context>

cubic-dev-ai · 2025-09-10T19:20:33Z

web/admin2_auth.json

    {
      "name": "fastapiusersauth",
-      "value": "h3qhacpHbE4_09HcOLlVW4lSee48m1UbjYTUiKYwNiw",
+      "value": "AAdfmA4NT0BXdlg80K3xVuLGVPPPqH7JfzVk82otIw0",


Avoid committing a real authentication cookie/token value; rely on global-setup to generate storage state at runtime or store a placeholder to prevent leaking credentials.

Prompt for AI agents

Address the following comment on web/admin2_auth.json at line 5: <comment>Avoid committing a real authentication cookie/token value; rely on global-setup to generate storage state at runtime or store a placeholder to prevent leaking credentials.</comment> <file context> @@ -2,10 +2,10 @@ { "name": "fastapiusersauth", - "value": "h3qhacpHbE4_09HcOLlVW4lSee48m1UbjYTUiKYwNiw", + "value": "AAdfmA4NT0BXdlg80K3xVuLGVPPPqH7JfzVk82otIw0", "domain": "localhost", "path": "/", </file context>

Suggested change

"value": "AAdfmA4NT0BXdlg80K3xVuLGVPPPqH7JfzVk82otIw0",

"value": "<redacted>",

cubic-dev-ai · 2025-09-10T19:20:33Z

web/src/app/chat/components/ChatPage.tsx

+  const {
+    connectors: federatedConnectorOAuthStatus,
+    refetch: refetchFederatedConnectors,
+  } = useFederatedOAuthStatus();


Redundant fetch: useFederatedOAuthStatus is invoked here and again inside FederatedOAuthModal, leading to duplicate network requests. Consider lifting the hook to ChatPage and passing data to the modal, or memoizing/centralizing the fetch.

Prompt for AI agents

Address the following comment on web/src/app/chat/components/ChatPage.tsx at line 168: <comment>Redundant fetch: useFederatedOAuthStatus is invoked here and again inside FederatedOAuthModal, leading to duplicate network requests. Consider lifting the hook to ChatPage and passing data to the modal, or memoizing/centralizing the fetch.</comment> <file context> @@ -161,6 +162,10 @@ export function ChatPage({ + const { + connectors: federatedConnectorOAuthStatus, + refetch: refetchFederatedConnectors, + } = useFederatedOAuthStatus(); const { user, isAdmin } = useUser(); </file context>

cubic-dev-ai · 2025-09-10T19:20:33Z

web/tests/e2e/connectors/federated_slack.spec.ts

+
+test.use({ storageState: "admin_auth.json" });
+
+const SLACK_CLIENT_ID = process.env.SLACK_CLIENT_ID || "";


Fail fast or skip the test if SLACK_CLIENT_ID is not set; defaulting to "" silently misconfigures the flow.

Prompt for AI agents

Address the following comment on web/tests/e2e/connectors/federated_slack.spec.ts at line 7: <comment>Fail fast or skip the test if SLACK_CLIENT_ID is not set; defaulting to "" silently misconfigures the flow.</comment> <file context> @@ -0,0 +1,108 @@ + +test.use({ storageState: "admin_auth.json" }); + +const SLACK_CLIENT_ID = process.env.SLACK_CLIENT_ID || ""; +const SLACK_CLIENT_SECRET = process.env.SLACK_CLIENT_SECRET || ""; + </file context>

cubic-dev-ai · 2025-09-10T19:20:33Z

web/tests/e2e/connectors/federated_slack.spec.ts

+
+async function navigateToUserSettings(page: Page) {
+  // Wait for any existing modals to close
+  await page.waitForTimeout(1000);


Avoid fixed delay; wait for a specific element/state signaling the UI is ready.

Prompt for AI agents

Address the following comment on web/tests/e2e/connectors/federated_slack.spec.ts at line 35: <comment>Avoid fixed delay; wait for a specific element/state signaling the UI is ready.</comment> <file context> @@ -0,0 +1,108 @@ + +async function navigateToUserSettings(page: Page) { + // Wait for any existing modals to close + await page.waitForTimeout(1000); + + // Wait for potential modal backdrop to disappear </file context>

cubic-dev-ai · 2025-09-10T19:20:33Z

web/tests/e2e/connectors/federated_slack.spec.ts

+  await createOrUpdateButton.click();
+
+  // Wait for success message or redirect
+  await page.waitForTimeout(2000);


Fixed wait introduces flakiness; wait for a deterministic condition (e.g., success UI, navigation, or network idle) instead.

Prompt for AI agents

Address the following comment on web/tests/e2e/connectors/federated_slack.spec.ts at line 30: <comment>Fixed wait introduces flakiness; wait for a deterministic condition (e.g., success UI, navigation, or network idle) instead.</comment> <file context> @@ -0,0 +1,108 @@ + await createOrUpdateButton.click(); + + // Wait for success message or redirect + await page.waitForTimeout(2000); +} + </file context>

cubic-dev-ai · 2025-09-10T19:20:33Z

web/tests/e2e/connectors/federated_slack.spec.ts

+  await page.waitForLoadState("networkidle");
+
+  // Click on Slack connector tile (specifically the one with "Logo Slack" text, not "Slack Bots")
+  await page.getByRole("link", { name: "Logo Slack" }).first().click();


Selector depends on icon alt text; prefer a text-based matcher that remains stable and avoids coupling to alt values.

Prompt for AI agents

Address the following comment on web/tests/e2e/connectors/federated_slack.spec.ts at line 16: <comment>Selector depends on icon alt text; prefer a text-based matcher that remains stable and avoids coupling to alt values.</comment> <file context> @@ -0,0 +1,108 @@ + await page.waitForLoadState("networkidle"); + + // Click on Slack connector tile (specifically the one with "Logo Slack" text, not "Slack Bots") + await page.getByRole("link", { name: "Logo Slack" }).first().click(); + await page.waitForLoadState("networkidle"); + </file context>

cubic-dev-ai · 2025-09-10T19:20:33Z

web/tests/e2e/connectors/federated_slack.spec.ts

+
+  // Check if the OAuth modal appears
+  await expect(
+    page.getByText(/improve answer quality by letting/i)


Assertion checks for text that does not exist in the OAuth modal; use a stable element like the "Skip for now" button to confirm the modal is shown.

Prompt for AI agents

Address the following comment on web/tests/e2e/connectors/federated_slack.spec.ts at line 85: <comment>Assertion checks for text that does not exist in the OAuth modal; use a stable element like the "Skip for now" button to confirm the modal is shown.</comment> <file context> @@ -0,0 +1,108 @@ + + // Check if the OAuth modal appears + await expect( + page.getByText(/improve answer quality by letting/i) + ).toBeVisible({ timeout: 10000 }); + await expect(page.getByText(/slack/i)).toBeVisible(); </file context>

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Weves added 2 commits September 10, 2025 12:00

Fix Connectors section in User Settings

727a105

Add to acitons

6a2dc4e

Weves requested a review from a team as a code owner September 10, 2025 19:02

Cleanup

06f0e4e

greptile-apps bot reviewed Sep 10, 2025

View reviewed changes

web/tests/e2e/connectors/federated_slack.spec.ts Outdated Show resolved Hide resolved

Update web/tests/e2e/connectors/federated_slack.spec.ts

ab00050

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

vercel bot deployed to Preview September 10, 2025 19:08 View deployment

justin-tahara approved these changes Sep 10, 2025

View reviewed changes

cubic-dev-ai bot reviewed Sep 10, 2025

View reviewed changes

jessicasingh7 approved these changes Sep 10, 2025

View reviewed changes

Cleanup federated slack connector

2f9ca82

vercel bot deployed to Preview September 10, 2025 20:51 View deployment

Fix

cbd078a

vercel bot deployed to Preview September 10, 2025 21:25 View deployment

Improve test

53ba481

Weves merged commit 6340c51 into main Sep 11, 2025
17 of 20 checks passed

Weves deleted the fix-missing-connectors-section branch September 11, 2025 02:28

vercel bot deployed to Preview September 11, 2025 02:29 View deployment

blacksmith-sh bot deleted a comment from Weves Sep 11, 2025

AnkitTukatek pushed a commit to TukaTek/onyx that referenced this pull request Sep 23, 2025

fix: missing connectors section (onyx-dot-app#5387)

d9d52f2

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: missing connectors section #5387

fix: missing connectors section #5387

Uh oh!

Weves commented Sep 10, 2025 •

edited

Loading

Uh oh!

vercel bot commented Sep 10, 2025 •

edited

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

	"value": "AAdfmA4NT0BXdlg80K3xVuLGVPPPqH7JfzVk82otIw0",
	"value": "<redacted>",


		test.use({ storageState: "admin_auth.json" });

		const SLACK_CLIENT_ID = process.env.SLACK_CLIENT_ID \|\| "";

fix: missing connectors section #5387

fix: missing connectors section #5387

Uh oh!

Conversation

Weves commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Backporting (check the box to trigger backport action)

Uh oh!

vercel bot commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Summary

Confidence score: 4/5

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Weves commented Sep 10, 2025 •

edited

Loading

vercel bot commented Sep 10, 2025 •

edited

Loading

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading

cubic-dev-ai bot Sep 10, 2025 •

edited

Loading