🤖 refactor: convert coderOauthService internals to Effect and adopt handlerGen for coder OAuth procedures

[ThomasK33]

Summary

Phase 6b of the progressive Effect migration (Wave 2): converts coderOauthService internals to Effect-native pipelines and moves all 5 coderOauth router procedures onto handlerGen, following the house pattern established in #4033/#4034 (tagged reason-carrying errors, Effect.gen internals, uninterruptible flow-starting mutations and cancel/teardown surfaces, forked callback pipelines, thin Effect.runPromise facades, shared toWireResult at facades). Existing tests pass unchanged.

Background

Phase 6b was deliberately split out of #4034 by the Phase 5/6a reports: coderOauthService.ts (~2,200 lines) carries correctness-critical finish/persist sequencing — a cancelled flow must never commit a replacement login — that must not be buried under mechanical churn. This PR converts the service while preserving that contract structurally rather than re-deriving it.

Implementation

• CoderOauthError (Schema.TaggedError, reason-carrying) + shared toWireResult fold at every facade; CoderTokenRequestResult stays a success-channel union because callers branch on invalidGrant.
• Effect surfaces for handlerGen: startDesktopFlowEffect (uninterruptible), waitForDesktopFlowEffect (interruptible delegate), cancelDesktopFlowEffect (uninterruptible), disconnectEffect (uninterruptible), refreshModelsEffect, getValidAuthEffect. Promise facades preserved 1:1 for pre-Effect callers.
• Finish/persist liveness contract preserved by construction: the cross-process lock critical sections (withCoderOauthLoginCommitLock / withCoderOauthRefreshLock) stay callback-owned Promise seams. commitDesktopLoginCrossProcess and rollbackPersistedAuth are byte-identical: the desktopFlows.has(flowId) liveness checks remain textually adjacent to the persist write (inside the locked write predicate) and to the finish calls, with no new awaits inserted between check and commit. The Effect layer wraps around the critical section (commitDesktopLoginLockedEffect owns the process-local mutex via acquireUseRelease and folds lock rejections into the flow-finishing "failed" outcome), never through it.
• getValidAuth: Effect.acquireUseRelease mutex pattern from 🤖 refactor: convert codex/governor/copilot OAuth services to Effect and adopt handlerGen for OAuth procedures #4034's codexOauthService; the refresh pipeline (refreshTokensEffect) ends in a whole-pipeline Effect.catchDefect fold, and getValidAuthEffect folds cross-process lock-acquisition defects, so the facade never rejects (previously a lock failure rejected).
• Forked pipelines: the desktop callback background task and the client-lease release/quarantine wiring are now Effect.runFork fibers mirroring desktopCallbackPipeline in muxGatewayOauthService/codexOauthService.
• Router: all 5 coderOauth procedures (startDesktopFlow, waitForDesktopFlow, cancelDesktopFlow, disconnect, refreshModels) now ride handlerGen, completing OAuth router coverage started in 🤖 refactor: convert codex/governor/copilot OAuth services to Effect and adopt handlerGen for OAuth procedures #4034.

Pre-review audits (the #4034 lessons, applied up front)

• Uninterruptible teardown end-to-end: disconnect, cancel, flow start, commit (persist → finish/rollback → revocation bookkeeping), plus manager-level finishEffect (already uninterruptible from 🤖 refactor: convert OAuthFlowManager flow lifecycle to Effect per-flow Scope #4033). waitForDesktopFlowEffect stays interruptible by design (template).
• Null-JSON guards: every response.json() is read via a caught tryPromise thunk typed Promise<unknown> and validated with isPlainObject/Array.isArray before any dereference.
• No defect escapes a facade where the pre-Effect contract was total: every method whose pre-Effect body was one big try/catch keeps an equivalent whole-pipeline fold (requestTokensEffect, revokeTokensEffect, quarantineStoredClientEffect, fetchGatewayProvidersEffect, fetchProviderCatalogEffect, validateDeploymentEffect, discoverEndpointsEffect, registerClientEffect, updateClientRedirectUriEffect — the last preserving the onUncertainOutcome side effect in its catch), and getValidAuth/refreshModels gained folds so those facades never reject.

Deliberate behavior deltas (rejection → wire Err)

Pre-Effect, getValidAuth()/refreshModels() rejected when cross-process lock acquisition or a config write threw; they now return Err(...) with the failure message. No test pinned the rejection behavior; all other wire behavior (URLs, request bodies, headers, error strings, persistence predicates) is unchanged.

Validation

• src/node/services/coderOauthService.test.ts: 88/88 pass unchanged — including the tests that pin the cancelled-flow no-commit contract ("does not persist tokens when the flow is cancelled during the exchange", "rolls back persisted credentials when cancelled during the persist write", "keeps the persisted login unrevoked when a post-persist cancel's rollback write fails", "disconnect cancels an in-flight re-login so it cannot commit afterwards", both overlapping-cancel snapshot tests). No new test added: the invariant was already pinned from multiple angles, and a new one would have been redundant.
• Sibling/consumer suites green: oauthFlowManager, oauthUtils, effectBridge, codexOauthService, muxGatewayOauthService, muxGovernorOauthService, copilotOauthService, mcpOauthService, providerModelFactory, coderService (426 tests).
• make static-check green.

Risks

Medium-touch conversion of a correctness-critical file. Highest-risk areas: (1) commit-path sequencing — mitigated by keeping the locked critical sections byte-identical Promise seams and relying on the extensive race-pinning test suite; (2) interruption semantics on handlerGen — mitigated by making every mutation/teardown surface uninterruptible and leaving only the wait surface interruptible; (3) Effect.runFork replacing void (async ...) for the callback/lease pipelines — same detached semantics, validated by the cancel/timeout/lease tests.

Lessons for Phase 7 (Config service: 19 router sites, file locks → Semaphore)

1. Wrap around locks, not through them. Callback-owned file-lock seams (withProvidersFileLock-style) convert cleanly by keeping the callback interior as a Promise seam and bridging with Effect.runPromise(toWireResult(...)) at the boundary; check-adjacent-to-write invariants survive verbatim. Replace a lock with an Effect Semaphore only when the acquire/release sides can both move into Effect in the same change — a half-converted lock is worse than a wrapped one.
2. One mutation surface per resource. AsyncMutex → Effect.acquireUseRelease transfers mechanically (acquire via Effect.promise, release via lock[Symbol.asyncDispose]()), one acquireUseRelease per resource, never combined. For Phase 7's config file locks this maps 1:1 onto a Semaphore.withPermits(1) shape later.
3. Classify methods by their pre-Effect catch discipline before converting. Total methods (whole-body try/catch) need whole-pipeline folds; partial methods keep defects as defects unless the facade contract says otherwise. Doing this classification up front (this PR) avoided the one-P2-per-round loop that hit 🤖 refactor: convert codex/governor/copilot OAuth services to Effect and adopt handlerGen for OAuth procedures #4034.
4. Budget for interruption review on every handlerGen adoption. With 19 router sites, enumerate which are mutations (uninterruptible), which are waits (interruptible), and which are reads (don't care) before converting; the decision is per-procedure, not per-service.
5. Generation counters + locked write predicates make interruption safe by default. Where Phase 7 mutations already use compare-and-swap predicates inside the file lock, an interrupted fiber can only cause a conservative refusal, never a torn write — same argument used here for refreshModels.

---

Generated with xum • Model: anthropic:claude-fable-5 • Thinking: xhigh

[ThomasK33]

@codex review

[ThomasK33]

@codex security review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 1b8ea31df6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".