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

[zereight]

Problem

When GITLAB_USE_OAUTH=true, the server acquires the OAuth token eagerly and synchronously at startup (runServer → initializeOAuthClient → getAccessToken), blocking the MCP handshake on a network call to GitLab.

For a self-hosted GitLab (often behind a VPN), the network frequently isn't ready at the instant the MCP client spawns the server. The refresh fetch then hangs past the client's connection timeout, and on any refresh error the code falls through to the interactive startOAuthFlow() — opening a browser on every cold start.

Fix

• Defer token acquisition. Construct the OAuth client synchronously at startup (no network) via a new createGitLabOAuthClient. The token resolves lazily on the first tool call (every call already passes through ensureValidOAuthToken), by which point the network is ready. initializeOAuthClient is kept and now delegates.
• Always deliver the token (fixes fix: defer OAuth token acquisition to first use; stop browser popups on transient refresh errors #495 review feedback). New testable helper ensureOAuthToken(client, currentToken, onToken): the fast path requires a held token AND a valid stored token, so the first call after lazy startup populates the token even when the on-disk token is still valid. Token-script setups resolve exactly once.
• Fast-fail refresh. 10s AbortSignal.timeout on the refresh request.
• Browser only when warranted. isAuthInvalidTokenResponse(status, body) returns true only for 400/401 with invalid_grant/invalid_token. Transient and config errors propagate to the caller without a popup.

Tests

• New test/oauth-startup.test.ts (14 cases): 8 classifier cases + 6 ensureOAuthToken cases (valid-cache delivery without network via closed-port URL, no re-delivery fast path, refresh + persist against a loopback /oauth/token mock, transient-failure propagation, token-script single resolution). Wired into the pure-unit group in scripts/run-mock-tests.sh.
• npm run build clean; full npm run test:mock green; test-auth-retry 19/19 and oauth-device-flow 22/22 still green.

Behavior change

First tool call (instead of startup) opens the browser when no stored token exists; a transient refresh failure fails that call instead of popping a browser. Documented in docs/configuration/environment-variables.md.

---

Supersedes #495. Thanks to @medyas for the original fix — this PR carries it forward with the cached-token fix and regression tests from the review.

[coderabbitai]

Warning

Review limit reached

Next included review available in 21 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 751de8b5-e278-44ec-83b6-4011e14d3e30

📥 Commits

Reviewing files that changed from the base of the PR and between a7d19ad and c142d87.

📒 Files selected for processing (5)

• docs/configuration/environment-variables.md
• index.ts
• oauth.ts
• scripts/run-mock-tests.sh
• test/oauth-startup.test.ts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[zereight]

@cursor please review this PR

[cursor]

PR #732 리뷰 올렸다. 코어 픽 자체는 맞다. #495에서 막혔던 “디스크 토큰이 유효하면 OAUTH_ACCESS_TOKEN을 안 넣는” 구멍은 currentToken && hasValidToken()으로 실제로 막혔고, 닫힌 포트로 네트워크 없이 검증한 테스트도 그 축을 제대로 찌른다. 로컬에서 npm run build + oauth-startup 14/14 통과, CI도 green이다.

머지 전에 손볼 건 두 개다. docs/auth/oauth-setup.md는 아직도 “서버 시작하자마자 브라우저가 뜬다”고 적혀 있어서, 이 PR이 바꾸는 동작이랑 정반대다. 그리고 ensureOAuthToken에는 wrapWithAuthRetry에 있는 in-flight lock이 없다. 만료 토큰으로 첫 tool call이 겹치면 refresh가 두 번 나가서 한쪽이 invalid_grant 맞고 브라우저를 다시 연다. 이 PR이 막으려던 그 증상이다.

차단할 버그는 못 봤다. 문서 불일치랑 first-use race는 경고로 남겼다. 자신감 8/10.

  

[cursor]

Review

방향은 맞다. #495에서 막혔던 cached-token 구멍(hasValidToken()만 보고 early return)도 currentToken && hasValidToken()으로 실제로 막혔고, 그 회귀를 닫힌 포트(127.0.0.1:9)로 네트워크 없이 검증한 것도 좋다. classifier / refresh persist / transient 500 / token-script once 구성도 이 PR이 고치는 축을 잘 찌른다.

로컬에서 npm run build + test/oauth-startup.test.ts 14/14 통과. CI도 green.

---

Warning

1. docs/auth/oauth-setup.md가 새 동작과 반대다

이 PR은 “브라우저는 서버 시작이 아니라 첫 tool call”이 핵심 행동 변경인데, 셋업 가이드는 아직 이렇게 적혀 있다:

When the server starts, it will: Detect that no OAuth token exists / Automatically open your default browser

environment-variables.md만 고치면 부족하다. 첫 연동 유저가 보는 문서가 이 파일이다. 서버를 띄워놓고 브라우저가 안 떠서 “또 망가졌네” 하게 된다. 여기도 first tool call 기준으로 맞춰야 한다.

2. first-use getAccessToken에 in-flight lock이 없다

wrapWithAuthRetry는 이미 refreshLock이 있다. ensureOAuthToken은 없다.

만료 토큰 + 첫 사용에서 tool call이 겹치면:

1. 둘 다 currentToken == null → 둘 다 refresh
2. GitLab이 refresh token을 rotate하면 한쪽이 invalid_grant
3. 그 쪽이 브라우저를 연다 — 이 PR이 막으려던 그 증상

stdio는 보통 한 번에 하나라 운 좋게 안 터질 수 있다. 그래도 lock 패턴이 바로 옆에 있는데 안 가져온 건 구멍이다. 간단한 in-flight Promise면 된다.

// 둘 다 currentToken=null, disk token expired
await Promise.all([
  ensureOAuthToken(client, null, onToken),
  ensureOAuthToken(client, null, onToken),
]);
// 두 번째 refresh가 invalid_grant → startOAuthFlow()

---

Note

• #495에서 요청한 e2e(buildAuthHeaders / outgoing Authorization) 는 헬퍼 추출로 우회했다. index.ts 배선은 3줄이라 납득은 된다. 다만 OAUTH_ACCESS_TOKEN = token 실수는 이 테스트가 못 잡는다.
• error as { authInvalid?: boolean } 대신 typed error(예: authInvalid 필드 있는 클래스 + type guard)가 더 안전하다. timeout AbortError는 속성 없어서 브라우저를 안 여는 경로는 맞다.
• startup catch 메시지가 여전히 "OAuth authentication failed"다. 지금은 client 생성/설정 오류만 잡는다.
• refresh timeout(10s) / AbortError → no browser 경로는 로직상 맞는데 테스트는 없다.

---

잘한 점

• lazy construct + 기존 ensureValidOAuthToken 경로 재사용. handshake를 GitLab 네트워크에 묶지 않는다.
• classifier가 보수적이다. invalid_client / 5xx / 429 / non-JSON은 브라우저를 안 연다.
• token-script는 프로세스당 1회. hasValidToken()이 script에서 항상 true인 기존 동작과 맞다.
• initializeOAuthClient를 남겨 둔 것도 맞다.

---

판정: 코어 픽은 merge 해도 된다. 머지 전에 oauth-setup.md는 고치는 게 맞다. first-use lock은 같이 넣는 걸 추천한다. 둘 다 없으면 “문서와 실제가 다르고, 병렬 첫 호출에서 브라우저가 다시 뜬다”.