-
Notifications
You must be signed in to change notification settings - Fork 1
refactor(auth): extract auth strategies into separate modules #737
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Refactors core auth initialization into a strategy-pattern layout, separating Studio/Dashboard/Standalone auth mode logic while keeping authStore as the orchestrator.
Changes:
- Added
resolveAuthMode()to determine runtime auth mode and route to the correct strategy. - Extracted per-mode initial-state + initialization logic into
studioAuth.ts,dashboardAuth.ts, andstandaloneAuth.ts. - Updated
subscribeToStateAndFetchCurrentUserto accept an explicituseProjectHostnameoption (with backward-compatible defaulting).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/auth/authStore.ts | Converts store into a thin orchestrator that selects a mode and delegates to strategy modules. |
| packages/core/src/auth/authMode.ts | Adds auth mode resolution (studio / dashboard / standalone) based on config + _context. |
| packages/core/src/auth/authStrategy.ts | Introduces shared strategy input/output types used by the orchestrator and strategies. |
| packages/core/src/auth/studioAuth.ts | Implements Studio strategy: studio localStorage token discovery + async cookie auth enablement + subscriptions. |
| packages/core/src/auth/dashboardAuth.ts | Implements Dashboard strategy: _context parsing + “wait for Comlink token” initial state + subscriptions. |
| packages/core/src/auth/standaloneAuth.ts | Implements Standalone strategy: OAuth callback detection + localStorage token discovery + subscriptions. |
| packages/core/src/auth/subscribeToStateAndFetchCurrentUser.ts | Adds optional fetch configuration to control project-hostname behavior (important for Studio cookie auth). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
This is a code organization change only — no behavioral changes to auth.
Refactors the auth system into a strategy pattern, extracting the monolithic
authStore.getInitialState/initializeinto three dedicated strategy modules — one per auth mode.Why: The previous implementation tangled Studio, Dashboard, and Standalone auth logic together in a single function with interleaved conditionals, making it difficult to reason about each mode independently. This refactor establishes clear code paths as a prerequisite for upcoming Studio auto-detection work.
What changed:
authMode.ts—resolveAuthMode()determines the active auth mode (studio,dashboard,standalone) from config and environmentauthStrategy.ts— shared types (AuthStrategyResult,AuthStrategyOptions) defining the contract between orchestrator and strategiesstudioAuth.ts— Studio strategy: localStorage token discovery, async cookie auth fallback, project-scoped hostnamedashboardAuth.ts— Dashboard strategy:_contextURL param parsing, no localStorage (token via Comlink)standaloneAuth.ts— Standalone strategy: OAuth callback handling, localStorage token, standard login flowauthStore.tsis now a thin orchestrator that delegates to the resolved strategysubscribeToStateAndFetchCurrentUser.tsaccepts an explicituseProjectHostnameoption instead of re-readingstudioMode.enabledAll public exports from
authStore.tsare unchanged.studioModeAuth.tsretains its original leaf functions — existing test mocks continue to work through the module boundary.Relates to SDK-772
What to review
authMode.ts— Is the mode resolution logic correct? Studio config wins, then dashboard detection, then standalone fallback.getXxxInitialStatematches the original behavior for that mode. Compare against the previousgetInitialStateconditionals.authStore.ts— Confirm the orchestrator correctly wires strategy results intoAuthStoreStateand theinitializefunction delegates properly.subscribeToStateAndFetchCurrentUser.ts— The new optionalfetchOptionsparameter defaults to the previous behavior when omitted (backwards compatible)._exports/index.tsre-exports are untouched.Testing
All existing auth tests pass with minimal changes. The existing 101 core auth tests and 40 React auth tests serve as the regression safety net:
authStore.test.ts(24 tests) — coversgetInitialStatefor all modes,initializesubscription lifecycle, and all bound actions.studioModeAuth.test.ts(10 tests) — coverscheckForCookieAuthandgetStudioTokenFromLocalStoragesubscribeToStateAndFetchCurrentUser.test.ts(3 tests) — covers user fetch on state transitionssubscribeToStorageEventsAndSetToken.test.ts(2 tests) — covers storage event handlinghandleAuthCallback,logout,refreshStampedToken,utils, and all React auth component/hook testsNo new test files were added — the refactor is purely structural and the existing tests exercise all code paths through the new module boundaries via vitest's module mocking.
Fun gif