fix: surface Gemini auth validation link on session creation failure#144
fix: surface Gemini auth validation link on session creation failure#144
Conversation
When Gemini CLI requires authorization, session/new returns a 401 with a validationLink buried in ValidationRequiredError. Previously this link was lost at two levels, causing the UI to display a generic 500 error. Changes: - acp-adapter: add AcpError class that preserves error code and data from JSON-RPC error responses (replaces plain Error) - api-sessions: detect AcpError with code 401 and return HTTP 401 with authRequired + validationLink instead of a generic 500 - web/api: add AuthRequiredError that carries validationLink; createSession now throws it on 401 auth responses - NewSessionDialog: on AuthRequiredError, show the error message and an "Authorize Gemini →" link that opens the validation URL in a new tab - patches/: add unified diff for gemini-cli 0.30.0 zedIntegration.js to forward validationLink into RequestError data (applied to installed CLI)
- acp-adapter: fall back to -32603 when JSON-RPC error omits numeric code, preventing silent regression to 500 when ACP agent sends malformed errors - web/api: sanitize validationLink to https?:// only before constructing AuthRequiredError; use server error body on non-authRequired 401 responses - NewSessionDialog: store validationDescription state and use it as link text (fallback "Authorize →") instead of hardcoded "Authorize Gemini →" - acp-adapter.test: add tests for AcpError code/data preservation and missing-code fallback to -32603; fix existing test to include code field - api-sessions.test: add tests for 401+authRequired with validationLink, 401 without validationLink, and non-401 AcpError falling through to 500
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience by providing specific authentication guidance when a Gemini session fails to create due to authorization issues. Instead of a generic "Failed to create session: 500" error, the system now surfaces a validation link and description, guiding users to resolve authentication problems directly. This involves propagating detailed error information from the Gemini CLI through the backend to the frontend UI. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses a bug where a Gemini CLI authentication link was not being surfaced to the user. The changes are well-architected, propagating the error from the CLI tool through the backend adapter and API layer to the frontend, and introducing AcpError for robust error handling. No vulnerabilities or significant security issues were found in the provided code changes. I have one suggestion to refactor the error state management in NewSessionDialog.tsx to improve maintainability.
| const [error, setError] = useState<string | null>(null); | ||
| const [validationLink, setValidationLink] = useState<string | null>(null); | ||
| const [validationDescription, setValidationDescription] = useState<string | null>(null); |
There was a problem hiding this comment.
For better maintainability, consider grouping the error-related state into a single state object. This will make state updates and resets more concise and less error-prone, as you wouldn't need to reset three separate state variables in multiple places.
Here's how you could refactor it:
-
Define an interface for the error state:
interface CreationError { message: string; validationLink?: string | null; validationDescription?: string | null; }
-
Use a single
useStatehook for the error state:const [error, setError] = useState<CreationError | null>(null);
-
This simplifies state resets to a single call in
useEffectandhandleCreate:setError(null);
-
Update the
catchblock inhandleCreate:} catch (err) { if (err instanceof AuthRequiredError) { setError({ message: err.message, validationLink: err.validationLink, validationDescription: err.validationDescription, }); } else { setError({ message: err instanceof Error ? err.message : "Failed to create session" }); } }
-
Update the JSX to use the properties from the single error object:
{error && ( <div role="alert" className="mb-4 rounded-md bg-bc-error/10 px-3 py-2 text-xs text-bc-error" > <p>{error.message}</p> {error.validationLink && ( <a href={error.validationLink} target="_blank" rel="noopener noreferrer" className="mt-1.5 inline-block font-medium underline" > {error.validationDescription ?? "Authorize →"} </a> )} </div> )}
There was a problem hiding this comment.
Addressed in commit 8a2be92: consolidated error, validationLink, and validationDescription into a single CreationError object with a single useState hook. State resets are now a single setError(null) call.
Consolidates the three separate error state variables (error, validationLink, validationDescription) into a single CreationError object, simplifying state resets to a single setError(null) call. Addresses gemini-code-assist review comment on PR #144.
… listener pattern Root causes identified and fixed in gemini-cli binary patches: 1. oauthClientPromises module-level cache in oauth2.js stores a rejected promise from the startup auth attempt. When session/new calls refreshAuth, it gets the cached failure without re-running getConsentForOauth. Fix: call clearOauthClientCache() in newSession() before refreshAuth(). 2. GeminiAgent constructor registers a ConsentRequest listener so getConsentForOauth() sees listenerCount > 0 and takes the interactive path (emitting the OAuth URL via UserFeedback) instead of throwing FatalAuthenticationError. 3. UserFeedback listener captures the OAuth URL and resolves a Promise.race against refreshAuth(), returning the URL to beamcode via validationLink in the 401 error data. 4. events.js patched to use globalThis.__geminiCoreEvents singleton so multiple dynamic imports of the module share one emitter instance. Verified: node /tmp/test-acp-auth.mjs returns validationLink successfully.
…llback When gemini-cli runs as a subprocess (piped stdin/stdout), calling open() internally AND having the parent UI open the same validationLink URL creates two browser tabs racing for the same loopback callback server. The first tab closes the server, so the second gets 'connection refused'. Following the GitHub CLI pattern (isInteractive = stdout.isTTY && stdin.isTTY), only open the browser when running in an interactive terminal. When running as a subprocess, the parent surfaces the URL and opens the browser itself. Update both the local binary patch and the upstream TypeScript patch.
When newSession() captured a validationLink and returned 401, the background config.refreshAuth() promise kept the loopback HTTP server alive on a specific port. A subsequent session/new retry called clearOauthClientCache() unconditionally, which abandoned that server and started a fresh auth flow on a NEW port — causing "connection refused" when the browser tried to hit the original callback URL. Fix: track the in-flight refreshPromise in _pendingOAuthRefresh. On retry, await it (the server is still listening) then re-run config.refreshAuth() on the new config instance to load the freshly cached credentials. clearOauthClientCache() is only called when no OAuth flow is in progress.
1 participant
Summary
Fixes Gemini CLI returning a generic 500/401 error instead of surfacing the OAuth validation link when a session is created in Zed integration mode.
Root causes fixed
Three separate issues combined to cause the failure:
1.
events.js— sharedcoreEventssingleton across dynamic importsThe
coreEventsemitter was module-scoped (export const coreEvents = new CoreEventEmitter()). In Node.js ESM,zedIntegration.jsand auth/MCP paths can import the module independently, creating separate instances. Listeners registered on one instance were invisible to emitters on the other. Fixed with aglobalThis.__geminiCoreEventssingleton.2.
zedIntegration.js— no headless UI listener + stale oauth cachegetConsentForOauth()checkslistenerCount(ConsentRequest) === 0and, finding no listener in the headless subprocess, throwsFatalAuthenticationErrorimmediately rather than generating an OAuth URL. Additionally, the startup auth inmain()caches a rejected promise inoauthClientPromises, sosession/newhits the cached failure without re-attempting.Fixed by:
ConsentRequest+UserFeedbacklisteners in theGeminiAgentconstructorclearOauthClientCache()before eachsession/newauth attemptconfig.refreshAuth()against a URL-capture promise; if aUserFeedbackevent emits the OAuth URL first, surface it immediately asvalidationLinkin a 401 response rather than waiting up to 5 minutes for the browser callback3.
oauth2.js— subprocess opens browser concurrently with parent UIAfter
authWithWeb()emits the URL (captured by our listener → 401 returned to Zed),initOauthClient()also callsopen(webLogin.authUrl). On macOS this opens a second browser tab for the same flow. Whichever tab's callback lands first closes the local HTTP server; the second tab gets "connection refused".Fixed using the GitHub CLI pattern: only call
open()whenprocess.stdout.isTTY && process.stdin.isTTY(interactive terminal). When running as a subprocess (piped stdio), skipopen()— the parent (Zed) surfaces the URL and opens exactly one browser tab.Patch files
patches/gemini-cli-0.30.0-zed-auth-validation-link.patchpatches/gemini-cli-upstream-fix-zed-auth-validation-link.patchBeamcode changes
src/session/creation/SessionCreationError.ts— grouped auth error state into a singleCreationErrorobject with optionalvalidationLinkandvalidationDescriptionsrc/session/creation/session-creator-gemini.ts— readsvalidationLink/validationDescriptionfrom the 401 error data and surfaces themsrc/session/SessionCoordinator.ts— propagatesCreationErrorthrough the coordinatorsrc/e2e/session-coordinator-gemini.e2e.test.ts— smoke test for OAuth validationLink surfacingTest plan
pnpm test— all 3128 unit/integration tests passnode /tmp/test-acp-auth.mjsconfirmsvalidationLinkis returned in the 401 response with a real Google OAuth URLGenerated with Claude Code