🤖 refactor: convert OAuthFlowManager flow lifecycle to Effect per-flow Scope

[ThomasK33]

Summary

Phase 5 of the progressive Effect migration (first phase of Wave 2, the wave's gating item deferred since #4027): converts OAuthFlowManager internals to Effect with real resource safety. Every registered desktop OAuth flow now owns a per-flow Scope whose release finalizers guarantee cleanup (registration-timeout clear, deferred settlement, loopback-server close) on every termination path — finish, cancel, caller-timeout race, duplicate registration, shutdownAll, and defects. The Promise-based public API is preserved as thin Effect.runPromise facades, so the three not-yet-converted OAuth services (coderOauthService, codexOauthService, muxGovernorOauthService) and all existing tests work unchanged.

Background

Wave 1 (#4022, #4025, #4027, #4028, #4030, #4031, #4032) established the house pattern: Effect.gen internals, thin runPromise facades, handlerGen for oRPC procedures. #4027 converted muxGatewayOauthService but explicitly deferred the shared flow-lifecycle manager: its resources (loopback http.Server, registration setTimeout, result deferred) were cleaned up via ad-hoc try/catch + fire-and-forget void closeServer(...), and a defect while resolving the deferred silently skipped the server close. This PR is the acquire/release case that deferral pointed at, and unblocks Phase 6 (batch conversion of the sibling OAuth services).

Implementation

Per-flow Scope design — register creates a Scope.makeUnsafe() per flow and moves ownership of the caller-acquired resources into it via one Effect.acquireRelease per resource (a combined acquisition would install its finalizer only after every step succeeded, leaking earlier resources on a later defect — the #4031 Codex P2 lesson). Release runs in reverse acquisition order, preserving the pre-Effect finish ordering: clear registration timeout → settle deferred (waiters unblock before the async close) → close loopback server (awaited).

Deferred settlement via finalizer — each ActiveFlow carries a mutable finalResult staged by the terminating path (finish/cancel/shutdown/replace); the settle finalizer resolves the caller's deferred with it. Settlement is therefore scope-guaranteed rather than an ad-hoc resolve call, with a defensive fallback result so waiters can never hang.

Caller-facing timeout race — waitFor maps to Effect.timeout over Effect.promise on the shared deferred: the local wait timer is fiber-managed (interruption clears it), stays separate from the registration-time timeout, and on any error result runs finish for shared cleanup. The cleanup's synchronous bookkeeping (map removal, completed-result recording) runs before waitFor resolves — exact parity with the old sync prefix — while the async release runs in an Effect.forkDetach fiber, replacing the old void this.finish(...) fire-and-forget with a supervised fiber that survives the caller's completion (verified by a live-runtime probe: detached fibers outlive the parent, runFork/runPromise execute synchronously to first suspension, and a throwing finalizer does not skip its siblings).

shutdownAll contract — preserved as async (Promise<void> facade): serviceContainer.dispose awaits it, and loopback-server closes are bounded by the server's force-finish socket handling. It never rejects; release defects are caught (Effect.catchDefect) and logged at debug level, per the startup/shutdown-must-never-crash rule.

Effect-native surface — waitForEffect / cancelEffect / finishEffect / cancelAllEffect / shutdownAllEffect are public (wire-shaped, never-failing — same shape as #4032's Effect surfaces). muxGatewayOauthService's Effect pipeline now yields finishEffect directly instead of Effect.promise(() => …finish(...)), and its registration-timeout callback uses Effect.runFork(finishEffect(...)) instead of void finish(...).

Not converted to Effect Deferred — the result deferred's identity is part of the public caller-owned OAuthFlowEntry (the three unconverted services construct entries with createDeferred), so swapping it would break the "existing callers unchanged" contract; revisit when Phase 6 converts entry construction.

Validation

• All 18 pre-existing oauthFlowManager tests pass byte-identical, plus all OAuth service suites (194 tests: coder/codex/muxGateway/muxGovernor/mcp/copilot/codexOauthAuth) and loopback-server/oauthUtils suites.
• Two new behavioral tests for the genuinely-new guarantees: (1) server close + timeout clear still happen when the deferred resolve throws (the pre-Effect code skipped the close — this test fails on the old implementation), and (2) the detached cleanup fiber completes after waitFor has already returned on the timeout path (guards against accidental child-fiber supervision, where the release would be interrupted with the caller).
• A standalone Effect v4 runtime probe validated the semantics the design relies on (finalizer independence under defects, reverse sequential release order, eager sync-prefix execution of runPromise/runFork, forkDetach outliving the parent, Effect.timeout + Effect.catch over Effect.promise).
• make static-check green.

Risks

Low-to-moderate: this is shared lifecycle code under four OAuth login flows (Gateway, Governor, Codex, Coder). The public API, observable ordering (map removal before finish resolves, deferred settlement before server close, synchronous register), and error strings are preserved exactly; regressions would surface as leaked loopback listeners, hung waitFor calls, or unsettled deferreds — all covered by the existing + new suites.

Lessons for Phase 6

Phase 6 is the batch conversion of coderOauthService, codexOauthService, muxGovernorOauthService, copilotOauthService, plus their ~20 router sites. Notes to make it mechanical:

• The manager now exposes never-failing, wire-shaped waitForEffect/cancelEffect/finishEffect/shutdownAllEffect, so converted service pipelines can yield them directly (see desktopCallbackPipeline in muxGatewayOauthService as the template), and registration-timeout callbacks should use Effect.runFork(manager.finishEffect(...)).
• beginFinish's sync-bookkeeping/async-release split is the pattern to reach for wherever a service needs "unregister now, release in background" semantics.
• Each sibling's startDesktopFlow should become uninterruptible like the gateway's (🤖 refactor: adopt handlerGen as the oRPC router default and convert gateway OAuth procedures #4032): a client abort between loopback acquisition and register would otherwise leak the server.
• coderOauthService is the outlier: it has extra commit-path liveness checks (has) and multi-step persist/commit finish calls (~10 desktopFlows.* sites vs ~5 in the others) — expect most of the Phase 6 effort there.
• Recommendation: two PRs. PR A: codex + governor + copilot service internals (near-identical DesktopFlow shape, mechanical) together with their router procedures moving to handlerGen (the waitFor/cancel handlers for the gateway can join here — the router comment at muxGatewayOauth already points at this). PR B: coderOauthService alone — its commit/persist liveness semantics deserve isolated review, and a combined PR would bury it under the mechanical churn.

---

Generated with mux • 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. 👍

Reviewed commit: dc2ce382ae

ℹ️ 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".