fix: defer OAuth token acquisition to first use; stop browser popups on transient refresh errors

index.ts

@@ -177,7 +177,7 @@ import { CookieJar, parse as parseCookie } from "tough-cookie";
 import { fileURLToPath, URL } from "node:url";
 import { z } from "zod";
 
-import { initializeOAuthClient, GitLabOAuth } from "./oauth.js";
+import { createGitLabOAuthClient, GitLabOAuth } from "./oauth.js";
 import { createGitLabOAuthProvider } from "./oauth-proxy.js";
 import { mcpAuthRouter } from "@modelcontextprotocol/sdk/server/auth/router.js";
 import { normalizeGitLabApiUrl } from "./utils/url.js";
@@ -12470,10 +12470,12 @@ async function runServer() {
       logger.info("Using OAuth authentication...");
       try {
         const gitlabBaseUrl = GITLAB_API_URL.replace(/\/api\/v4$/, "");
-        const oauthResult = await initializeOAuthClient(gitlabBaseUrl);
-        oauthClient = oauthResult.client;
-        OAUTH_ACCESS_TOKEN = oauthResult.accessToken;
-        logger.info("OAuth authentication successful");
+        // Construct the client synchronously (no network). The token is
+        // acquired lazily on the first tool call via ensureValidOAuthToken,
+        // by which point the network is ready. This avoids blocking startup
+        // and avoids opening a browser on transient boot-time network errors.
+        oauthClient = createGitLabOAuthClient(gitlabBaseUrl);
+        logger.info("OAuth enabled; token acquired lazily on first use.");
       } catch (error) {
         logger.error("OAuth authentication failed:", error);
         process.exit(1);

oauth.ts

@@ -29,6 +29,29 @@ const pendingAuthRequests = new Map<
   }
 >();
 
+const OAUTH_REFRESH_TIMEOUT_MS = 10000;
+
+/**
+ * Decide whether a failed token-refresh response means the refresh token is
+ * genuinely dead (interactive re-login would fix it) versus a transient or
+ * configuration problem (where opening a browser is wrong).
+ *
+ * Returns true ONLY for a 400/401 whose JSON body has error "invalid_grant"
+ * or "invalid_token". Everything else — config errors (invalid_client,
+ * invalid_request), 5xx, 429, and non-JSON bodies — returns false.
+ */
+export function isAuthInvalidTokenResponse(status: number, bodyText: string): boolean {
+  if (status !== 400 && status !== 401) {
+    return false;
+  }
+  try {
+    const body = JSON.parse(bodyText) as { error?: string };
+    return body.error === "invalid_grant" || body.error === "invalid_token";
+  } catch {
+    return false;
+  }
+}
+
 interface TokenData {
   access_token: string;
   refresh_token?: string;
@@ -224,11 +247,15 @@ export class GitLabOAuth {
         "Content-Type": "application/x-www-form-urlencoded",
       },
       body: params.toString(),
+      signal: AbortSignal.timeout(OAUTH_REFRESH_TIMEOUT_MS),
     });
 
     if (!response.ok) {
-      const errorText = await response.text();
-      throw new Error(`Token refresh failed: ${response.status} ${errorText}`);
+      const bodyText = await response.text();
+      const authInvalid = isAuthInvalidTokenResponse(response.status, bodyText);
+      throw Object.assign(new Error(`Token refresh failed: ${response.status} ${bodyText}`), {
+        authInvalid,
+      });
     }
 
     const data = (await response.json()) as {
@@ -577,8 +604,13 @@ export class GitLabOAuth {
           tokenData = await this.refreshAccessToken(tokenData.refresh_token);
           this.saveToken(tokenData);
         } catch (error) {
-          logger.error("Token refresh failed. Starting new OAuth flow...", error);
-          tokenData = await this.startOAuthFlow();
+          if ((error as { authInvalid?: boolean }).authInvalid === true) {
+            logger.error("Refresh token invalid, starting new OAuth flow...", error);
+            tokenData = await this.startOAuthFlow();
+          } else {
+            logger.error("Token refresh failed (transient/network), not opening browser.", error);
+            throw error;
+          }
         }
       } else {
         logger.info("No refresh token available. Starting new OAuth flow...");
@@ -616,11 +648,11 @@ export class GitLabOAuth {
 }
 
 /**
- * Create and initialize a GitLabOAuth client.
- * Performs initial authentication (triggers browser flow if needed).
- * Returns the client instance and the initial access token.
+ * Construct a GitLabOAuth client from environment configuration.
+ * Does NOT perform any network access — no token is acquired here.
+ * Throws if GITLAB_OAUTH_CLIENT_ID is missing.
  */
-export async function initializeOAuthClient(gitlabUrl: string = "https://gitlab.com"): Promise<{ client: GitLabOAuth; accessToken: string }> {
+export function createGitLabOAuthClient(gitlabUrl: string = "https://gitlab.com"): GitLabOAuth {
   const clientId = process.env.GITLAB_OAUTH_CLIENT_ID;
   const clientSecret = process.env.GITLAB_OAUTH_CLIENT_SECRET;
   const redirectUri = process.env.GITLAB_OAUTH_REDIRECT_URI || "http://127.0.0.1:8888/callback";
@@ -632,14 +664,23 @@ export async function initializeOAuthClient(gitlabUrl: string = "https://gitlab.
     );
   }
 
-  const oauth = new GitLabOAuth({
+  return new GitLabOAuth({
     clientId,
     clientSecret,
     redirectUri,
     gitlabUrl,
     scopes: [process.env.GITLAB_READ_ONLY_MODE === "true" ? "read_api" : "api"],
     tokenStoragePath,
   });
+}
+
+/**
+ * Create and initialize a GitLabOAuth client.
+ * Performs initial authentication (triggers browser flow if needed).
+ * Returns the client instance and the initial access token.
+ */
+export async function initializeOAuthClient(gitlabUrl: string = "https://gitlab.com"): Promise<{ client: GitLabOAuth; accessToken: string }> {
+  const oauth = createGitLabOAuthClient(gitlabUrl);
 
   // Single call: triggers browser flow if needed, or reads cached token
   const accessToken = await oauth.getAccessToken();

package.json

@@ -51,7 +51,7 @@
     "changelog": "auto-changelog -p",
     "test": "npm run test:all",
     "test:all": "npm run build && npm run test:mock && npm run test:live",
-    "test:mock": "node --import tsx/esm --test test/remote-auth-simple-test.ts && node --import tsx/esm --test test/mcp-oauth-tests.ts && node --import tsx/esm --test test/streamable-http-static-token-auth.test.ts && tsx test/oauth-tests.ts && tsx test/test-list-merge-requests.ts && node --import tsx/esm --test test/test-merge-request-pipelines.ts && tsx test/test-list-project-members.ts && tsx test/test-download-attachment.ts && node --import tsx/esm --test test/test-upload-markdown.ts && node --import tsx/esm --test test/test-job-artifacts.ts && node --import tsx/esm --test test/test-deployment-tools.ts && node --import tsx/esm --test test/test-merge-request-approval-state-tools.ts && node --import tsx/esm --test test/test-search-code.ts && node --import tsx/esm --test test/test-tags.ts && node --import tsx/esm --test test/test-toolset-filtering.ts && node --import tsx/esm --test test/test-ci-lint.ts && node --import tsx/esm --test test/test-todos.ts && node --import tsx/esm --test test/test-auth-retry.ts && node --import tsx/esm --test test/test-issue-description-patch.ts && node --import tsx/esm --test test/test-geteffectiveprojectid.ts && node --import tsx/esm --test test/test-get-file-blame.ts && node --import tsx/esm --test test/stateless/codec.test.ts test/stateless/client-id.test.ts test/stateless/callback-proxy.test.ts test/stateless/session-id.test.ts test/stateless/session-id-integration.test.ts test/stateless/config-ttl.test.ts && node --import tsx/esm --test test/utils/tool-args.test.ts && node --import tsx/esm --test test/utils/merge-request-position.test.ts && node --import tsx/esm --test test/nullish-tool-arguments-schema.test.ts && node --import tsx/esm --test test/test-ci-variables.ts",
+    "test:mock": "node --import tsx/esm --test test/remote-auth-simple-test.ts && node --import tsx/esm --test test/mcp-oauth-tests.ts && node --import tsx/esm --test test/streamable-http-static-token-auth.test.ts && tsx test/oauth-tests.ts && tsx test/test-list-merge-requests.ts && node --import tsx/esm --test test/test-merge-request-pipelines.ts && tsx test/test-list-project-members.ts && tsx test/test-download-attachment.ts && node --import tsx/esm --test test/test-upload-markdown.ts && node --import tsx/esm --test test/test-job-artifacts.ts && node --import tsx/esm --test test/test-deployment-tools.ts && node --import tsx/esm --test test/test-merge-request-approval-state-tools.ts && node --import tsx/esm --test test/test-search-code.ts && node --import tsx/esm --test test/test-tags.ts && node --import tsx/esm --test test/test-toolset-filtering.ts && node --import tsx/esm --test test/test-ci-lint.ts && node --import tsx/esm --test test/test-todos.ts && node --import tsx/esm --test test/test-auth-retry.ts && node --import tsx/esm --test test/test-issue-description-patch.ts && node --import tsx/esm --test test/test-geteffectiveprojectid.ts && node --import tsx/esm --test test/test-get-file-blame.ts && node --import tsx/esm --test test/stateless/codec.test.ts test/stateless/client-id.test.ts test/stateless/callback-proxy.test.ts test/stateless/session-id.test.ts test/stateless/session-id-integration.test.ts test/stateless/config-ttl.test.ts && node --import tsx/esm --test test/utils/tool-args.test.ts && node --import tsx/esm --test test/utils/merge-request-position.test.ts && node --import tsx/esm --test test/nullish-tool-arguments-schema.test.ts && node --import tsx/esm --test test/test-ci-variables.ts && node --import tsx/esm --test test/oauth-refresh-classify.test.ts",
     "test:stateless": "npm run build && node --import tsx/esm --test test/stateless/codec.test.ts test/stateless/client-id.test.ts test/stateless/callback-proxy.test.ts test/stateless/session-id.test.ts test/stateless/session-id-integration.test.ts test/stateless/config-ttl.test.ts",
     "test:mcp-oauth": "npm run build && node --import tsx/esm --test test/mcp-oauth-tests.ts",
     "test:live": "node test/validate-api.js",

test/oauth-refresh-classify.test.ts

@@ -0,0 +1,45 @@
+/**
+ * Unit tests for isAuthInvalidTokenResponse.
+ *
+ * Pure-function tests — no env vars or external services needed.
+ * Importing ../oauth.js only constructs a logger; it does not touch the
+ * network or start any server.
+ */
+
+import { describe, test } from "node:test";
+import assert from "node:assert";
+import { isAuthInvalidTokenResponse } from "../oauth.js";
+
+describe("isAuthInvalidTokenResponse", () => {
+  test("400 invalid_grant is auth-invalid", () => {
+    assert.strictEqual(isAuthInvalidTokenResponse(400, '{"error":"invalid_grant"}'), true);
+  });
+
+  test("400 invalid_token is auth-invalid", () => {
+    assert.strictEqual(isAuthInvalidTokenResponse(400, '{"error":"invalid_token"}'), true);
+  });
+
+  test("401 invalid_grant is auth-invalid", () => {
+    assert.strictEqual(isAuthInvalidTokenResponse(401, '{"error":"invalid_grant"}'), true);
+  });
+
+  test("400 invalid_client is not auth-invalid (config error)", () => {
+    assert.strictEqual(isAuthInvalidTokenResponse(400, '{"error":"invalid_client"}'), false);
+  });
+
+  test("400 invalid_request is not auth-invalid (config error)", () => {
+    assert.strictEqual(isAuthInvalidTokenResponse(400, '{"error":"invalid_request"}'), false);
+  });
+
+  test("500 server_error is not auth-invalid (transient)", () => {
+    assert.strictEqual(isAuthInvalidTokenResponse(500, '{"error":"server_error"}'), false);
+  });
+
+  test("429 is not auth-invalid (rate limited)", () => {
+    assert.strictEqual(isAuthInvalidTokenResponse(429, '{"error":"invalid_grant"}'), false);
+  });
+
+  test("non-JSON body is not auth-invalid", () => {
+    assert.strictEqual(isAuthInvalidTokenResponse(400, "gateway timeout"), false);
+  });
+});