refactor(#276): extract ConnectionSession — phase 2#279
Merged
Conversation
src/application/connection-session.ts now owns the auth + config + ClickHouse connection lifecycle: OAuth PKCE login/refresh, Basic-auth probing, IdP config resolution/memoization, identity (isSignedIn/email/ host), token storage, sign-in/out, ensureConfig/ensureFreshToken, and the cross-tab dashboard auth handoff (grant + receive, origin- and source-pinned). Constructible without App/AppState/DOM; rendering is inverted through the injected onAuthLost shell callback — the session never renders or toasts (issue rule: typed state/errors, the UI decides presentation). The default sign-out message stays byte-identical in the session deliberately (see the issue comment). app.ts wires it as app.conn and keeps the SAME single live chCtx object (aliased, never reconstructed — ch-client's authedFetch mutates authConfirmed in place and clearTokens/connectBasic/applyAuthSnapshot mutate origin). Scalar auth fields (token/refreshToken/authMode/chAuth/ basicUserClaim/idpId/selectIdp/chUsername) left the App contract — no view reads them; view/bootstrap-consumed members stay as one-line Phase-2 delegates (loadConfig included — main.ts's OAuth-callback path needs it), all slated for Phase-5 removal. main.ts, login.ts, dashboard.ts, results.ts, shortcuts.ts untouched. The isTokenExpired asymmetry is preserved on purpose: isSignedIn uses zero skew (display accuracy), getToken the default 60s buffer (refresh headroom). Review round: fake-app now mirrors the production chCtx identity invariant (app.chCtx === app.conn.chCtx, one shared object in makeApp); SessionStorageLike extends core/auth-handoff's StorageLike and SessionCrypto reuses core/pkce's now-exported PkceCrypto instead of hand-copies; the new spec injects node:crypto's webcrypto explicitly (no ambient-global fragility) and shares jwt()/memStorage() via tests/helpers/auth-fixtures.ts instead of adding a third copy. Security review (auth-focused): no findings — handoff pinning, key clears, PKCE one-shots, and header construction verified byte-identical to the original. Tests: 60-test connection-session spec at 100/95.5/100/100; ~17 assertion sites re-pointed to app.conn.*; gate green (104 files / 3000 tests), build green. Part of #276 (phase 2 of 5; roadmap #68 updated). Claude-Session: ad39bf19-1690-43c7-abb4-f0084fca00a2 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Part of #276 — Phase 2 of the app.ts → application-services refactor (design notes + considered deviations in the Phase-2 issue comment; PR 1 = #278, merged).
ConnectionSession(src/application/connection-session.ts, 60-test spec at 100/95.5/100/100) now owns the auth + connection lifecycle: OAuth PKCE login/refresh, Basic-auth probing, IdP config resolution/memoization, identity, token storage, sign-in/out,ensureConfig/ensureFreshToken, and both halves of the cross-tab dashboard auth handoff. Constructible withoutApp/AppState/DOM; rendering is inverted through an injectedonAuthLostshell callback — the session never renders or toasts.Key invariants preserved (each pinned in the issue comment and verified byte-identical by review):
chCtxobject —app.chCtxaliasesconn.chCtx;authedFetchmutatesauthConfirmedand sign-in paths mutateoriginin place, never reconstructed.isTokenExpiredasymmetry —isSignedIn()zero skew vsgetToken()60s buffer, deliberately not unified.hasAuthempty-grant rejection, PKCE verifier/state one-shot removal, superset key clear on sign-out, byte-identical error strings.connectBasiccommits without rendering; the shell action re-renders after the awaited probe (existing test ordering preserved).App contract: scalar auth fields (
token/refreshToken/authMode/chAuth/basicUserClaim/idpId/selectIdp/chUsername) removed — no view reads them; tests re-point toapp.conn.*(~17 sites). View/bootstrap-consumed members stay as one-line Phase-2 delegates (incl.loadConfig, whichmain.ts's OAuth callback requires), all marked for Phase-5 removal.main.ts,login.ts,dashboard.ts,results.ts,shortcuts.tsare untouched.app.types.ts'sChCtxbecame an alias of the session's type instead of a second hand-copied shape.Review rounds: 5-angle code review (correctness angles clean; fixes applied: fake-app now mirrors the production
chCtxidentity invariant,SessionStorageLike/SessionCryptoreuse the canonicalcoretypes —PkceCryptois now exported — the new spec injectsnode:crypto'swebcryptoexplicitly, andjwt()/memStorage()moved to a sharedtests/helpers/auth-fixtures.ts). Security review: no findings.Verification: full gate green (104 files / 3,000 tests + tsc +
check:arch), build green (dist/sql.html).Acceptance criteria this PR completes: authentication and ClickHouse connection lifecycle live in
ConnectionSessionwithout rendering dependencies ✅; application services do not accept the fullAppobject ✅ (cumulative with #278).Checklist
npm testpasses (the per-file coverage gate is non-negotiable)npm run buildsucceeds (single-filedist/sql.html)src/core/, network insrc/net/(injected fetch), DOM insrc/ui/CHANGELOG.md([Unreleased]) updated if behavior or the deployed surface changed🤖 Generated with Claude Code