google-gemini / google-gemini/gemini-cli
bug: OAuth callback timeout is retained after the flow completes
- Dominant language
- TypeScript
- Stars
- 107k
- Forks
- 14.6k
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 45
Description
## What happened?
The shared OAuth callback flow creates a five-minute timeout for every authentication attempt but does not clear it when the callback server closes after success, provider error, state rejection, or another terminal path.
In `packages/core/src/utils/oauth-flow.ts:226-243`:
- `timeoutId` is assigned a five-minute timer.
- The abort listener closes the server and rejects the response promise when that timer fires.
- The server's `close` handler removes the abort listener but never calls `clearTimeout(timeoutId)`.
As a result, a successfully completed flow retains the timer callback and its closure until the full timeout elapses. When it eventually fires, it aborts an already-settled flow. `unref()` prevents the timer alone from keeping Node alive, but does not release the retained objects while the process continues running.
The older Code Assist OAuth flow explicitly clears its corresponding timeout, and its tests assert that cleanup.
## How can this be reproduced?
1. Use fake timers and spy on `clearTimeout`.
2. Call `startCallbackServer()` and await its selected port.
3. Send a valid callback containing the expected state and await `response` successfully.
4. Observe that the server closes but `clearTimeout` is never called.
5. Advancing fake time by five minutes still executes the stale timeout callback.
Repeated successful authentication attempts accumulate one retained closure each for up to five minutes.
## What did you expect to happen?
Every terminal path should perform idempotent cleanup: clear the timeout, remove listeners, and close the callback server. No timer belonging to a completed flow should fire later.
## Why this matters
Long-running CLI/SDK processes that perform repeated OAuth flows retain unnecessary server/controller/promise state. The stale callback also makes lifecycle behavior harder to reason about and can interfere with tests using fake timers.
## Suggested direction
- Centralize settlement and cleanup in one idempotent helper.
- Call `clearTimeout(timeoutId)` whenever the server/response settles.
- Ensure the request success, OAuth error, state error, parser error, server error, and timeout paths all use that helper.
- Add assertions that the timeout and listeners are removed for both successful and failed flows.
## Client information
Found by source audit on current `main`, commit `f47d6c6f7`. This is platform-independent OAuth lifecycle logic; `/about` output is not applicable.
Contributor guide
Research direction
Start in packages/core/src/utils/oauth-flow.ts:226-243 and trace startCallbackServer through its success, error, rejection, parser, server-error, and timeout paths. Use fake timers with a clearTimeout spy, then add assertions that completed flows clear the timeout and remove listeners without later firing the stale callback.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- authentication, cli
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 74/100