feat(connections-setup): add ConnectionsSetup component#2484
feat(connections-setup): add ConnectionsSetup component#2484viktormarinho wants to merge 21 commits intomainfrom
Conversation
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…slot-resolution tests Removes the unnecessary `await import()` in the `findMatchingConnections` test that caused TS errors (module-not-found and implicit-any on the map callback). `findMatchingConnections` is now included in the top-level static import alongside `resolveInitialPhase`. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…error handling, icon source
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
🧪 BenchmarkShould we run the Virtual MCP strategy benchmark for this PR? React with 👍 to run the benchmark.
Benchmark will run on the next push after you react. |
Release OptionsShould a new version be published when this PR is merged? React with an emoji to vote on the release type:
Current version: Deployment
|
There was a problem hiding this comment.
5 issues found across 14 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/mesh/src/web/components/connections-setup/use-connection-poller.ts">
<violation number="1" location="apps/mesh/src/web/components/connections-setup/use-connection-poller.ts:25">
P2: Reset the poll start time when the connectionId changes, otherwise a new connection can inherit the previous start time and time out immediately.</violation>
</file>
<file name="apps/mesh/src/web/components/connections-setup/connections-setup.tsx">
<violation number="1" location="apps/mesh/src/web/components/connections-setup/connections-setup.tsx:13">
P2: `handleSlotComplete` derives `next` from a potentially stale `completed` closure. If multiple slot completions happen quickly, later updates can clobber earlier ones and `onComplete` can receive incomplete data. Use the functional `setCompleted` updater to ensure you always merge from the latest state.</violation>
</file>
<file name="apps/mesh/src/web/components/connections-setup/slot-card.tsx">
<violation number="1" location="apps/mesh/src/web/components/connections-setup/slot-card.tsx:36">
P1: Side effect during render: `onComplete()` is called inside the render body. React's rules require render to be pure — calling parent callbacks that trigger state updates elsewhere is a side effect. Move this block into a `useEffect` that reacts to `poller.isActive`.</violation>
<violation number="2" location="apps/mesh/src/web/components/connections-setup/slot-card.tsx:50">
P2: If the poller times out without ever fetching connection data (`poller.connection` is null), `selectedConnection` remains null while the phase transitions to `auth-oauth`/`auth-token`. Both auth component render guards require `selectedConnection && ...`, so neither renders — leaving the user in a stuck state with no way to proceed or retry.</violation>
<violation number="3" location="apps/mesh/src/web/components/connections-setup/slot-card.tsx:54">
P1: Async network request fired during render: `isConnectionAuthenticated(...)` is an async side effect invoked in the render body with no cleanup. This violates React's purity rules for render and risks setting state on an unmounted component. Move this logic into a `useEffect` with a stale/abort guard (e.g., `let cancelled = false` in the effect, checked in `.then()`).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…state - Move isConnectionAuthenticated out of render body into useQuery (authCheckId state triggers the query after poller timeout/error) - Add completedIdRef guard so onComplete fires exactly once per connection even under concurrent-mode re-renders - Fix stuck UI when poller.connection is null on timeout: auth phases now fall back to authCheckId for the connection ID - Use functional setCompleted updater in ConnectionsSetup to avoid stale closure on rapid slot completions; move allDone/onComplete check to render with a wasAllDoneRef guard - Reset startTimeRef in useConnectionPoller whenever connectionId changes (not only when it was previously 0) to prevent a new connection inheriting a stale start time Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/mesh/src/web/components/connections-setup/connections-setup.tsx">
<violation number="1" location="apps/mesh/src/web/components/connections-setup/connections-setup.tsx:30">
P2: Calling `onComplete` during render introduces a side effect and can trigger render loops if the callback updates state. Move this into a `useEffect` that runs when `allDone` transitions to true.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const allDone = | ||
| allSlotIds.length > 0 && allSlotIds.every((id) => completed[id]); | ||
|
|
||
| if (allDone && !wasAllDoneRef.current) { |
There was a problem hiding this comment.
P2: Calling onComplete during render introduces a side effect and can trigger render loops if the callback updates state. Move this into a useEffect that runs when allDone transitions to true.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/web/components/connections-setup/connections-setup.tsx, line 30:
<comment>Calling `onComplete` during render introduces a side effect and can trigger render loops if the callback updates state. Move this into a `useEffect` that runs when `allDone` transitions to true.</comment>
<file context>
@@ -9,22 +9,31 @@ export interface ConnectionsSetupProps {
+ const allDone =
+ allSlotIds.length > 0 && allSlotIds.every((id) => completed[id]);
+
+ if (allDone && !wasAllDoneRef.current) {
+ wasAllDoneRef.current = true;
+ onComplete(completed);
</file context>
…y/react
Sets up happy-dom + RTL infrastructure and adds component-level tests for
SlotCard and ConnectionsSetup alongside the existing pure-function tests.
Infrastructure:
- bunfig.toml: two-stage preload (dom registration before RTL import)
- apps/mesh/test-setup.ts: GlobalRegistrator.register({ url: ... })
- apps/mesh/test-setup-rtl.ts: expect.extend(jest-dom matchers) + afterEach cleanup
- src/types/bun-testing.d.ts: extends bun:test Matchers with jest-dom types
SlotCard tests (slot-card.test.tsx) — 20 tests:
- All phase transitions: loading, error, install, polling, done, picker
- install → polling → done (calls onComplete once, guarded against re-render duplication)
- polling timeout → auth-oauth / auth-token (including null-connection fix)
- OAuth/token → back to polling
- done → picker / install on Change; picker select active/inactive; Install fresh
ConnectionsSetup tests (connections-setup.test.tsx) — 5 tests:
- Renders one card per slot
- Does not call onComplete until all slots satisfy
- Calls onComplete with correct slotId → connectionId map
- Does not call onComplete twice on re-renders
- Calls onComplete again after reset + re-completion
Mocking strategy (Option A): hooks (useSlotResolution, useConnectionPoller) and
child components are stubbed so tests cover rendering and state-machine wiring
without network calls. connections-setup.test.tsx deliberately does NOT mock
./slot-card to avoid cross-file module registry contamination in Bun 1.3.5.
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The previous state machine went straight to "done" when the poller saw status === "active". This was wrong: many connections (Gmail, Google Docs, etc.) become transport-active immediately after creation even without OAuth — the MCP server responds 200 because it's reachable, but the connection still needs an OAuth token to actually function. The fix: onPollerActive now queues an auth check (exactly like onPollerTimeout) instead of immediately marking the slot done. The auth check result then decides: active + supportsOAuth: true → auth-oauth (connection up, OAuth needed) active + supportsOAuth: false → done (connection up, no auth needed) timeout + supportsOAuth: true → auth-oauth (same) timeout + supportsOAuth: false → auth-token (not working, needs bearer token) onAuthStatus now takes a required `source: "active" | "timeout"` parameter. Tests and component updated to match. Also removes the DOM component tests added in the previous commit — they were mocking the hooks so they couldn't catch this class of real-world bug. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…401+WWW-Auth isConnectionAuthenticated() only sets supportsOAuth:true when the server returns a 401 with a WWW-Authenticate header. Many OAuth services (e.g. Gmail) respond 200 to the MCP initialize call even without a token — so supportsOAuth was always false and the slot incorrectly transitioned to "done". The fix: replace onAuthStatus(supportsOAuth, source) with resolveAuthPhase() which checks the connection's own oauth_config field alongside hasOAuthToken: if (connection.oauth_config && !hasOAuthToken) → auth-oauth if (!isAuthenticated && supportsOAuth) → auth-oauth (explicit 401 challenge) if (source === "active" && isAuthenticated) → done otherwise → auth-token Tests updated to cover the Gmail case explicitly: active + oauth_config + no token → auth-oauth ✓ active + oauth_config + has token → done ✓ active + no oauth_config → done ✓ timeout + no oauth_config → auth-token ✓ any + 401+WWW-Auth → auth-oauth ✓ Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/mesh/src/web/components/connections-setup/slot-card-transitions.ts">
<violation number="1" location="apps/mesh/src/web/components/connections-setup/slot-card-transitions.ts:78">
P2: `resolveAuthPhase` returns `auth-token` on timeout even when the auth check reports `isAuthenticated: true`, which represents a working connection. This will incorrectly prompt users for token auth after a late but successful auth check. Treat any `isAuthenticated` result as `done` regardless of timeout source.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| (!authStatus.isAuthenticated && authStatus.supportsOAuth); | ||
|
|
||
| if (needsOAuth) return "auth-oauth"; | ||
| if (source === "active" && authStatus.isAuthenticated) return "done"; |
There was a problem hiding this comment.
P2: resolveAuthPhase returns auth-token on timeout even when the auth check reports isAuthenticated: true, which represents a working connection. This will incorrectly prompt users for token auth after a late but successful auth check. Treat any isAuthenticated result as done regardless of timeout source.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/web/components/connections-setup/slot-card-transitions.ts, line 78:
<comment>`resolveAuthPhase` returns `auth-token` on timeout even when the auth check reports `isAuthenticated: true`, which represents a working connection. This will incorrectly prompt users for token auth after a late but successful auth check. Treat any `isAuthenticated` result as `done` regardless of timeout source.</comment>
<file context>
@@ -48,22 +48,35 @@ export function onPollerTimeout(
+ (!authStatus.isAuthenticated && authStatus.supportsOAuth);
+
+ if (needsOAuth) return "auth-oauth";
+ if (source === "active" && authStatus.isAuthenticated) return "done";
+ return "auth-token";
}
</file context>
| if (source === "active" && authStatus.isAuthenticated) return "done"; | |
| if (authStatus.isAuthenticated) return "done"; |
The auth check (isConnectionAuthenticated) was configured with staleTime:Infinity, meaning the very first result was cached forever. When the query fired immediately after connection creation the proxy might return 200 (not yet ready to enforce OAuth), so the cache was permanently poisoned with supportsOAuth:false and the auth phase was never shown. The connection detail page uses useSuspenseQuery with no staleTime (defaults to 0) so it always gets fresh data — which is why it correctly detected OAuth was needed. The fix: drop staleTime:Infinity so the query behaves like every other auth-status check in the codebase. The existing cache invalidation in slot-auth-oauth.tsx and slot-auth-token.tsx still ensures a fresh check after each auth step. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Summary
Adds a
ConnectionsSetupcomponent — a context-agnostic React component that guides users through installing, authenticating, and configuring a declared set of MCP connections. Designed for onboarding flows (project templates) and marketplace agent installs.onComplete(Record<slotId, connectionId>)when all slots are satisfiedUsage
What's included
slot-resolution.ts— pure phase determination logic (5 unit tests)use-slot-resolution.ts— fetches registry item + checks existing connections bymetadata.registry_item_iduse-connection-poller.ts— pollsCOLLECTION_CONNECTIONS_GETevery 2s untilstatus === "active"or 15s timeoutslot-done.tsx— collapsed done cardslot-install-form.tsx— install form pre-filled viaextractConnectionDataslot-auth-oauth.tsx— OAuth authorize flow (mirrors connection detail page)slot-auth-token.tsx— API token input formslot-card.tsx— per-slot state machine: loading → picker/install → polling → auth-oauth/auth-token → doneconnections-setup.tsx— root componentKEYS.registryItem,KEYS.connectionPollTest plan
bun test apps/mesh/src/web/components/connections-setup/— 5 passingbun run check— cleanbun run lint— 0 warnings, 0 errors🤖 Generated with Claude Code
Summary by cubic
Adds a context-agnostic ConnectionsSetup component to install, authenticate (OAuth or token), and verify required MCP connections. Integrates a Quick Setup dialog and ensures fresh auth checks to avoid marking connections as done too early.
New Features
Bug Fixes
Written for commit 5e4a3b6. Summary will update on new commits.