fix(mcp): preserve authorized sessions during login (#122129)

Follow-up to #122115: startMcpOAuthAuthorization suppressed stored tokens
unconditionally, forcing a browser authorization on every mcp login even with a
valid session. Suppression is again gated on a recorded authorization-required
challenge; start returns a closed authorized|redirect result and the CLI
early-returns the already-logged-in outcome.

Co-authored-by: Ayaan Zaidi <hi@obviy.us>

src/agents/mcp-oauth.test.ts

@@ -169,6 +169,34 @@ describe("MCP OAuth provider", () => {
 
   afterEach(() => closeOpenClawStateDatabaseForTest());
 
+  it("reuses a valid stored session without persisting an authorization redirect", async () => {
+    await withTempHome(
+      async () => {
+        const provider = createMcpOAuthClientProvider({ identity: REMOTE_IDENTITY });
+        await provider.saveTokens({
+          access_token: "stored-access",
+          refresh_token: "stored-refresh",
+          token_type: "Bearer",
+          expires_in: 3600,
+        });
+        const before = readMcpOAuthStore(REMOTE_IDENTITY.storeKey);
+        authMock.mockImplementationOnce(async (loginProvider) =>
+          (await loginProvider.tokens()) ? "AUTHORIZED" : await persistRedirect(loginProvider),
+        );
+
+        await expect(
+          startMcpOAuthAuthorization(REMOTE_IDENTITY, resolvedOAuthConfig(REMOTE_IDENTITY), {}),
+        ).resolves.toEqual({ status: "authorized" });
+        expect(readMcpOAuthStore(REMOTE_IDENTITY.storeKey)).toEqual(before);
+      },
+      {
+        prefix: "openclaw-mcp-oauth-existing-session-",
+        skipSessionCleanup: true,
+        env: { OPENCLAW_CONFIG_PATH: undefined, OPENCLAW_STATE_DIR: undefined },
+      },
+    );
+  });
+
   it("preserves insufficient scope and forces the next login through authorization", async () => {
     await withTempHome(
       async () => {
@@ -231,7 +259,7 @@ describe("MCP OAuth provider", () => {
         });
         await expect(
           startMcpOAuthAuthorization(REMOTE_IDENTITY, resolvedOAuthConfig(REMOTE_IDENTITY), {}),
-        ).resolves.toMatchObject({ state: "state-1234567890" });
+        ).resolves.toMatchObject({ status: "redirect", state: "state-1234567890" });
         expect(provider.tokens()).toMatchObject({ access_token: "decoy-token" });
 
         authMock.mockImplementationOnce(async (loginProvider) => {
@@ -749,6 +777,9 @@ describe("MCP OAuth provider", () => {
           resolvedOAuthConfig(CALENDLY_IDENTITY),
           {},
         );
+        if (session.status !== "redirect") {
+          throw new Error("expected MCP OAuth redirect");
+        }
 
         expect(session.redirectUrl).toBe("http://localhost:8989/oauth/callback");
         expect(authMock.mock.calls[1]?.[0]?.clientMetadata.redirect_uris).toEqual([
@@ -903,6 +934,9 @@ describe("MCP OAuth provider", () => {
         };
         try {
           const first = await startMcpOAuthAuthorization(identity, config, {});
+          if (first.status !== "redirect") {
+            throw new Error("expected first MCP OAuth redirect");
+          }
           expect(readMcpOAuthStore(identity.storeKey)).toMatchObject({
             codeVerifier: expect.any(String),
             lastAuthorizationUrl: first.authorizationUrl,
@@ -918,6 +952,9 @@ describe("MCP OAuth provider", () => {
           expect(readMcpOAuthStore(identity.storeKey)).not.toHaveProperty("codeVerifier");
 
           const second = await startMcpOAuthAuthorization(identity, config, {});
+          if (second.status !== "redirect") {
+            throw new Error("expected second MCP OAuth redirect");
+          }
           await expect(
             completeMcpOAuthAuthorization(identity, config, { code: "wrong-code" }),
           ).rejects.toThrow();
@@ -928,6 +965,9 @@ describe("MCP OAuth provider", () => {
           });
 
           const third = await startMcpOAuthAuthorization(identity, config, {});
+          if (third.status !== "redirect") {
+            throw new Error("expected third MCP OAuth redirect");
+          }
           expect(third.authorizationUrl).not.toBe(second.authorizationUrl);
           await expect(
             completeMcpOAuthAuthorization(identity, config, {

src/agents/mcp-oauth.ts

@@ -34,6 +34,10 @@ type ResolvedHttpMcpTransportConfig = Extract<
   { kind: "http" }
 >;
 
+type McpOAuthAuthorizationStartResult =
+  | { status: "authorized" }
+  | { status: "redirect"; authorizationUrl: string; redirectUrl: string; state: string };
+
 /** Persisted OAuth credential presence and authorization state for one MCP server. */
 export type McpOAuthCredentialsStatus = {
   hasTokens: boolean;
@@ -333,7 +337,7 @@ async function runMcpOAuthAuthorizationAttempt(
     suppressStoredTokens?: boolean;
   },
   lease: OpenClawStateLeaseContext,
-): Promise<void> {
+): Promise<"authorized" | "redirect"> {
   const provider = createMcpOAuthClientProvider({
     identity: params.identity,
     config: params.config,
@@ -341,7 +345,7 @@ async function runMcpOAuthAuthorizationAttempt(
     suppressStoredTokens: params.suppressStoredTokens,
     lease,
   });
-  await auth(provider, {
+  const result = await auth(provider, {
     serverUrl: params.identity.serverUrl,
     authorizationCode: normalizeOptionalString(params.authorizationCode),
     resourceMetadataUrl: params.resourceMetadataUrl,
@@ -349,13 +353,14 @@ async function runMcpOAuthAuthorizationAttempt(
     fetchFn: withMcpOAuthLeaseSignal(params.fetchFn, lease.signal),
   });
   lease.assertOwned();
+  return result === "AUTHORIZED" ? "authorized" : "redirect";
 }
 
 export async function startMcpOAuthAuthorization(
   identity: McpOAuthIdentity,
   config: ResolvedHttpMcpTransportConfig,
   opts: { redirectUrl?: string },
-): Promise<{ authorizationUrl: string; redirectUrl: string; state: string }> {
+): Promise<McpOAuthAuthorizationStartResult> {
   const storeKey = identity.storeKey;
   return await withMcpOAuthLease(storeKey, async (lease) => {
     const store = readMcpOAuthStore(storeKey);
@@ -376,17 +381,18 @@ export async function startMcpOAuthAuthorization(
         ? new URL(pendingChallenge.resourceMetadataUrl)
         : undefined,
       scope: normalizeOptionalString(pendingChallenge?.scope),
-      suppressStoredTokens: true,
+      suppressStoredTokens: pendingChallenge?.requiresAuthorization === true,
     };
+    let result: "authorized" | "redirect";
     try {
-      await runMcpOAuthAuthorizationAttempt(attempt, lease);
+      result = await runMcpOAuthAuthorizationAttempt(attempt, lease);
     } catch (error) {
       if (
         !normalizeOptionalString(opts.redirectUrl) &&
         !normalizeOptionalString(config.oauth?.redirectUrl) &&
         isMcpOAuthRedirectRegistrationError(error)
       ) {
-        await runMcpOAuthAuthorizationAttempt(
+        result = await runMcpOAuthAuthorizationAttempt(
           {
             ...attempt,
             config: { ...config.oauth, redirectUrl: LOCALHOST_REDIRECT_URL },
@@ -397,13 +403,16 @@ export async function startMcpOAuthAuthorization(
         throw error;
       }
     }
+    if (result === "authorized") {
+      return { status: "authorized" };
+    }
     const pending = readMcpOAuthStore(storeKey);
     const authorizationUrl = pending.lastAuthorizationUrl;
     const state = authorizationUrl ? new URL(authorizationUrl).searchParams.get("state") : null;
     if (!authorizationUrl || !pending.codeVerifier || !pending.redirectUrl || !state) {
       throw new Error("MCP OAuth authorization session was not persisted.");
     }
-    return { authorizationUrl, redirectUrl: pending.redirectUrl, state };
+    return { status: "redirect", authorizationUrl, redirectUrl: pending.redirectUrl, state };
   });
 }
 

src/cli/mcp-cli.login-loopback.test.ts

@@ -80,6 +80,7 @@ function mockRedirectFlow(redirectUrl: string): void {
   authorizationUrl.searchParams.set("redirect_uri", redirectUrl);
   authorizationUrl.searchParams.set("state", "state-1234567890");
   mocks.startMcpOAuthAuthorization.mockResolvedValue({
+    status: "redirect",
     authorizationUrl: authorizationUrl.toString(),
     redirectUrl,
     state: "state-1234567890",
@@ -140,6 +141,21 @@ describe("mcp login loopback callback", () => {
     });
   });
 
+  it("reports an existing session without starting the loopback", async () => {
+    await withTempHome("openclaw-cli-mcp-loopback-home-", async () => {
+      await configureServer();
+      mocks.startMcpOAuthAuthorization.mockResolvedValue({ status: "authorized" });
+
+      await program.parseAsync(["mcp", "login", "docs"], { from: "user" });
+
+      expect(mocks.runtime.log).toHaveBeenCalledWith('MCP OAuth credentials saved for "docs".');
+      expect(mocks.completeMcpOAuthAuthorization).not.toHaveBeenCalled();
+      expect(
+        mocks.runtime.log.mock.calls.some(([line]) => String(line).includes("Open this URL")),
+      ).toBe(false);
+    });
+  });
+
   it("falls back immediately to the printed manual command when binding fails", async () => {
     await withTempHome("openclaw-cli-mcp-loopback-home-", async () => {
       await configureServer();

src/cli/mcp-cli.ts

@@ -1333,6 +1333,10 @@ export function registerMcpCli(program: Command) {
       const manualCommand = formatCliCommand(`openclaw mcp login ${name} --code <code>`);
       try {
         const session = await startMcpOAuthAuthorization(identity, resolved, {});
+        if (session.status === "authorized") {
+          defaultRuntime.log(`MCP OAuth credentials saved for "${name}".`);
+          return;
+        }
         if (session.state.length >= 16) {
           try {
             callbackServer = await startOAuthLoopbackCallbackServer({