-
-
Notifications
You must be signed in to change notification settings - Fork 227
feat: unify local and hosted authorization paths #2028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
d42f31e
bbbc59a
765afdf
d86d744
356fc2a
dcf33d9
5c8cbba
ec3a1af
32118c4
50cb19f
bdbe583
8ffcfcc
937ed9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import { useEffect, useRef } from "react"; | ||
| import { useMutation } from "convex/react"; | ||
| import { | ||
| hasMigrationCompleted, | ||
| runLocalStateMigration, | ||
| } from "@/lib/local-state-migration"; | ||
| import { HOSTED_MODE } from "@/lib/config"; | ||
| import { useLogger } from "./use-logger"; | ||
|
|
||
| interface UseLocalStateMigrationOptions { | ||
| /** True when Convex auth has resolved (signed-in user OR guest). */ | ||
| isAuthenticated: boolean; | ||
| /** | ||
| * Optional org id to migrate into. Undefined defers to Convex's | ||
| * resolveProjectOrganizationId which falls back to the actor's default | ||
| * organization (provisioned by `users:ensureUser`). | ||
| */ | ||
| organizationId?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Runs the legacy-localStorage → Convex migration exactly once per install. | ||
| * | ||
| * Skips when: | ||
| * - HOSTED_MODE (hosted users never had localStorage state) | ||
| * - The migration flag is already set | ||
| * - Convex auth hasn't resolved | ||
| * - Migration is already in flight (prevents duplicate fires across renders) | ||
| */ | ||
| export function useLocalStateMigration({ | ||
| isAuthenticated, | ||
| organizationId, | ||
| }: UseLocalStateMigrationOptions): void { | ||
| const logger = useLogger("LocalStateMigration"); | ||
| const inFlightRef = useRef(false); | ||
| const doneRef = useRef(false); | ||
| const createProject = useMutation("projects:createProject" as any); | ||
|
|
||
| useEffect(() => { | ||
| if (HOSTED_MODE) return; | ||
| if (!isAuthenticated) return; | ||
| if (doneRef.current) return; | ||
| if (inFlightRef.current) return; | ||
| if (hasMigrationCompleted()) { | ||
| doneRef.current = true; | ||
|
Comment on lines
+165
to
+171
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In signed-in local sessions, this effect can run as soon as Convex auth and Useful? React with 👍 / 👎. |
||
| return; | ||
| } | ||
|
|
||
| inFlightRef.current = true; | ||
| runLocalStateMigration({ | ||
| createProject: createProject as any, | ||
| organizationId, | ||
| logger, | ||
| }) | ||
| .then((result) => { | ||
| if (result.ok) { | ||
| doneRef.current = true; | ||
| if (result.projectsMigrated > 0) { | ||
| logger.info("Local state migration completed", { | ||
| projectsMigrated: result.projectsMigrated, | ||
| }); | ||
| } | ||
| } else { | ||
| logger.warn("Local state migration partially failed; will retry", { | ||
| errors: result.errors, | ||
| }); | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| }) | ||
| .catch((error) => { | ||
| logger.error("Local state migration threw", { | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| }) | ||
| .finally(() => { | ||
| inFlightRef.current = false; | ||
| }); | ||
| }, [isAuthenticated, organizationId, createProject, logger]); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,6 +39,7 @@ import { HOSTED_MODE } from "@/lib/config"; | |
| import { | ||
| injectHostedServerMapping, | ||
| tryGetHostedServerDisplayName, | ||
| tryResolveProjectServer, | ||
| } from "@/lib/apis/web/context"; | ||
| import type { OAuthTestProfile } from "@/lib/oauth/profile"; | ||
| import { authFetch } from "@/lib/session-token"; | ||
|
|
@@ -733,6 +734,18 @@ export function useServerState({ | |
| const guardedTestConnection = useCallback( | ||
| async (serverConfig: MCPServerConfig, serverName: string) => { | ||
| assertClientConfigSynced(); | ||
| // Opt into the resolver path when both projectId and a Convex serverId | ||
| // are populated in the API context; otherwise fall back to legacy | ||
| // {serverConfig, serverId} so brand-new servers (not yet synced to | ||
| // Convex) keep working. The 2-arg call signature is preserved when no | ||
| // resolver context is available so existing test mocks keep matching. | ||
| const resolved = tryResolveProjectServer(serverName); | ||
| if (resolved) { | ||
| return testConnection(serverConfig, resolved.serverId, { | ||
|
Comment on lines
+775
to
+777
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When local mode has a Convex mapping, this resolver branch is also taken for OAuth servers. The callers pass a Useful? React with 👍 / 👎. |
||
| projectId: resolved.projectId, | ||
| serverName, | ||
|
Comment on lines
+776
to
+779
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When local mode has a populated Convex mapping (for migrated/existing synced servers), this connects the MCP client manager under the Convex Useful? React with 👍 / 👎. |
||
| }); | ||
| } | ||
| return testConnection(serverConfig, serverName); | ||
| }, | ||
| [assertClientConfigSynced] | ||
|
|
@@ -741,6 +754,13 @@ export function useServerState({ | |
| const guardedReconnectServer = useCallback( | ||
| async (serverName: string, serverConfig: MCPServerConfig) => { | ||
| assertClientConfigSynced(); | ||
| const resolved = tryResolveProjectServer(serverName); | ||
| if (resolved) { | ||
| return reconnectServer(resolved.serverId, serverConfig, { | ||
| projectId: resolved.projectId, | ||
| serverName, | ||
| }); | ||
| } | ||
| return reconnectServer(serverName, serverConfig); | ||
| }, | ||
| [assertClientConfigSynced] | ||
|
|
@@ -1683,54 +1703,45 @@ export function useServerState({ | |
| oauthFlowProfile: formOAuthProfile, | ||
| hasClientSecret: nextHasClientSecret, | ||
| }; | ||
| if (HOSTED_MODE) { | ||
| let syncErr: unknown; | ||
| try { | ||
| const serverId = await syncServerToConvex( | ||
| formData.name, | ||
| serverEntryForSave, | ||
| clientSecretSyncOptions | ||
| ); | ||
| if (serverId) { | ||
| hostedServerId = serverId; | ||
| injectHostedServerMapping(formData.name, serverId); | ||
| } | ||
| } catch (err) { | ||
| syncErr = err; | ||
| logger.warn("Sync to Convex failed (pre-connection)", { | ||
| serverName: formData.name, | ||
| err, | ||
| }); | ||
| } | ||
| // OAuth in hosted mode requires a Convex serverId to bind credentials | ||
| // to. Without it, prepareHostedProjectOAuthRedirect is a no-op and the | ||
| // local fallback can't persist tokens either (mcp-oauth.saveTokens is | ||
| // gated on !HOSTED_MODE), so the OAuth dance would complete without a | ||
| // durable credential. Fail loudly instead. | ||
| if (formData.useOAuth && !hostedServerId) { | ||
| const errorMessage = | ||
| syncErr instanceof Error | ||
| ? `Could not save the hosted server before starting OAuth: ${syncErr.message}` | ||
| : "Could not save the hosted server before starting OAuth. Please try again."; | ||
| dispatch({ | ||
| type: "CONNECT_FAILURE", | ||
| name: formData.name, | ||
| error: errorMessage, | ||
| }); | ||
| toast.error(errorMessage); | ||
| return; | ||
| } | ||
| } else { | ||
| syncServerToConvex( | ||
| // Both modes: await Convex sync so the returned serverId is available | ||
| // for OAuth binding (hosted) and for the new {projectId, serverId} | ||
| // request shape (local mode resolver path). Failure is non-fatal in | ||
| // local mode — the legacy {serverConfig, serverId: name} body still | ||
| // works as a fallback. | ||
| let syncErr: unknown; | ||
| try { | ||
| const serverId = await syncServerToConvex( | ||
| formData.name, | ||
| serverEntryForSave, | ||
| clientSecretSyncOptions | ||
| ).catch((err) => | ||
| logger.warn("Background sync to Convex failed (pre-connection)", { | ||
| serverName: formData.name, | ||
| err, | ||
| }) | ||
| ); | ||
| if (serverId) { | ||
| hostedServerId = serverId; | ||
| injectHostedServerMapping(formData.name, serverId); | ||
| } | ||
| } catch (err) { | ||
| syncErr = err; | ||
| logger.warn("Sync to Convex failed (pre-connection)", { | ||
| serverName: formData.name, | ||
| err, | ||
| }); | ||
| } | ||
| if (HOSTED_MODE && formData.useOAuth && !hostedServerId) { | ||
| // OAuth in hosted mode requires a Convex serverId to bind credentials | ||
| // to; without it the OAuth dance would complete without a durable | ||
| // credential. Local-mode OAuth follows the same constraint post- | ||
| // unification but the legacy localStorage fallback still catches it. | ||
| const errorMessage = | ||
| syncErr instanceof Error | ||
| ? `Could not save the hosted server before starting OAuth: ${syncErr.message}` | ||
| : "Could not save the hosted server before starting OAuth. Please try again."; | ||
| dispatch({ | ||
| type: "CONNECT_FAILURE", | ||
| name: formData.name, | ||
| error: errorMessage, | ||
| }); | ||
| toast.error(errorMessage); | ||
| return; | ||
| } | ||
| if (!isAuthenticated) { | ||
| const project = appState.projects[appState.activeProjectId]; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a local signed-in or guest session has legacy local projects and Convex initially returns no remote projects, this new migration hook runs in the same App mount as the existing
use-project-statemigration effect (client/src/hooks/use-project-state.ts:760-823). That older effect still callsprojects:createProjectfor every unshared local project and has no awareness ofMIGRATION_FLAG_KEY, while this hook also callsrunLocalStateMigrationfrom localStorage, so the first boot after this change can create each local project twice before either subscription observes the new remote rows. Gate or remove the old effect before enabling this one to avoid duplicate Convex projects/servers.Useful? React with 👍 / 👎.