Bound OAuth token exchanges with a cancelling timeout (CL-7235)

[TheGreatAxios]

Closes CL-7235.

The defect

The four OAuth connect modules — github, gmail, huggingface and openrouter — each post once to their provider's token endpoint with no timeout. An unresponsive provider hangs the exchange indefinitely.

The fix

A shared postExchangeRequest helper (packages/connections/src/oauth-exchange-fetch.ts) wraps that single call in AbortSignal.timeout, and all four modules now go through it instead of four separate copies of the same wiring.

The important property: this cancels rather than abandons. AbortSignal.timeout aborts the underlying request when the timer fires; it does not race a timer while leaving the fetch running unobserved. Three other tickets in this batch (CL-7193, CL-7215, CL-7230) exist precisely because a timeout was implemented the abandoning way, so this deliberately does not add a fourth.

It also follows existing precedent rather than inventing a pattern — probes.ts already applies AbortSignal.timeout to every outbound call in this package.

A timeout rejects the same way any other network failure does, so each caller's existing catch-and-convert-to-{ ok: false, message } handling is unchanged, and the existing exchange_failed redirect path in oauth-routes.ts (already covered by oauth-routes.test.ts) handles it with no new code.

Verification

• packages/connections: 256 pass, 6 skip, 0 fail
• WORKBENCH_CHECK_SINCE=origin/main bun run typecheck: clean
• WORKBENCH_CHECK_SINCE=origin/main bun run test: clean
• bun run lint: 0 errors (8 pre-existing warnings, all unrelated files)
• bun run check:structural: clean
• Prettier clean on all six touched files
• Reviewed by Greybeard (approach, before implementation) and Critique (implementation, after commit) — both passed with no findings requiring changes.

Not merged.

[TheGreatAxios]

Greybeard · approved

Reviewed the approach before implementation: one shared postExchangeRequest helper in packages/connections/src/oauth-exchange-fetch.ts, mirroring probes.ts's existing AbortSignal.timeout pattern rather than inventing a new one. An injectable timeoutMs (default OAUTH_EXCHANGE_TIMEOUT_MS = 10_000) lets tests prove the abort-not-abandon behavior without real 10s waits.

Checked before locking the value: apps/hub's Bun.serve runs with idleTimeout: 0 (apps/hub/src/index.ts:3528) and this repo has no reverse-proxy config, so nothing upstream kills the request before the 10s inner timeout would fire.

Two framing notes, not blockers: gmail has no exported ExchangeFetch type today, so that module is adding an export, not replacing one; index.ts re-exports ExchangeFetch under provider-specific aliases for two of the four, which stay type-compatible since OAuthExchangeFetch only adds an optional signal field.

No architectural objections.

[TheGreatAxios]

Critique · approved

Verified the implementation against the agreed approach.

• Cancelling, not abandoning: postExchangeRequest passes the real AbortSignal.timeout(timeoutMs) into fetchImpl's init.signal — same primitive probes.ts already uses. oauth-exchange-fetch.test.ts's first test proves it behaviorally: a fetchImpl that never resolves rejects once the signal fires, driven by a real 100ms timer.
• All four providers wired: grepped every doFetch/fetchImpl call site in github-connect.ts:56, gmail-connect.ts:82, huggingface-connect.ts:85, openrouter-connect.ts:50 — each now routes through postExchangeRequest. No leftover unwired call site.
• Type safety: tsc --noEmit clean in packages/connections; signal?: AbortSignal is genuinely optional, never assigned undefined (respects exactOptionalPropertyTypes). No caller outside these four modules imports their ExchangeFetch type, so no blast radius.
• Credential-leak risk: none — every timeout/abort rejection flows through the same unchanged catch (cause) { ok: false, message } blocks that already handled other network failures. No new error path. Test fixtures use obviously-fake values.
• Test coverage: shared helper tests cover abort-not-abandon, custom timeoutMs honored, success path, and default-timeout wiring; each of the four connect files gets one wiring-assertion test (signal present, not pre-aborted) rather than duplicating the slow real-timer test five times — correct separation. The two previously test-less files (huggingface, openrouter) also gained baseline success/failure coverage.
• Scope: no caller outside packages/connections invokes any of the four exchangeCodeFor* functions; diff stays within the four connect modules, the new shared helper, and their tests.

No findings.