From 2b69d1ab915a47c6da5b0ff0ff73a3368c7b8ff6 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 28 Jul 2026 15:45:21 +0200 Subject: [PATCH 1/4] feat(#512): add epoch-fenced connection lifecycle --- CHANGELOG.md | 9 + docs/ARCHITECTURE.md | 17 +- src/application/connection-session.ts | 210 +++++++++++++++++----- src/application/schema-catalog-service.ts | 19 +- src/core/connection-lifecycle.ts | 189 +++++++++++++++++++ src/net/ch-client.ts | 58 ++++-- src/net/oauth.ts | 36 ++-- src/styles.css | 12 +- src/ui/app-header.ts | 25 ++- src/ui/app-shell.ts | 14 +- src/ui/app.ts | 28 +-- tests/helpers/fake-app.ts | 3 + tests/unit/app.test.ts | 31 +++- tests/unit/ch-client.test.ts | 174 ++++++++++++++++++ tests/unit/connection-lifecycle.test.ts | 114 ++++++++++++ tests/unit/connection-session.test.ts | 203 ++++++++++++++++++++- tests/unit/oauth.test.ts | 4 +- tests/unit/schema-catalog-service.test.ts | 17 +- 18 files changed, 1028 insertions(+), 135 deletions(-) create mode 100644 src/core/connection-lifecycle.ts create mode 100644 tests/unit/connection-lifecycle.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 59558382..cbb30cf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,15 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] ### Added +- **Connection state now has one explicit, epoch-fenced lifecycle** (#512 + phase 1). OAuth/Basic credentials, single-flight token refresh, transport + success/failure, auth loss, reauthentication, and explicit sign-out flow + through `ConnectionSession`; stale refreshes cannot publish tokens or + lifecycle state into a newer login. The accessible header chip reacts to + that lifecycle (green connected, amber offline, red auth-required) instead + of inferring connectivity from the best-effort server-version probe. HTTP + query failures remain query errors and do not falsely mark the transport + offline. - **A Dashboard row can create a blank Panel directly from the Dashboards tree** (#515). Its new plus action opens the shared name/description dialog, then atomically commits one empty panel-role query and one canonically laid diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8a4fe28a..ac7f5dd2 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -48,7 +48,7 @@ module is tested with plain stubs at the per-file coverage gate. | Module | Owns | |---|---| | `query-execution-service` (`app.exec`) | the shared request/stream/normalize read core + the script transport loop (retry classification, stop-on-first-failure, per-attempt `query_id`); stateless `kill(queryId)` — cancellation is caller-owned (`AbortController`s live with the owning session) | -| `connection-session` (`app.conn`) | auth + connection lifecycle: OAuth PKCE login/refresh, Basic probing, IdP config, identity, token storage, sign-out, and **the single live `chCtx` object** (mutated in place — `authConfirmed` by `net/ch-client`, `origin` by sign-in — never reconstructed) | +| `connection-session` (`app.conn`) | authoritative auth + connection lifecycle (`starting` / `connected` / `refreshing` / `offline` / `auth-required` / `reauthenticating` / `signed-out`), OAuth PKCE login/refresh, Basic probing, IdP config, identity, token storage, sign-out, and **the single live `chCtx` object** (mutated in place — `authConfirmed` by `net/ch-client`, `origin` by sign-in — never reconstructed) | | `schema-catalog-service` (`app.catalog`) | server version, schema tree, lazy columns, SQL reference/completions, entity-doc cache, `invalidate()` | | `workbench-parameter-session` (`app.params`) | `{name:Type}` analysis/prepare/gate policy, input-vs-execute hardening, enum inference, recent values; reads the live shared `AppState` slices through accessors | | `export-service` (`app.exports`) | direct + script export behind an injectable `ExportSink` (`pickFile`/`pickDirectory`); hold-back exception inspection, `.partial` semantics, its own cancellation state | @@ -92,6 +92,17 @@ invalidates the catalog before the login screen renders. There is no route-remount mechanism — the dashboard is its own browser tab whose closure JS never observes — so no teardown theater beyond that. +Connection lifecycle ownership follows the same rule. `ConnectionSession` +publishes one read-only signal and assigns a monotonically increasing +credential epoch whenever credentials are installed or invalidated. Refresh is +single-flight within an epoch; a late refresh or transport response cannot +write tokens, report auth loss, or repaint the replacement epoch. +`net/ch-client` reports only successful 2xx transport settlement as connected +and rejected, non-aborted `fetch` as offline. HTTP query failures — including a +post-confirmation 401/403 — remain query outcomes, not connection state. The +header chip is a pure projection of this lifecycle; `serverVersion` remains +display metadata and is never connection authority. + ## The `App` object `createApp(env)` still returns one `app` object, but it is now a composition @@ -112,7 +123,9 @@ test seam the parameter session reads live). Unchanged from the beginning and now applied uniformly: every side effect is passed in, never imported — `createApp(env)` injects `document/window/location/fetch/crypto/sessionStorage`, `ch-client` functions -take a `ctx = {fetch, origin, getToken, refresh, authHeader, onSignedOut}`, +take a `ctx = {fetch, origin, getToken, refresh, authHeader, +onSignedOut(detail?, expectedEpoch?), currentEpoch?, onTransportConnected?, +onTransportOffline?}`, and every `create*Service(deps)` receives its transport/clock/uid/storage explicitly. The suite needs no network/DOM mocking libraries — plain stubs suffice, and coverage is genuine. diff --git a/src/application/connection-session.ts b/src/application/connection-session.ts index 185608fa..44d9e00e 100644 --- a/src/application/connection-session.ts +++ b/src/application/connection-session.ts @@ -12,8 +12,16 @@ // this module's job any more — see each call site below for the specific note). import { decodeJwtPayload, isTokenExpired } from '../core/jwt.js'; +import { + reduceConnectionLifecycle, + type AuthenticationPriorState, + type ConnectionLifecycleEvent, + type ConnectionLifecycleState, +} from '../core/connection-lifecycle.js'; import { generatePKCE, randomState } from '../core/pkce.js'; import type { PkceCrypto } from '../core/pkce.js'; +import { signal } from '@preact/signals-core'; +import type { ReadonlySignal } from '@preact/signals-core'; import { resolveTarget } from '../core/target.js'; import { buildAuthorizeUrl, refreshTokens, bearerFromTokens } from '../net/oauth.js'; import { memoizeConfig, loadConfigDoc, resolveIdp } from '../net/oauth-config.js'; @@ -74,7 +82,10 @@ export interface SessionChCtx { getToken(): Promise; refresh(): Promise; authHeader(token: string): string; - onSignedOut(detail?: string): void; + onSignedOut(detail?: string, expectedEpoch?: number): void; + currentEpoch(): number; + onTransportConnected(): void; + onTransportOffline(error?: unknown): void; } // ── The session ─────────────────────────────────────────────────────────── @@ -86,6 +97,9 @@ export interface ConnectionSession { readonly basePath: string; readonly hostHint: string; readonly chCtx: SessionChCtx; + /** Authoritative connection/auth lifecycle. Consumers may observe but never + * mutate it; all transitions flow through this session. */ + readonly connection: ReadonlySignal; // state accessors (test-support + shell display; do not log values) token(): string | null; @@ -125,8 +139,18 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection let refreshTok: string | null = ss.getItem('oauth_refresh_token'); let authMode: 'oauth' | 'basic' = ss.getItem('ch_basic_auth') ? 'basic' : 'oauth'; const basicCreds = (): string | null => ss.getItem('ch_basic_auth'); + const currentCredential = (): string | null => (authMode === 'basic' ? basicCreds() : token); const basicUser = (): string => ss.getItem('ch_basic_user') || ''; const originHost = (o: string): string => { try { return new URL(o).host; } catch { return ''; } }; + const hasRestoredCredentials = authMode === 'basic' ? !!basicCreds() : !!token; + const connectionSignal = signal( + hasRestoredCredentials ? { kind: 'starting', epoch: 0 } : { kind: 'signed-out', epoch: 0 }, + ); + const transition = (event: ConnectionLifecycleEvent): ConnectionLifecycleState => { + const next = reduceConnectionLifecycle(connectionSignal.value, event); + if (next !== connectionSignal.value) connectionSignal.value = next; + return next; + }; // config.json may list several IdPs. Fetch the doc once; resolve OIDC // discovery per selected IdP. The chosen IdP id is persisted so it survives @@ -171,7 +195,7 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection ? basicUser() : chUsername(decodeJwtPayload(token))); - function setTokens(id: string, refresh?: string): void { + function storeTokens(id: string, refresh?: string): void { token = id; ss.setItem('oauth_id_token', id); if (refresh) { @@ -183,6 +207,12 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection ss.removeItem('oauth_verifier'); ss.removeItem('oauth_state'); } + function setTokens(id: string, refresh?: string): void { + authMode = 'oauth'; + storeTokens(id, refresh); + chCtx.authConfirmed = false; + transition({ type: 'credentials-installed' }); + } function clearTokens(): void { token = null; refreshTok = null; @@ -196,46 +226,123 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection // `signOut` is exactly today's app.ts `clearTokens()` — no render. (app.ts's // own `app.signOut` additionally re-renders the login screen; that's the // shell's job now, done by whatever caller notices signOut() was called.) - const signOut = (): void => clearTokens(); + const signOut = (): void => { + transition({ type: 'signed-out' }); + clearTokens(); + }; + + function authenticationPrior(): AuthenticationPriorState { + const current = connectionSignal.value; + if (current.kind === 'auth-required' || current.kind === 'signed-out') return current; + // Interactive authentication is entered from the login surface today. + // Keep this fail-safe explicit in case a future caller opens it elsewhere. + return { kind: 'signed-out', epoch: current.epoch }; + } + function assertCurrentAuthentication(epoch: number): void { + const current = connectionSignal.value; + if (current.epoch !== epoch || current.kind !== 'reauthenticating') { + throw new Error('Authentication attempt superseded'); + } + } // --- OAuth ------------------------------------------------------------- async function beginOAuth(idpArg?: string, targetOrigin?: string): Promise { - if (idpArg) selectIdp(idpArg); - // A picked saved-connection can target another cluster: stash its origin so - // the rebuilt chCtx (after the redirect reload) POSTs the bearer there. - // Survives the redirect like oauth_state/oauth_idp; cleared for serving-host SSO. - if (targetOrigin) ss.setItem('oauth_origin', targetOrigin); - else ss.removeItem('oauth_origin'); - const cfg = await resolveConfig(); - const { verifier, challenge } = await generatePKCE(cryptoObj); - const state = randomState(cryptoObj); - ss.setItem('oauth_verifier', verifier); - ss.setItem('oauth_state', state); - const returnParams = new URLSearchParams(loc.search); - ['code', 'state', 'scope', 'authuser', 'prompt', 'error', 'error_description', 'error_uri'] - .forEach((key) => returnParams.delete(key)); - const returnSearch = returnParams.toString(); - ss.setItem('oauth_return_route', JSON.stringify({ - state, search: returnSearch ? `?${returnSearch}` : '', - })); - loc.href = buildAuthorizeUrl(cfg, { - redirectUri: loc.origin + basePath, - challenge, - state, - }); + const prior = authenticationPrior(); + const authenticating = transition({ type: 'start-authentication' }); + try { + if (idpArg) selectIdp(idpArg); + // A picked saved-connection can target another cluster: stash its origin so + // the rebuilt chCtx (after the redirect reload) POSTs the bearer there. + // Survives the redirect like oauth_state/oauth_idp; cleared for serving-host SSO. + if (targetOrigin) ss.setItem('oauth_origin', targetOrigin); + else ss.removeItem('oauth_origin'); + const cfg = await resolveConfig(); + assertCurrentAuthentication(authenticating.epoch); + const { verifier, challenge } = await generatePKCE(cryptoObj); + assertCurrentAuthentication(authenticating.epoch); + const state = randomState(cryptoObj); + ss.setItem('oauth_verifier', verifier); + ss.setItem('oauth_state', state); + const returnParams = new URLSearchParams(loc.search); + ['code', 'state', 'scope', 'authuser', 'prompt', 'error', 'error_description', 'error_uri'] + .forEach((key) => returnParams.delete(key)); + const returnSearch = returnParams.toString(); + ss.setItem('oauth_return_route', JSON.stringify({ + state, search: returnSearch ? `?${returnSearch}` : '', + })); + loc.href = buildAuthorizeUrl(cfg, { + redirectUri: loc.origin + basePath, + challenge, + state, + }); + } catch (error) { + transition({ type: 'failed-authentication', epoch: authenticating.epoch, prior }); + throw error; + } } - async function refresh(): Promise { + let refreshSlot: { epoch: number; promise: Promise } | null = null; + let authLossReportedEpoch: number | null = null; + + function requireAuthentication(expectedEpoch: number, detail?: string): void { + const current = connectionSignal.value; + const message = detail || 'Your session expired — please sign in again.'; + if (current.epoch !== expectedEpoch) return; + if (current.kind === 'auth-required' || current.kind === 'signed-out') { + // An unexpected operation can discover an already-empty session. Keep + // explicit signed-out state intact, but still let the shell reveal its + // login surface once for this epoch. + if (authLossReportedEpoch !== current.epoch) { + authLossReportedEpoch = current.epoch; + deps.onAuthLost(message); + } + return; + } + const next = transition({ type: 'auth-required', epoch: expectedEpoch, detail: message }); + clearTokens(); + authLossReportedEpoch = next.epoch; + deps.onAuthLost(message); + } + + function refresh(): Promise { // Basic credentials don't expire and can't be refreshed; a surviving 401 // means the password is wrong → authedFetch falls through to onSignedOut. - if (authMode === 'basic') return false; - const cfg = await resolveConfig(); - const tokens = await refreshTokens(fetchFn, cfg, refreshTok); - const bearer = bearerFromTokens(tokens, cfg.bearer); - if (!bearer) return false; - // `bearer` is only ever truthy when `tokens` (its own source) is non-null. - setTokens(bearer, tokens?.refresh_token); - return true; + if (authMode === 'basic') return Promise.resolve(false); + const epoch = connectionSignal.value.epoch; + if (refreshSlot?.epoch === epoch) return refreshSlot.promise; + + transition({ type: 'begin-refresh', epoch }); + let promise!: Promise; + promise = (async () => { + try { + const cfg = await resolveConfig(); + const tokens = await refreshTokens(fetchFn, cfg, refreshTok); + const bearer = bearerFromTokens(tokens, cfg.bearer); + // A newer credential scope owns the session now. This settlement may + // neither write tokens nor publish lifecycle state. + if (connectionSignal.value.epoch !== epoch) return false; + if (!bearer) { + requireAuthentication(epoch); + return false; + } + // A same-session refresh does not create a new epoch. + storeTokens(bearer, tokens?.refresh_token); + transition({ type: 'refresh-succeeded', epoch }); + return true; + } catch (error) { + // Config/discovery failure is not evidence that credentials are bad. + // Preserve them and expose a recoverable connectivity state. + if (connectionSignal.value.epoch === epoch) { + transition({ type: 'transport-offline', epoch, detail: 'Unable to refresh session' }); + } + throw error; + } + })().finally(() => { + // A late old finally must not erase a newer epoch's refresh slot. + if (refreshSlot?.promise === promise) refreshSlot = null; + }); + refreshSlot = { epoch, promise }; + return promise; } async function getToken(): Promise { @@ -243,9 +350,11 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection if (authMode === 'basic') return basicCreds(); if (!token) return null; if (!isTokenExpired(token)) return token; + const epoch = connectionSignal.value.epoch; if (await refresh()) return token; - clearTokens(); - return null; + // An interactive sign-in may have superseded this refresh while it was in + // flight. Return the current credential; never invalidate the newer scope. + return connectionSignal.value.epoch !== epoch ? currentCredential() : null; } // --- ClickHouse context -------------------------------------------------- @@ -276,11 +385,19 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection getToken, refresh, authHeader, + currentEpoch: () => connectionSignal.value.epoch, + onTransportConnected: () => { + const epoch = connectionSignal.value.epoch; + transition({ type: 'transport-connected', epoch }); + }, + onTransportOffline: () => { + const epoch = connectionSignal.value.epoch; + transition({ type: 'transport-offline', epoch, detail: 'Network unavailable' }); + }, // detail is set when CH rejects a *valid* login (authorization denial); the // no-arg calls (no token / expired + refresh failed) fall back to expiry. - onSignedOut: (detail?: string) => { - clearTokens(); - deps.onAuthLost(detail || 'Your session expired — please sign in again.'); + onSignedOut: (detail?: string, expectedEpoch?: number) => { + requireAuthentication(expectedEpoch ?? connectionSignal.value.epoch, detail); }, }; @@ -308,6 +425,8 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection // throwaway ctx so a bad password surfaces CH's own reason here (rejected as // a thrown Error) instead of auto-triggering onAuthLost. async function connectBasic({ username, password, host }: { username: string; password: string; host?: string }): Promise { + const prior = authenticationPrior(); + const authenticating = transition({ type: 'start-authentication' }); const user = String(username || '').trim(); const target = resolveTarget(host, loc.origin); const creds = btoa(unescape(encodeURIComponent(user + ':' + (password || '')))); @@ -319,13 +438,21 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection refresh: async () => false, onSignedOut: (detail?: string) => { throw new Error(detail || 'Authentication failed'); }, }; - await queryJsonFn(probeCtx, 'SELECT 1'); + try { + await queryJsonFn(probeCtx, 'SELECT 1'); + assertCurrentAuthentication(authenticating.epoch); + } catch (error) { + transition({ type: 'failed-authentication', epoch: authenticating.epoch, prior }); + throw error; + } // Probe passed → commit the session and switch the live ctx to the target. authMode = 'basic'; ss.setItem('ch_basic_auth', creds); ss.setItem('ch_basic_user', user); ss.setItem('ch_basic_origin', target); chCtx.origin = target; + chCtx.authConfirmed = false; + transition({ type: 'credentials-installed' }); } // --- dashboard (#149 D1) ------------------------------------------------- @@ -343,6 +470,7 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection basePath, hostHint, chCtx, + connection: connectionSignal as ReadonlySignal, token: () => token, refreshToken: () => refreshTok, authMode: () => authMode, diff --git a/src/application/schema-catalog-service.ts b/src/application/schema-catalog-service.ts index 2da76b1a..84c01ddd 100644 --- a/src/application/schema-catalog-service.ts +++ b/src/application/schema-catalog-service.ts @@ -9,8 +9,8 @@ // exact error handling (loadSchema's catch → `schemaError`), the // version-string handling, and the completions-rebuild trigger points. No // imports from `src/ui/**` or `src/editor/**` (a pretest check enforces -// this) — DOM (`setConn`'s `app.dom.connStatus` write, the schemaError-driven -// banner effect) stays in app.ts, driven by the same signals/hooks as today. +// this). Connection status is owned by ConnectionSession rather than inferred +// from this service's best-effort version probe. // // #313 Phase 1 deleted the old function-name-only `entityDoc(name)` hover-doc // seam (and its `docCache`) once the CM6 adapter's last caller moved onto the @@ -91,12 +91,9 @@ export interface SchemaCatalogStateSlice { // ── Injected DOM/render hooks (used INSIDE the moved bodies) ──────────────── export interface SchemaCatalogHooks { - /** Fired synchronously right after loadVersion's probe settles (success or - * failure) — the shell's own `setConn(online)` DOM update (app.ts, - * NOT moved here: it touches `app.dom.connStatus`/`app.state.serverVersion` - * display formatting, not the catalog). Optional so a caller that doesn't - * care about connection-status chrome (e.g. a test harness) can omit it. */ - onConnStatusChanged?(online: boolean): void; + /** Display-only notification after the best-effort version probe succeeds. + * It must never be interpreted as connection authority. */ + onServerVersionLoaded?(version: string): void; /** loadColumns' completion hook — app.ts's `app.renderVarStrip()` (the #172 * v2 upgrade path: a newly-loaded column may resolve a String var's * schema-cache-inferred enum suggestion; renderVarStrip's own signature @@ -651,10 +648,8 @@ export function createSchemaCatalogService(deps: SchemaCatalogDeps): SchemaCatal try { await deps.ensureConfig(); state.serverVersion = await deps.loadServerVersion(deps.ctx()); - hooks.onConnStatusChanged?.(true); - } catch { - hooks.onConnStatusChanged?.(false); - } + hooks.onServerVersionLoaded?.(state.serverVersion); + } catch { /* Best-effort metadata; ch-client reports transport lifecycle. */ } } async function loadSchemaImpl(): Promise { diff --git a/src/core/connection-lifecycle.ts b/src/core/connection-lifecycle.ts new file mode 100644 index 00000000..943e940c --- /dev/null +++ b/src/core/connection-lifecycle.ts @@ -0,0 +1,189 @@ +// The connection lifecycle is deliberately a small, pure state machine. The +// application/session layer owns effects (OAuth, storage and transport); it +// feeds their results here with the epoch captured when the work started. +// Epochs make a late response harmless after a newer login, credential change, +// or sign-out has superseded it. + +/** Every lifecycle state that can be resumed after a token refresh. */ +export type ConnectionLifecycleResumeState = + | ConnectionLifecycleStableState<'connected'> + | ConnectionLifecycleStableState<'starting'> + | ConnectionLifecycleStableState<'offline'>; + +type ConnectionLifecycleStableKind = + | 'starting' + | 'connected' + | 'offline' + | 'auth-required' + | 'reauthenticating' + | 'signed-out'; + +/** A non-refreshing lifecycle state. `detail` is deliberately display-safe + * context (for example, an authentication explanation), never credentials. */ +export interface ConnectionLifecycleStableState { + kind: K; + epoch: number; + detail?: string; +} + +/** Refreshing retains the exact pre-refresh state, so a successful refresh + * resumes an in-progress startup or an offline state rather than claiming the + * transport is connected. */ +export interface ConnectionLifecycleRefreshingState { + kind: 'refreshing'; + epoch: number; + detail?: string; + resume: ConnectionLifecycleResumeState; +} + +/** The complete, discriminated connection lifecycle value. */ +export type ConnectionLifecycleState = + | ConnectionLifecycleStableState<'starting'> + | ConnectionLifecycleStableState<'connected'> + | ConnectionLifecycleRefreshingState + | ConnectionLifecycleStableState<'offline'> + | ConnectionLifecycleStableState<'auth-required'> + | ConnectionLifecycleStableState<'reauthenticating'> + | ConnectionLifecycleStableState<'signed-out'>; + +/** The safe state an unsuccessful interactive authentication may restore. */ +export type AuthenticationPriorState = + | ConnectionLifecycleStableState<'auth-required'> + | ConnectionLifecycleStableState<'signed-out'>; + +type CurrentEpochEvent = { epoch: number; detail?: string }; + +/** Inputs to {@link reduceConnectionLifecycle}. Events that complete async + * work carry its captured epoch. New user/session intents do not: they create + * a new epoch themselves. */ +export type ConnectionLifecycleEvent = + | { type: 'start-authentication'; detail?: string } + | { type: 'credentials-installed'; detail?: string } + | ({ type: 'begin-refresh' } & CurrentEpochEvent) + | ({ type: 'refresh-succeeded' } & CurrentEpochEvent) + | ({ type: 'transport-connected' } & CurrentEpochEvent) + | ({ type: 'transport-offline' } & CurrentEpochEvent) + | ({ type: 'auth-required' } & CurrentEpochEvent) + | { type: 'signed-out'; detail?: string } + | ({ type: 'failed-authentication'; prior: AuthenticationPriorState } & CurrentEpochEvent); + +/** Start a session before credentials or a transport result are available. */ +export function initialConnectionLifecycle(detail?: string): ConnectionLifecycleStableState<'starting'> { + return stable('starting', 0, detail); +} + +function stable( + kind: K, epoch: number, detail?: string, +): ConnectionLifecycleStableState { + return detail === undefined ? { kind, epoch } : { kind, epoch, detail }; +} + +function isRefreshable(state: ConnectionLifecycleState): state is ConnectionLifecycleResumeState { + return state.kind === 'connected' || state.kind === 'starting' || state.kind === 'offline'; +} + +function isCurrent(state: ConnectionLifecycleState, event: CurrentEpochEvent): boolean { + return state.epoch === event.epoch; +} + +/** + * Reduce one lifecycle event. Illegal transitions and stale async completions + * return `state` itself, allowing callers to skip rendering/persistence by + * identity. The reducer never mutates its input. + */ +export function reduceConnectionLifecycle( + state: ConnectionLifecycleState, + event: ConnectionLifecycleEvent, +): ConnectionLifecycleState { + switch (event.type) { + case 'start-authentication': + return stable('reauthenticating', state.epoch + 1, event.detail); + + case 'credentials-installed': + return stable('starting', state.epoch + 1, event.detail); + + case 'signed-out': + return stable('signed-out', state.epoch + 1, event.detail); + + case 'begin-refresh': + if (!isCurrent(state, event) || !isRefreshable(state)) return state; + return event.detail === undefined + ? { kind: 'refreshing', epoch: state.epoch, resume: state } + : { kind: 'refreshing', epoch: state.epoch, detail: event.detail, resume: state }; + + case 'refresh-succeeded': + if (!isCurrent(state, event) || state.kind !== 'refreshing') return state; + return state.resume; + + case 'transport-connected': + if (!isCurrent(state, event) || (state.kind !== 'starting' && state.kind !== 'offline')) return state; + return stable('connected', state.epoch, event.detail); + + case 'transport-offline': + if (!isCurrent(state, event) + || state.kind === 'offline' + || state.kind === 'auth-required' + || state.kind === 'signed-out' + || state.kind === 'reauthenticating') return state; + return stable('offline', state.epoch, event.detail); + + case 'auth-required': + if (!isCurrent(state, event) || state.kind === 'auth-required' || state.kind === 'signed-out') return state; + // An authentication requirement invalidates the credentials that scoped + // all outstanding work. Advance the epoch so those completions cannot + // subsequently overwrite the login-required state. + return stable('auth-required', state.epoch + 1, event.detail); + + case 'failed-authentication': + if (!isCurrent(state, event) || state.kind !== 'reauthenticating') return state; + return stable(event.prior.kind, state.epoch, event.prior.detail); + } +} + +export type ConnectionPresentationTone = 'success' | 'error' | 'warning' | 'offline' | 'neutral'; + +/** A DOM-agnostic projection consumed by the header/status surface. */ +export interface ConnectionLifecyclePresentation { + /** Short visible chip text. */ + label: string; + /** An announcement suitable for the status chip's accessible name. */ + ariaLabel: string; + /** Complete class string for the existing connection chip. */ + className: string; + /** Semantic colour treatment, useful to non-DOM consumers too. */ + tone: ConnectionPresentationTone; +} + +/** Project a lifecycle value into the one status-chip display contract. */ +export function connectionLifecyclePresentation(state: ConnectionLifecycleState): ConnectionLifecyclePresentation { + switch (state.kind) { + case 'connected': + return presentation(state.kind, 'Connected', 'connected', 'success'); + case 'auth-required': + return presentation(state.kind, 'Sign in required', 'authentication required', 'error'); + case 'starting': + return presentation(state.kind, 'Connecting…', 'connecting', 'warning'); + case 'refreshing': + return presentation(state.kind, 'Refreshing…', 'refreshing', 'warning'); + case 'reauthenticating': + return presentation(state.kind, 'Signing in…', 'reauthenticating', 'warning'); + case 'offline': + return presentation(state.kind, 'Offline', 'offline', 'offline'); + case 'signed-out': + return presentation(state.kind, 'Signed out', 'signed out', 'neutral'); + } +} + +function presentation( + kind: ConnectionLifecycleState['kind'], + label: string, + ariaState: string, + tone: ConnectionPresentationTone, +): ConnectionLifecyclePresentation { + return { + label, + ariaLabel: `ClickHouse connection: ${ariaState}`, + className: `conn-status connection-chip is-${kind} tone-${tone}`, + tone, + }; +} diff --git a/src/net/ch-client.ts b/src/net/ch-client.ts index 50e2825c..58688e23 100644 --- a/src/net/ch-client.ts +++ b/src/net/ch-client.ts @@ -20,18 +20,26 @@ import { sqlString } from '../core/format.js'; * real implementations in production, plain stubs in tests. `authConfirmed` * and `dataLakeCatalogSettingUnsupported` are one-shot-then-remember latches * `authedFetch`/`querySystemAware` set on `ctx` itself (see their docstrings) - * — optional here because they start unset. */ + * — optional here because they start unset. The epoch/lifecycle hooks are + * optional too, preserving the smaller seam used by existing callers. */ export interface ChCtx { fetch: typeof fetch; origin: string; getToken(): Promise; refresh(): Promise; - onSignedOut(detail?: string): void; + onSignedOut(detail?: string, expectedEpoch?: number): void; /** Picks the Authorization scheme (Bearer vs Basic); defaults to Bearer * inside `authedFetch` when absent. */ authHeader?: (token: string) => string; authConfirmed?: boolean; dataLakeCatalogSettingUnsupported?: boolean; + /** Identifies the active credential/session generation. A request captures + * this before its first await so stale work cannot affect its replacement. */ + currentEpoch?: () => number; + /** Receives a current request's successful HTTP 2xx transport settlement. */ + onTransportConnected?: () => void; + /** Receives a current non-abort rejection from the injected fetch seam. */ + onTransportOffline?: (error?: unknown) => void; } /** The injected SQL-string-quoting function a few call sites take as a @@ -58,6 +66,13 @@ function errMessage(e: unknown): string { return typeof message === 'string' && message ? message : String(e); } +// A client that supplied no epoch hook remains backward-compatible: every +// request is current. When it did supply one, no stale request may alter the +// replacement credential generation's lifecycle/auth state. +function isCurrentEpoch(ctx: ChCtx, requestEpoch: number | undefined): boolean { + return requestEpoch === undefined || ctx.currentEpoch?.() === requestEpoch; +} + /** Generic ClickHouse `FORMAT JSON` response shape — only `.data` is ever * read here; every other field (meta, statistics, rows_before_limit_at_least…) * is ignored by this module. */ @@ -91,9 +106,10 @@ export function chUrl(origin: string, opts: ChUrlOpts = {}): string { * when authentication cannot be recovered. */ export async function authedFetch(ctx: ChCtx, url: string, sql: string, signal?: AbortSignal): Promise { + const requestEpoch = ctx.currentEpoch?.(); const token = await ctx.getToken(); if (!token) { - ctx.onSignedOut(); + if (isCurrentEpoch(ctx, requestEpoch)) ctx.onSignedOut(undefined, requestEpoch); throw new Error('not signed in'); } let bearer = token; @@ -102,14 +118,30 @@ export async function authedFetch(ctx: ChCtx, url: string, sql: string, signal?: // default to Bearer so the seam stays optional. const authHeader = ctx.authHeader || ((t: string) => 'Bearer ' + t); for (;;) { - const resp = await ctx.fetch(url, { - method: 'POST', - body: sql, - headers: { Authorization: authHeader(bearer) }, - signal, - }); + let resp: Response; + try { + resp = await ctx.fetch(url, { + method: 'POST', + body: sql, + headers: { Authorization: authHeader(bearer) }, + signal, + }); + } catch (e) { + // Only a rejected fetch is a transport failure. HTTP failures are normal + // responses and caller cancellation is deliberately invisible here. + const aborted = signal?.aborted || (e as { name?: unknown } | null)?.name === 'AbortError'; + if (isCurrentEpoch(ctx, requestEpoch) && !aborted) ctx.onTransportOffline?.(e); + throw e; + } + // The request may have crossed a sign-in/sign-out boundary while fetch was + // pending. Its response still belongs to its caller, but cannot change the + // new epoch's connection/auth state or start a refresh with its token. + if (!isCurrentEpoch(ctx, requestEpoch)) return resp; // A 2xx confirms the credentials are good for the rest of the session. - if (resp.ok) ctx.authConfirmed = true; + if (resp.ok) { + ctx.authConfirmed = true; + ctx.onTransportConnected?.(); + } let authExpired = resp.status === 401 || resp.status === 403; if (!authExpired && !resp.ok) { const peek = await resp.clone().text(); @@ -123,17 +155,21 @@ export async function authedFetch(ctx: ChCtx, url: string, sql: string, signal?: // caller shows it as a normal query error instead of force-logging-out. if (ctx.authConfirmed) return resp; if (attempt === 0 && (await ctx.refresh())) { + if (!isCurrentEpoch(ctx, requestEpoch)) return resp; // A successful refresh always yields a fresh, usable token — the // refresh() contract this seam relies on. bearer = (await ctx.getToken())!; + if (!isCurrentEpoch(ctx, requestEpoch)) return resp; attempt++; continue; } + if (!isCurrentEpoch(ctx, requestEpoch)) return resp; // First-contact 401/403 with a non-expired token: CH rejected the login // itself — an authorization/identity problem, not session expiry. Surface // CH's own reason so it's diagnosable. const reason = parseExceptionText(await resp.clone().text()); - ctx.onSignedOut(authDeniedMessage(resp.status, reason)); + if (!isCurrentEpoch(ctx, requestEpoch)) return resp; + ctx.onSignedOut(authDeniedMessage(resp.status, reason), requestEpoch); throw new Error('signed out'); } return resp; diff --git a/src/net/oauth.ts b/src/net/oauth.ts index 07bf320f..241e2940 100644 --- a/src/net/oauth.ts +++ b/src/net/oauth.ts @@ -75,28 +75,26 @@ export async function exchangeCodeForTokens(fetchFn: typeof fetch, cfg: Resolved } /** - * Redeem a refresh_token for fresh tokens. Returns the token JSON, or null on - * any failure (caller treats null as "must re-login"). + * Redeem a refresh_token for fresh tokens. Returns null when the IdP rejects + * the refresh (caller treats that as "must re-login"). Transport failures are + * rethrown so the connection lifecycle can preserve credentials and report a + * recoverable offline state instead of misclassifying the network as auth loss. */ export async function refreshTokens(fetchFn: typeof fetch, cfg: ResolvedIdpConfig, refreshToken: string | null | undefined): Promise { if (!refreshToken) return null; - try { - const body: Record = { - grant_type: 'refresh_token', - client_id: cfg.clientId, - refresh_token: refreshToken, - }; - if (cfg.clientSecret) body.client_secret = cfg.clientSecret; - const resp = await fetchFn(cfg.tokenUri, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: new URLSearchParams(body), - }); - if (!resp.ok) return null; - return await resp.json() as TokenResponse; - } catch { - return null; - } + const body: Record = { + grant_type: 'refresh_token', + client_id: cfg.clientId, + refresh_token: refreshToken, + }; + if (cfg.clientSecret) body.client_secret = cfg.clientSecret; + const resp = await fetchFn(cfg.tokenUri, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: new URLSearchParams(body), + }); + if (!resp.ok) return null; + return await resp.json() as TokenResponse; } /** diff --git a/src/styles.css b/src/styles.css index fba92f78..4d094762 100644 --- a/src/styles.css +++ b/src/styles.css @@ -594,10 +594,18 @@ h1, h2, h3, h4, h5, h6 { .connection-state { display: none; } .conn-status::before { content: ''; width: 7px; height: 7px; border-radius: var(--r-xs); - background: var(--success-fg); box-shadow: 0 0 6px var(--success-fg); + background: var(--fg-faint); box-shadow: none; flex-shrink: 0; } -.conn-status.dim::before { background: var(--fg-faint); box-shadow: none; } +.conn-status.tone-success::before { + background: var(--success-fg); box-shadow: 0 0 6px var(--success-fg); +} +.conn-status.tone-error::before { + background: var(--error-fg); +} +.conn-status.tone-offline::before { + background: var(--warn-fg); +} .hd-btn { width: 26px; height: 26px; border: none; background: transparent; diff --git a/src/ui/app-header.ts b/src/ui/app-header.ts index b8199867..1a363e79 100644 --- a/src/ui/app-header.ts +++ b/src/ui/app-header.ts @@ -1,6 +1,7 @@ import { h } from './dom.js'; import { Icon } from './icons.js'; import { userShortName } from '../core/format.js'; +import { connectionLifecyclePresentation } from '../core/connection-lifecycle.js'; import { libraryControls, QUERY_FILE_MENU } from './file-menu.js'; import type { FileMenuSurfaceContext } from './file-menu.js'; import type { App } from './app.types.js'; @@ -33,19 +34,37 @@ export function routeButton( }, h('span', { class: 'surface-label' }, label)); } +/** Paint the existing chip from the session's authoritative lifecycle. Safe + * before the header exists so app-shell can use one reactive effect across + * its mount/setHeader ordering. */ +export function applyConnectionStatus(app: Pick): void { + const chip = app.dom.connStatus; + if (!chip) return; + const lifecycle = app.conn.connection.value; + const presentation = connectionLifecyclePresentation(lifecycle); + chip.className = presentation.className; + chip.dataset.connectionState = lifecycle.kind; + chip.setAttribute('aria-label', presentation.ariaLabel); + chip.title = app.conn.host(); + chip.querySelector('.connection-host')!.textContent = app.conn.host(); + chip.querySelector('.connection-state')!.textContent = presentation.label; +} + /** The one application header used by both Workbench and Dashboard. */ export function buildAppHeader(app: App, options: AppHeaderOptions = {}): HTMLElement { app.dom.themeBtn = h('button', { class: 'hd-btn', title: 'Toggle theme', onclick: () => app.toggleTheme(), }, app.state.theme === 'dark' ? Icon.sun() : Icon.moon()); + const connection = connectionLifecyclePresentation(app.conn.connection.value); app.dom.connStatus = h('div', { - class: `conn-status connection-chip${app.state.serverVersion ? '' : ' dim'}`, + class: connection.className, + 'data-connection-state': app.conn.connection.value.kind, role: 'status', - 'aria-label': app.state.serverVersion ? 'ClickHouse connection: connected' : 'ClickHouse connection: connecting', + 'aria-label': connection.ariaLabel, title: app.conn.host(), }, h('span', { class: 'connection-host' }, app.conn.host()), - h('span', { class: 'connection-state' }, app.state.serverVersion ? 'Connected' : 'Connecting…')); + h('span', { class: 'connection-state' }, connection.label)); app.dom.userBtn = h('button', { class: 'hd-btn user-btn', title: app.conn.email(), onclick: () => app.actions.openUserMenu(), }, h('span', { class: 'user-short' }, userShortName(app.conn.email())), Icon.chevDown()); diff --git a/src/ui/app-shell.ts b/src/ui/app-shell.ts index 0cd2de38..ffff32a4 100644 --- a/src/ui/app-shell.ts +++ b/src/ui/app-shell.ts @@ -38,6 +38,7 @@ import { buildSidebarUpper, renderUpperRoleTabs } from './sidebar-upper.js'; import { renderDashboardTree, cancelDashboardTreeClicks } from './dashboard-tree.js'; import { renderSavedHistory } from './saved-history.js'; import { renderLibraryTitle } from './file-menu.js'; +import { applyConnectionStatus } from './app-header.js'; import type { DragCtx, DragRect, DragStartEvent, SplitterAxis } from './splitters.js'; import { startDrag } from './splitters.js'; import type { App } from './app.types.js'; @@ -272,6 +273,14 @@ export function mountAppShell(deps: AppShellDeps): AppShellHandle { state.libraryDirty.value; renderLibraryTitle(app); })); + // ConnectionSession is the single authority for the status chip. This + // effect intentionally registers before the caller builds the header; its + // first no-op subscribes to the signal, and setHeader below performs the + // initial paint once the chip exists. + disposers.push(effect(() => { + app.conn.connection.value; + applyConnectionStatus(app); + })); // Mobile mode (#126): mirror the viewport width into `isMobile` (drives the // schema tree's drag/hover affordances, the results drop target, and the // auto-navigation in the action wrappers) via the injected matchMedia seam. @@ -292,7 +301,10 @@ export function mountAppShell(deps: AppShellDeps): AppShellHandle { catalog.loadReference(); return { - setHeader: (header: Element) => { headerSlot.replaceChildren(header); }, + setHeader: (header: Element) => { + headerSlot.replaceChildren(header); + applyConnectionStatus(app); + }, queryHost, dashboardHost, showHost: (kind) => { diff --git a/src/ui/app.ts b/src/ui/app.ts index cc7fa66f..0bf062ec 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -514,25 +514,15 @@ export function createApp(env: CreateAppEnv = {}): App { // data + completions, hover-doc cache — now lives in // `application/schema-catalog-service.ts`, constructible without // App/AppState/DOM (see that file for the ported bodies, byte-identical to - // this file's history). `setConn`/`updateBanner` (DOM) and the schema/ - // banner effects stay HERE, driven by the same `state.serverVersion`/ - // `state.schemaError` the service writes — those signals/effects are - // exactly as before. - function setConn(online: boolean): void { - const chip = app.dom.connStatus; - if (!chip) return; - chip.classList.toggle('dim', !online); - const host = app.conn.host(); - chip.title = online ? host : `${host} · offline`; - chip.setAttribute('aria-label', `ClickHouse connection: ${online ? 'connected' : 'offline'}`); - const state = chip.querySelector('.connection-state'); - if (state) state.textContent = online ? 'Connected' : 'Offline'; + // this file's history). `updateBanner` (DOM) and the schema/banner effects + // stay HERE. The header connection chip is driven exclusively by + // ConnectionSession's lifecycle signal; a metadata probe is not connection + // authority. + function updateOpenServerVersion(version: string): void { const server = app.dom.userMenu?.querySelector('.um-server'); - if (server) { - const version = app.state.serverVersion; - server.hidden = !version; - server.textContent = version ? `CH ${shortVersion(version)}` : ''; - } + if (!server) return; + server.hidden = false; + server.textContent = `CH ${shortVersion(version)}`; } const catalog = createSchemaCatalogService({ loadServerVersion: ch.loadServerVersion, @@ -548,7 +538,7 @@ export function createApp(env: CreateAppEnv = {}): App { sqlString, state: app.state, hooks: { - onConnStatusChanged: setConn, + onServerVersionLoaded: updateOpenServerVersion, renderVarStrip: () => app.renderVarStrip(), refreshEditorReference: () => app.sqlEditor.refreshReference(), }, diff --git a/tests/helpers/fake-app.ts b/tests/helpers/fake-app.ts index 88eb9fe4..b239d303 100644 --- a/tests/helpers/fake-app.ts +++ b/tests/helpers/fake-app.ts @@ -22,6 +22,7 @@ // other sibling fields. import { vi } from 'vitest'; import dagre from '@dagrejs/dagre'; +import { signal } from '@preact/signals-core'; import { createState, activeTab, reconcileTabsWithSavedQueries } from '../../src/state.js'; import type { MutateWorkspace } from '../../src/state.js'; import { createNoopPort } from '../../src/editor/editor-port.js'; @@ -212,6 +213,7 @@ export function statefulWorkspaceRepo(initial: StoredWorkspaceV5 | null = null): const chCtxDefaults: ChCtx = { fetch, origin: '', authConfirmed: true, getToken: async () => null, refresh: async () => false, authHeader: () => '', onSignedOut: () => {}, + currentEpoch: () => 0, onTransportConnected: () => {}, onTransportOffline: () => {}, }; // A minimal `ConnectionSession` stub (#276 Phase 2) — most render-module @@ -223,6 +225,7 @@ const connDefaults: ConnectionSession = { basePath: '', hostHint: '', chCtx: chCtxDefaults, + connection: signal({ kind: 'connected' as const, epoch: 0 }), token: () => null, refreshToken: () => null, authMode: () => 'basic', diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index 63eb28df..45290b27 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -1145,14 +1145,28 @@ describe('renderApp shell', () => { expect(app.dom.userBtn!.getAttribute('title')).toBe('me@example.com'); await Promise.resolve(); }); - it('keeps the Workbench header focused on the ClickHouse endpoint', () => { + it('keeps the Workbench header focused on the endpoint and projects the lifecycle reactively', () => { const app = createApp(env()); + // Metadata is display-only and cannot make the connection green. app.state.serverVersion = '26.7.1.42'; app.renderApp(); expect(qs(app.root, '.connection-host').textContent).toBe('ch.example'); + expect(qs(app.root, '.connection-state').textContent).toBe('Connecting…'); + expect(qs(app.root, '.conn-status').classList.contains('is-starting')).toBe(true); + expect(qs(app.root, '.conn-status').dataset.connectionState).toBe('starting'); + expect(qs(app.root, '.conn-status').getAttribute('aria-label')).toBe('ClickHouse connection: connecting'); + + app.conn.chCtx.onTransportConnected(); expect(qs(app.root, '.connection-state').textContent).toBe('Connected'); expect(qs(app.root, '.conn-status').getAttribute('title')).toBe('ch.example'); expect(qs(app.root, '.conn-status').getAttribute('aria-label')).toBe('ClickHouse connection: connected'); + expect(qs(app.root, '.conn-status').classList.contains('tone-success')).toBe(true); + + app.state.serverVersion = null; + app.conn.chCtx.onTransportOffline(new TypeError('network down')); + expect(qs(app.root, '.connection-state').textContent).toBe('Offline'); + expect(qs(app.root, '.conn-status').dataset.connectionState).toBe('offline'); + expect(qs(app.root, '.conn-status').classList.contains('tone-offline')).toBe(true); }); it('toggles theme via the header button', () => { const { app } = rendered(); @@ -1227,15 +1241,15 @@ describe('loadVersion / loadSchema', () => { expect(app.dom.connStatus!.title).toBe(app.conn.host()); expect(qs(app.dom.connStatus!, '.connection-state')?.textContent).toBe('Connected'); }); - it('marks offline when the version query fails', async () => { + it('does not misclassify an HTTP version-query failure as transport offline', async () => { const e = env({ fetch: makeFetch([[(u, sql) => /version/.test(sql), resp({ ok: false, status: 500, text: 'err' })]]) }); const app = createApp(e); app.renderApp(); await app.catalog.loadVersion(); - expect(app.dom.connStatus!.title).toBe(`${app.conn.host()} · offline`); + expect(app.dom.connStatus!.title).toBe(app.conn.host()); expect(qs(app.dom.connStatus!, '.connection-host')?.textContent).toBe(app.conn.host()); - expect(qs(app.dom.connStatus!, '.connection-state')?.textContent).toBe('Offline'); - expect(app.dom.connStatus!.getAttribute('aria-label')).toBe('ClickHouse connection: offline'); + expect(qs(app.dom.connStatus!, '.connection-state')?.textContent).toBe('Connected'); + expect(app.dom.connStatus!.getAttribute('aria-label')).toBe('ClickHouse connection: connected'); }); it('updates an open user menu when the server-version probe finishes', async () => { const e = env({ fetch: makeFetch([[(u, sql) => /version/.test(sql), resp({ json: { data: [{ v: '26.3.1.42' }] } })]]) }); @@ -3351,15 +3365,16 @@ describe('auth flows', () => { const app = createApp(e); expect(await app.conn.chCtx.getToken()).toBeNull(); }); - it('onSignedOut shows the given message, else a session-expired default', async () => { + it('onSignedOut reports the first auth loss once per credential epoch', async () => { const app = createApp(env()); app.renderApp(); // authorization denial: CH-supplied message is shown verbatim on the login screen app.conn.chCtx.onSignedOut('ClickHouse denied your account (HTTP 403). Server: nope'); expect(qs(app.root, '.login-error').textContent).toContain('denied your account'); - // genuine expiry: no detail → the reworded default + // A duplicate signal from another concurrent request cannot replace the + // first, more useful explanation or trigger another teardown. app.conn.chCtx.onSignedOut(); - expect(qs(app.root, '.login-error').textContent).toContain('session expired'); + expect(qs(app.root, '.login-error').textContent).toContain('denied your account'); }); it('login(idp, origin) stashes oauth_origin for a cross-origin cluster; sign-out clears it', async () => { const loc = { host: 'ch', origin: 'https://ch', pathname: '/sql', search: '', hash: '', href: 'https://ch/sql' } as Location; diff --git a/tests/unit/ch-client.test.ts b/tests/unit/ch-client.test.ts index f1593062..cc2eb2b1 100644 --- a/tests/unit/ch-client.test.ts +++ b/tests/unit/ch-client.test.ts @@ -82,6 +82,16 @@ function ctxWith(fetchImpl: FetchImpl, over: Partial = {}) { }; } +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((done, fail) => { + resolve = done; + reject = fail; + }); + return { promise, resolve, reject }; +} + describe('chUrl', () => { it('uses default format and compression', () => { expect(chUrl('https://o')).toBe('https://o?default_format=JSONStringsEachRowWithProgress&enable_http_compression=1'); @@ -106,12 +116,64 @@ describe('authedFetch', () => { await expect(authedFetch(ctx, 'u', 'sql')).rejects.toThrow('not signed in'); expect(ctx.onSignedOut).toHaveBeenCalled(); }); + it('does not signal auth loss when a missing-token request becomes stale', async () => { + let epoch = 1; + const token = deferred(); + const ctx = ctxWith(() => jsonResp({}), { currentEpoch: () => epoch, getToken: () => token.promise }); + const pending = authedFetch(ctx, 'u', 'sql'); + epoch = 2; + token.resolve(null); + await expect(pending).rejects.toThrow('not signed in'); + expect(ctx.onSignedOut).not.toHaveBeenCalled(); + }); it('returns the response on success', async () => { const ctx = ctxWith(async () => jsonResp({ ok: 1 })); const r = await authedFetch(ctx, 'u', 'sql'); expect(r.ok).toBe(true); expect(ctx.fetchMock.mock.calls[0][1].headers.Authorization).toBe('Bearer tok'); }); + it('reports a successful current transport connection when lifecycle hooks are supplied', async () => { + const onTransportConnected = vi.fn(); + const ctx = ctxWith(async () => jsonResp({ ok: 1 }), { currentEpoch: () => 4, onTransportConnected }); + await authedFetch(ctx, 'u', 'sql'); + expect(onTransportConnected).toHaveBeenCalledTimes(1); + }); + it('reports a non-abort fetch rejection as transport-offline', async () => { + const failure = new Error('network unavailable'); + const onTransportOffline = vi.fn(); + const ctx = ctxWith(async () => { throw failure; }, { currentEpoch: () => 4, onTransportOffline }); + await expect(authedFetch(ctx, 'u', 'sql')).rejects.toBe(failure); + expect(onTransportOffline).toHaveBeenCalledWith(failure); + }); + it('does not report a stale fetch rejection as transport-offline', async () => { + let epoch = 1; + const failure = new Error('network unavailable'); + const rejectedFetch = deferred(); + const onTransportOffline = vi.fn(); + const ctx = ctxWith(async () => rejectedFetch.promise, { currentEpoch: () => epoch, onTransportOffline }); + const pending = authedFetch(ctx, 'u', 'sql'); + epoch = 2; + rejectedFetch.reject(failure); + await expect(pending).rejects.toBe(failure); + expect(onTransportOffline).not.toHaveBeenCalled(); + }); + it('does not report HTTP failures or caller cancellation as transport-offline', async () => { + const onTransportOffline = vi.fn(); + const httpCtx = ctxWith(async () => textResp('server error', false, 500), { onTransportOffline }); + await authedFetch(httpCtx, 'u', 'sql'); + const controller = new AbortController(); + controller.abort(); + const abortCtx = ctxWith(async () => { throw new Error('cancelled request'); }, { onTransportOffline }); + await expect(authedFetch(abortCtx, 'u', 'sql', controller.signal)).rejects.toThrow('cancelled request'); + expect(onTransportOffline).not.toHaveBeenCalled(); + }); + it('does not report an AbortError rejection as transport-offline', async () => { + const onTransportOffline = vi.fn(); + const abortError = Object.assign(new Error('cancelled request'), { name: 'AbortError' }); + const ctx = ctxWith(async () => { throw abortError; }, { onTransportOffline }); + await expect(authedFetch(ctx, 'u', 'sql')).rejects.toBe(abortError); + expect(onTransportOffline).not.toHaveBeenCalled(); + }); it('refreshes once on 401 then retries', async () => { let n = 0; const ctx = ctxWith(async () => (n++ === 0 ? jsonResp({}, false, 401) : jsonResp({ ok: 1 })), { @@ -142,6 +204,118 @@ describe('authedFetch', () => { await authedFetch(ctx, 'u', 'sql'); expect(ctx.authConfirmed).toBe(true); }); + it('fences a stale successful response from lifecycle and authentication state', async () => { + let epoch = 1; + const response = deferred(); + const onTransportConnected = vi.fn(); + const ctx = ctxWith(async () => response.promise, { currentEpoch: () => epoch, onTransportConnected }); + const pending = authedFetch(ctx, 'u', 'sql'); + epoch = 2; + response.resolve(jsonResp({ ok: 1 })); + expect((await pending).ok).toBe(true); + expect(ctx.authConfirmed).toBeUndefined(); + expect(onTransportConnected).not.toHaveBeenCalled(); + }); + it('fences a stale auth-failure response from refresh and sign-out', async () => { + let epoch = 1; + const onTransportOffline = vi.fn(); + const ctx = ctxWith(async () => jsonResp({}, false, 401), { + currentEpoch: () => epoch, + refresh: vi.fn(async () => true), + onTransportOffline, + }); + const pending = authedFetch(ctx, 'u', 'sql'); + epoch = 2; + expect((await pending).status).toBe(401); + expect(ctx.refresh).not.toHaveBeenCalled(); + expect(ctx.onSignedOut).not.toHaveBeenCalled(); + expect(onTransportOffline).not.toHaveBeenCalled(); + }); + it('does not retry or sign out when refresh settles after its request becomes stale', async () => { + let epoch = 1; + const refresh = deferred(); + const refreshStarted = deferred(); + const ctx = ctxWith(async () => jsonResp({}, false, 401), { + currentEpoch: () => epoch, + refresh: vi.fn(async () => { + refreshStarted.resolve(); + return refresh.promise; + }), + }); + const pending = authedFetch(ctx, 'u', 'sql'); + await refreshStarted.promise; + expect(ctx.refresh).toHaveBeenCalledTimes(1); + epoch = 2; + refresh.resolve(true); + expect((await pending).status).toBe(401); + expect(ctx.fetchMock).toHaveBeenCalledTimes(1); + expect(ctx.onSignedOut).not.toHaveBeenCalled(); + }); + it('does not retry when the post-refresh token lookup becomes stale', async () => { + let epoch = 1; + let tokenCalls = 0; + const freshToken = deferred(); + const freshTokenStarted = deferred(); + const ctx = ctxWith(async () => textResp('expired', false, 401), { + currentEpoch: () => epoch, + refresh: vi.fn(async () => true), + getToken: vi.fn(async () => { + tokenCalls += 1; + if (tokenCalls === 1) return 'old'; + freshTokenStarted.resolve(); + return freshToken.promise; + }), + }); + const pending = authedFetch(ctx, 'u', 'sql'); + await freshTokenStarted.promise; + epoch = 2; + freshToken.resolve('new'); + expect((await pending).status).toBe(401); + expect(ctx.fetchMock).toHaveBeenCalledTimes(1); + expect(ctx.onSignedOut).not.toHaveBeenCalled(); + }); + it('does not sign out when an unsuccessful refresh becomes stale', async () => { + let epoch = 1; + const refresh = deferred(); + const refreshStarted = deferred(); + const ctx = ctxWith(async () => textResp('expired', false, 401), { + currentEpoch: () => epoch, + refresh: vi.fn(async () => { + refreshStarted.resolve(); + return refresh.promise; + }), + }); + const pending = authedFetch(ctx, 'u', 'sql'); + await refreshStarted.promise; + epoch = 2; + refresh.resolve(false); + expect((await pending).status).toBe(401); + expect(ctx.onSignedOut).not.toHaveBeenCalled(); + }); + it('does not sign out when parsing the denial reason crosses an epoch', async () => { + let epoch = 1; + const denialText = deferred(); + const textStarted = deferred(); + const response: FakeResponse = { + ok: false, + status: 403, + text: async () => { + textStarted.resolve(); + return denialText.promise; + }, + clone() { return this; }, + }; + const ctx = ctxWith(async () => response, { + currentEpoch: () => epoch, + refresh: vi.fn(async () => false), + }); + const pending = authedFetch(ctx, 'u', 'sql'); + await textStarted.promise; + epoch = 2; + denialText.resolve('Code: 516. Authentication failed'); + expect((await pending).status).toBe(403); + expect(ctx.onSignedOut).not.toHaveBeenCalled(); + }); it('once authenticated, a later 403 is returned as a query error (no sign-out)', async () => { // e.g. SHOW CREATE USER → HTTP 403 / UNKNOWN_USER, mid-session. const ctx = ctxWith(async () => textResp('Code: 192. DB::Exception: There is no user x', false, 403), diff --git a/tests/unit/connection-lifecycle.test.ts b/tests/unit/connection-lifecycle.test.ts new file mode 100644 index 00000000..bcd96bfa --- /dev/null +++ b/tests/unit/connection-lifecycle.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it } from 'vitest'; +import { + connectionLifecyclePresentation, + initialConnectionLifecycle, + reduceConnectionLifecycle, + type AuthenticationPriorState, + type ConnectionLifecycleState, +} from '../../src/core/connection-lifecycle.js'; + +const connected = (epoch = 4) => ({ kind: 'connected' as const, epoch }); +const starting = (epoch = 4) => ({ kind: 'starting' as const, epoch }); +const offline = (epoch = 4) => ({ kind: 'offline' as const, epoch }); +const authRequired = (epoch = 4) => ({ kind: 'auth-required' as const, epoch, detail: 'expired' }); +const signedOut = (epoch = 4) => ({ kind: 'signed-out' as const, epoch, detail: 'user choice' }); +const reauthenticating = (epoch = 4) => ({ kind: 'reauthenticating' as const, epoch }); + +describe('connection lifecycle reducer', () => { + it('starts at epoch zero and only includes detail when supplied', () => { + expect(initialConnectionLifecycle()).toEqual({ kind: 'starting', epoch: 0 }); + expect(initialConnectionLifecycle('restoring')).toEqual({ kind: 'starting', epoch: 0, detail: 'restoring' }); + }); + + it('starts interactive authentication in a new epoch', () => { + expect(reduceConnectionLifecycle(connected(), { type: 'start-authentication', detail: 'renew' })) + .toEqual({ kind: 'reauthenticating', epoch: 5, detail: 'renew' }); + }); + + it('installs credentials in a new epoch', () => { + expect(reduceConnectionLifecycle(authRequired(), { type: 'credentials-installed' })) + .toEqual({ kind: 'starting', epoch: 5 }); + }); + + it('signs out in a new epoch', () => { + expect(reduceConnectionLifecycle(connected(), { type: 'signed-out', detail: 'user choice' })) + .toEqual({ kind: 'signed-out', epoch: 5, detail: 'user choice' }); + }); + + it('begins a current refresh from every resumable state, retaining that exact state', () => { + for (const state of [connected(), starting(), offline()]) { + const refreshed = reduceConnectionLifecycle(state, { type: 'begin-refresh', epoch: 4, detail: 'renewing' }); + expect(refreshed).toMatchObject({ kind: 'refreshing', epoch: 4, detail: 'renewing' }); + expect(refreshed.kind === 'refreshing' && refreshed.resume).toBe(state); + } + }); + + it('returns the retained resume state after a successful refresh, never inventing connected', () => { + for (const state of [connected(), starting(), offline()]) { + const refreshing = reduceConnectionLifecycle(state, { type: 'begin-refresh', epoch: 4 }); + const resumed = reduceConnectionLifecycle(refreshing, { type: 'refresh-succeeded', epoch: 4 }); + expect(resumed).toBe(state); + } + }); + + it('moves current starting/offline transport to connected', () => { + expect(reduceConnectionLifecycle(starting(), { type: 'transport-connected', epoch: 4 })) + .toEqual({ kind: 'connected', epoch: 4 }); + expect(reduceConnectionLifecycle(offline(), { type: 'transport-connected', epoch: 4, detail: 'probe' })) + .toEqual({ kind: 'connected', epoch: 4, detail: 'probe' }); + }); + + it('moves current non-protected transport state offline', () => { + for (const state of [starting(), connected(), { kind: 'refreshing', epoch: 4, resume: connected() } as ConnectionLifecycleState]) { + expect(reduceConnectionLifecycle(state, { type: 'transport-offline', epoch: 4, detail: 'network' })) + .toEqual({ kind: 'offline', epoch: 4, detail: 'network' }); + } + }); + + it('does not let transport offline overwrite authentication or sign-out states', () => { + for (const state of [authRequired(), signedOut(), reauthenticating()]) { + expect(reduceConnectionLifecycle(state, { type: 'transport-offline', epoch: 4 })).toBe(state); + } + }); + + it('moves a current live state to auth required', () => { + expect(reduceConnectionLifecycle(connected(), { type: 'auth-required', epoch: 4, detail: 'expired' })) + .toEqual({ kind: 'auth-required', epoch: 5, detail: 'expired' }); + }); + + it('restores the supplied auth failure prior state at the current epoch', () => { + const prior: AuthenticationPriorState = { kind: 'signed-out', epoch: 1, detail: 'cancelled' }; + expect(reduceConnectionLifecycle(reauthenticating(), { + type: 'failed-authentication', epoch: 4, prior, + })).toEqual({ kind: 'signed-out', epoch: 4, detail: 'cancelled' }); + }); + + it('returns the same object for stale and illegal events', () => { + const refreshing: ConnectionLifecycleState = { kind: 'refreshing', epoch: 4, resume: connected() }; + const cases: Array<[ConnectionLifecycleState, Parameters[1]]> = [ + [connected(), { type: 'begin-refresh', epoch: 3 }], + [connected(), { type: 'refresh-succeeded', epoch: 4 }], + [connected(), { type: 'transport-connected', epoch: 4 }], + [offline(), { type: 'auth-required', epoch: 3 }], + [signedOut(), { type: 'auth-required', epoch: 4 }], + [reauthenticating(), { type: 'failed-authentication', epoch: 3, prior: signedOut() }], + [connected(), { type: 'failed-authentication', epoch: 4, prior: authRequired() }], + [refreshing, { type: 'transport-connected', epoch: 4 }], + ]; + for (const [state, event] of cases) expect(reduceConnectionLifecycle(state, event)).toBe(state); + }); +}); + +describe('connection lifecycle presentation', () => { + it.each([ + [connected(), 'Connected', 'ClickHouse connection: connected', 'conn-status connection-chip is-connected tone-success', 'success'], + [starting(), 'Connecting…', 'ClickHouse connection: connecting', 'conn-status connection-chip is-starting tone-warning', 'warning'], + [{ kind: 'refreshing', epoch: 4, resume: offline() } as ConnectionLifecycleState, 'Refreshing…', 'ClickHouse connection: refreshing', 'conn-status connection-chip is-refreshing tone-warning', 'warning'], + [offline(), 'Offline', 'ClickHouse connection: offline', 'conn-status connection-chip is-offline tone-offline', 'offline'], + [authRequired(), 'Sign in required', 'ClickHouse connection: authentication required', 'conn-status connection-chip is-auth-required tone-error', 'error'], + [reauthenticating(), 'Signing in…', 'ClickHouse connection: reauthenticating', 'conn-status connection-chip is-reauthenticating tone-warning', 'warning'], + [signedOut(), 'Signed out', 'ClickHouse connection: signed out', 'conn-status connection-chip is-signed-out tone-neutral', 'neutral'], + ] as const)('%s projects a concise accessible chip', (state, label, ariaLabel, className, tone) => { + expect(connectionLifecyclePresentation(state)).toEqual({ label, ariaLabel, className, tone }); + }); +}); diff --git a/tests/unit/connection-session.test.ts b/tests/unit/connection-session.test.ts index 716ec854..fc558465 100644 --- a/tests/unit/connection-session.test.ts +++ b/tests/unit/connection-session.test.ts @@ -23,7 +23,12 @@ interface FakeResponse { ok: boolean; status: number; json(): Promise; function jsonResponse(status: number, body: unknown): FakeResponse { return { ok: status >= 200 && status < 300, status, json: async () => body, text: async () => JSON.stringify(body) }; } -type RouteFn = (url: string, init?: RequestInit) => FakeResponse | null; +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((done) => { resolve = done; }); + return { promise, resolve }; +} +type RouteFn = (url: string, init?: RequestInit) => FakeResponse | null | Promise; // One shared config.json doc: 'g' is a default (bearer) IdP; 'basicidp' maps // ch_auth=basic with a custom basic_user_claim, for the chUsername/authHeader @@ -44,7 +49,7 @@ function makeFetch(routes: RouteFn[] = []): { fn: typeof fetch; calls: string[] const fn = vi.fn(async (url: string, init?: RequestInit): Promise => { calls.push(url); for (const r of routes) { - const resp = r(url, init); + const resp = await r(url, init); if (resp) return resp; } if (url.endsWith('/config.json')) return jsonResponse(200, CONFIG_DOC_RAW); @@ -143,6 +148,57 @@ describe('construction seeding', () => { expect(session.chAuth()).toBe('bearer'); expect(session.basicUserClaim()).toBe(''); }); + + it('starts restored credentials in starting and an empty session signed out', () => { + const { session: restored } = setup({ storage: memStorage({ oauth_id_token: validToken }) }); + const { session: empty } = setup(); + expect(restored.connection.value).toEqual({ kind: 'starting', epoch: 0 }); + expect(empty.connection.value).toEqual({ kind: 'signed-out', epoch: 0 }); + expect(restored.chCtx.currentEpoch()).toBe(0); + }); +}); + +describe('connection lifecycle ownership', () => { + it('publishes only transport settlements as connected/offline', () => { + const { session } = setup({ storage: memStorage({ oauth_id_token: validToken }) }); + session.chCtx.onTransportConnected(); + expect(session.connection.value).toEqual({ kind: 'connected', epoch: 0 }); + session.chCtx.onTransportOffline(new TypeError('network')); + expect(session.connection.value).toEqual({ kind: 'offline', epoch: 0, detail: 'Network unavailable' }); + session.chCtx.onTransportConnected(); + expect(session.connection.value).toEqual({ kind: 'connected', epoch: 0 }); + }); + + it('installs and explicitly removes credential scopes in new epochs', () => { + const { session } = setup(); + session.setTokens(validToken, 'r1'); + expect(session.connection.value).toEqual({ kind: 'starting', epoch: 1 }); + expect(session.chCtx.currentEpoch()).toBe(1); + session.signOut(); + expect(session.connection.value).toEqual({ kind: 'signed-out', epoch: 2 }); + }); + + it('invalidates involuntary auth loss once and reports it once', () => { + const { session, onAuthLost } = setup({ storage: memStorage({ oauth_id_token: validToken }) }); + session.chCtx.onTransportConnected(); + session.chCtx.onSignedOut('expired by IdP'); + expect(session.connection.value).toEqual({ kind: 'auth-required', epoch: 1, detail: 'expired by IdP' }); + expect(session.token()).toBeNull(); + expect(onAuthLost).toHaveBeenCalledTimes(1); + session.chCtx.onSignedOut('duplicate'); + expect(session.connection.value).toEqual({ kind: 'auth-required', epoch: 1, detail: 'expired by IdP' }); + expect(onAuthLost).toHaveBeenCalledTimes(1); + }); + + it('ignores auth loss captured by an older credential epoch', () => { + const { session, onAuthLost } = setup({ storage: memStorage({ oauth_id_token: validToken }) }); + const oldEpoch = session.chCtx.currentEpoch(); + session.setTokens('replacement-token'); + session.chCtx.onSignedOut('stale failure', oldEpoch); + expect(session.token()).toBe('replacement-token'); + expect(session.connection.value).toEqual({ kind: 'starting', epoch: oldEpoch + 1 }); + expect(onAuthLost).not.toHaveBeenCalled(); + }); }); // ── isSignedIn (zero skew) vs getToken (default skew) ─────────────────────── @@ -280,11 +336,116 @@ describe('refresh (via getToken)', () => { }); it('returns false when the token endpoint yields no usable bearer', async () => { - const { session } = setup({ + const { session, onAuthLost } = setup({ storage: memStorage({ oauth_id_token: expiredToken, oauth_refresh_token: 'r0' }), routes: [(url) => (url.endsWith('/token') ? jsonResponse(200, {}) : null)], }); await expect(session.getToken()).resolves.toBeNull(); + expect(session.connection.value).toMatchObject({ kind: 'auth-required', epoch: 1 }); + expect(onAuthLost).toHaveBeenCalledTimes(1); + }); + + it('preserves credentials and reports offline when refresh configuration cannot load', async () => { + const { session, storage, onAuthLost } = setup({ + storage: memStorage({ oauth_id_token: expiredToken, oauth_refresh_token: 'r0' }), + routes: [(url) => (url.endsWith('/config.json') ? jsonResponse(500, {}) : null)], + }); + await expect(session.getToken()).rejects.toThrow(); + expect(session.connection.value).toEqual({ + kind: 'offline', epoch: 0, detail: 'Unable to refresh session', + }); + expect(session.token()).toBe(expiredToken); + expect(storage.getItem('oauth_refresh_token')).toBe('r0'); + expect(onAuthLost).not.toHaveBeenCalled(); + }); + + it('preserves credentials and reports offline when the IdP transport is unavailable', async () => { + const { session, storage, onAuthLost } = setup({ + storage: memStorage({ oauth_id_token: expiredToken, oauth_refresh_token: 'r0' }), + routes: [async (url) => { + if (url.endsWith('/token')) throw new TypeError('network down'); + return null; + }], + }); + await expect(session.getToken()).rejects.toThrow('network down'); + expect(session.connection.value).toEqual({ + kind: 'offline', epoch: 0, detail: 'Unable to refresh session', + }); + expect(session.token()).toBe(expiredToken); + expect(storage.getItem('oauth_refresh_token')).toBe('r0'); + expect(onAuthLost).not.toHaveBeenCalled(); + }); + + it('shares one refresh promise for all callers in the same epoch', async () => { + const tokenResponse = deferred(); + let tokenCalls = 0; + const { session } = setup({ + storage: memStorage({ oauth_id_token: expiredToken, oauth_refresh_token: 'r0' }), + routes: [async (url) => { + if (!url.endsWith('/token')) return null; + tokenCalls += 1; + return tokenResponse.promise; + }], + }); + const first = session.getToken(); + const second = session.getToken(); + await vi.waitFor(() => expect(tokenCalls).toBe(1)); + expect(session.connection.value.kind).toBe('refreshing'); + expect(tokenCalls).toBe(1); + tokenResponse.resolve(jsonResponse(200, { id_token: 'shared-token', refresh_token: 'r1' })); + await expect(Promise.all([first, second])).resolves.toEqual(['shared-token', 'shared-token']); + expect(tokenCalls).toBe(1); + expect(session.connection.value).toEqual({ kind: 'starting', epoch: 0 }); + }); + + it('fences a late refresh after a newer credential scope is installed', async () => { + const oldResponse = deferred(); + const { session, storage, onAuthLost } = setup({ + storage: memStorage({ oauth_id_token: expiredToken, oauth_refresh_token: 'old-refresh' }), + routes: [async (url) => (url.endsWith('/token') ? oldResponse.promise : null)], + }); + const oldGet = session.getToken(); + await vi.waitFor(() => expect(session.connection.value.kind).toBe('refreshing')); + session.setTokens('new-login-token', 'new-refresh'); + const newEpoch = session.connection.value.epoch; + oldResponse.resolve(jsonResponse(200, { id_token: 'stale-token', refresh_token: 'stale-refresh' })); + await expect(oldGet).resolves.toBe('new-login-token'); + expect(session.connection.value).toEqual({ kind: 'starting', epoch: newEpoch }); + expect(storage.getItem('oauth_id_token')).toBe('new-login-token'); + expect(storage.getItem('oauth_refresh_token')).toBe('new-refresh'); + expect(onAuthLost).not.toHaveBeenCalled(); + }); + + it('does not let an old finally clear a newer epoch refresh slot', async () => { + const oldResponse = deferred(); + const newResponse = deferred(); + let oldCalls = 0; + let newCalls = 0; + const { session } = setup({ + storage: memStorage({ oauth_id_token: expiredToken, oauth_refresh_token: 'old-refresh' }), + routes: [async (url, init) => { + if (!url.endsWith('/token')) return null; + const body = String(init?.body || ''); + if (body.includes('old-refresh')) { + oldCalls += 1; + return oldResponse.promise; + } + newCalls += 1; + return newResponse.promise; + }], + }); + const oldGet = session.getToken(); + await vi.waitFor(() => expect(oldCalls).toBe(1)); + session.setTokens(expiredToken, 'new-refresh'); + const newGetA = session.getToken(); + await vi.waitFor(() => expect(newCalls).toBe(1)); + oldResponse.resolve(jsonResponse(200, { id_token: 'stale' })); + await oldGet; + const newGetB = session.getToken(); + expect(newCalls).toBe(1); + newResponse.resolve(jsonResponse(200, { id_token: 'fresh' })); + await expect(Promise.all([newGetA, newGetB])).resolves.toEqual(['fresh', 'fresh']); + expect(newCalls).toBe(1); }); }); @@ -332,6 +493,14 @@ describe('beginOAuth', () => { search: '?ws=ops&surface=dashboard&mode=view&keep=1', }); }); + + it('restores the prior lifecycle when redirect preparation fails', async () => { + const { session } = setup({ + routes: [(url) => (url.endsWith('/config.json') ? jsonResponse(500, {}) : null)], + }); + await expect(session.beginOAuth('g')).rejects.toThrow(); + expect(session.connection.value).toEqual({ kind: 'signed-out', epoch: 1 }); + }); }); // ── config ─────────────────────────────────────────────────────────────────── @@ -448,6 +617,34 @@ describe('connectBasic', () => { const { session: s2 } = setup({ queryJson: fakeQueryJson(async (ctx) => { ctx.onSignedOut(); return {}; }) }); await expect(s2.connectBasic({ username: 'bob', password: 'bad' })).rejects.toThrow('Authentication failed'); }); + + it('does not let a slow older probe overwrite a newer successful login', async () => { + const alice = deferred(); + const bob = deferred(); + let probes = 0; + const { session, storage } = setup({ + queryJson: fakeQueryJson(async (ctx) => { + probes += 1; + const token = await ctx.getToken(); + const decoded = atob(token || ''); + if (decoded.startsWith('alice:')) await alice.promise; + else await bob.promise; + return { data: [{ 1: 1 }] }; + }), + }); + const older = session.connectBasic({ username: 'alice', password: 'old', host: 'old.example' }); + const newer = session.connectBasic({ username: 'bob', password: 'new', host: 'new.example' }); + await vi.waitFor(() => expect(probes).toBe(2)); + bob.resolve(); + await expect(newer).resolves.toBeUndefined(); + const winningEpoch = session.connection.value.epoch; + alice.resolve(); + await expect(older).rejects.toThrow('Authentication attempt superseded'); + expect(session.connection.value).toEqual({ kind: 'starting', epoch: winningEpoch }); + expect(storage.getItem('ch_basic_user')).toBe('bob'); + expect(storage.getItem('ch_basic_origin')).toBe('https://new.example:8443'); + expect(session.chCtx.origin).toBe('https://new.example:8443'); + }); }); // ── signOut / onSignedOut ──────────────────────────────────────────────────── diff --git a/tests/unit/oauth.test.ts b/tests/unit/oauth.test.ts index fddad706..67b8d696 100644 --- a/tests/unit/oauth.test.ts +++ b/tests/unit/oauth.test.ts @@ -112,9 +112,9 @@ describe('refreshTokens', () => { const f = vi.fn(async () => tokenResp(false, {})); expect(await refreshTokens(asFetch(f), googleCfg, 'rt')).toBeNull(); }); - it('returns null when fetch throws', async () => { + it('rethrows a transport failure so callers do not misclassify it as rejected credentials', async () => { const f = vi.fn(async () => { throw new Error('net'); }); - expect(await refreshTokens(asFetch(f), googleCfg, 'rt')).toBeNull(); + await expect(refreshTokens(asFetch(f), googleCfg, 'rt')).rejects.toThrow('net'); }); }); diff --git a/tests/unit/schema-catalog-service.test.ts b/tests/unit/schema-catalog-service.test.ts index 20b0283f..3d06074f 100644 --- a/tests/unit/schema-catalog-service.test.ts +++ b/tests/unit/schema-catalog-service.test.ts @@ -45,7 +45,7 @@ function makeState(initial: unknown[] | null = null): SchemaCatalogStateSlice { function makeHooks(): Mocked> { return { - onConnStatusChanged: vi.fn>(), + onServerVersionLoaded: vi.fn>(), renderVarStrip: vi.fn(), refreshEditorReference: vi.fn(), }; @@ -78,18 +78,17 @@ const baseSchema = (): SchemaDb[] => ([ // ── loadVersion ────────────────────────────────────────────────────────────── describe('loadVersion', () => { - it('sets serverVersion and reports online on success', async () => { + it('sets serverVersion on success', async () => { const state = makeState(); const hooks = makeHooks(); const deps = makeDeps({ state, hooks, loadServerVersion: vi.fn(async () => '25.1.2.100') }); const svc = createSchemaCatalogService(deps); await svc.loadVersion(); expect(state.serverVersion).toBe('25.1.2.100'); - expect(hooks.onConnStatusChanged).toHaveBeenCalledTimes(1); - expect(hooks.onConnStatusChanged).toHaveBeenCalledWith(true); + expect(hooks.onServerVersionLoaded).toHaveBeenCalledWith('25.1.2.100'); }); - it('reports offline and leaves serverVersion untouched on a probe failure', async () => { + it('leaves serverVersion untouched on a best-effort probe failure', async () => { const state = makeState(); state.serverVersion = 'stale'; const hooks = makeHooks(); @@ -101,13 +100,7 @@ describe('loadVersion', () => { const svc = createSchemaCatalogService(deps); await svc.loadVersion(); expect(state.serverVersion).toBe('stale'); - expect(hooks.onConnStatusChanged).toHaveBeenCalledWith(false); - }); - - it('tolerates a caller that omits onConnStatusChanged', async () => { - const deps = makeDeps({ hooks: { renderVarStrip: vi.fn(), refreshEditorReference: vi.fn() } }); - const svc = createSchemaCatalogService(deps); - await expect(svc.loadVersion()).resolves.toBeUndefined(); + expect(hooks.onServerVersionLoaded).not.toHaveBeenCalled(); }); }); From 95d57bf771ba53eda770395743eac5e5509937f9 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 28 Jul 2026 15:51:59 +0200 Subject: [PATCH 2/4] fix(#512): preserve lifecycle evidence during refresh --- build/check-boundaries.mjs | 22 ++++++- src/application/connection-session.ts | 1 + src/core/connection-lifecycle.ts | 10 ++++ src/styles.css | 6 +- tests/unit/app.test.ts | 28 +++++++++ .../connection-lifecycle-boundaries.test.js | 30 ++++++++++ tests/unit/connection-lifecycle.test.ts | 59 ++++++++++++++++++- 7 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 tests/unit/connection-lifecycle-boundaries.test.js diff --git a/build/check-boundaries.mjs b/build/check-boundaries.mjs index 90589832..7f900500 100644 --- a/build/check-boundaries.mjs +++ b/build/check-boundaries.mjs @@ -179,8 +179,28 @@ for (const rule of RULES) { } } +// Issue #512 Phase 1: connection readiness has one authority. These are the +// modules that own or project the lifecycle, and none may regain the retired +// server-version shortcut. `serverVersion` remains legitimate catalog/query +// capability metadata and user-menu display elsewhere. +const connectionAuthorityFiles = [ + 'src/core/connection-lifecycle.ts', + 'src/application/connection-session.ts', + 'src/net/ch-client.ts', + 'src/ui/app-header.ts', + 'src/ui/app-shell.ts', +]; +for (const relFile of connectionAuthorityFiles) { + const file = path.join(repoRoot, relFile); + if (!fs.existsSync(file)) continue; + checkedFiles += 1; + if (/\bserverVersion\b/.test(fs.readFileSync(file, 'utf8'))) { + violations.push(`${relFile} → serverVersion (issue #512: lifecycle/readiness must not be inferred from version metadata)`); + } +} + if (violations.length) { - console.error('check-boundaries: layer-boundary violations (issue #276):'); + console.error('check-boundaries: architecture violations:'); for (const line of violations) console.error(` ${line}`); process.exit(1); } diff --git a/src/application/connection-session.ts b/src/application/connection-session.ts index 44d9e00e..ccb5e6ad 100644 --- a/src/application/connection-session.ts +++ b/src/application/connection-session.ts @@ -334,6 +334,7 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection // Preserve them and expose a recoverable connectivity state. if (connectionSignal.value.epoch === epoch) { transition({ type: 'transport-offline', epoch, detail: 'Unable to refresh session' }); + transition({ type: 'refresh-failed', epoch }); } throw error; } diff --git a/src/core/connection-lifecycle.ts b/src/core/connection-lifecycle.ts index 943e940c..aec9b6ee 100644 --- a/src/core/connection-lifecycle.ts +++ b/src/core/connection-lifecycle.ts @@ -61,6 +61,7 @@ export type ConnectionLifecycleEvent = | { type: 'credentials-installed'; detail?: string } | ({ type: 'begin-refresh' } & CurrentEpochEvent) | ({ type: 'refresh-succeeded' } & CurrentEpochEvent) + | ({ type: 'refresh-failed' } & CurrentEpochEvent) | ({ type: 'transport-connected' } & CurrentEpochEvent) | ({ type: 'transport-offline' } & CurrentEpochEvent) | ({ type: 'auth-required' } & CurrentEpochEvent) @@ -112,10 +113,15 @@ export function reduceConnectionLifecycle( : { kind: 'refreshing', epoch: state.epoch, detail: event.detail, resume: state }; case 'refresh-succeeded': + case 'refresh-failed': if (!isCurrent(state, event) || state.kind !== 'refreshing') return state; return state.resume; case 'transport-connected': + if (isCurrent(state, event) && state.kind === 'refreshing') { + if (state.resume.kind === 'connected') return state; + return { ...state, resume: stable('connected', state.epoch, event.detail) }; + } if (!isCurrent(state, event) || (state.kind !== 'starting' && state.kind !== 'offline')) return state; return stable('connected', state.epoch, event.detail); @@ -125,6 +131,10 @@ export function reduceConnectionLifecycle( || state.kind === 'auth-required' || state.kind === 'signed-out' || state.kind === 'reauthenticating') return state; + if (state.kind === 'refreshing') { + if (state.resume.kind === 'offline') return state; + return { ...state, resume: stable('offline', state.epoch, event.detail) }; + } return stable('offline', state.epoch, event.detail); case 'auth-required': diff --git a/src/styles.css b/src/styles.css index 4d094762..fd3f1fb5 100644 --- a/src/styles.css +++ b/src/styles.css @@ -3593,7 +3593,11 @@ table.res-table tbody tr:hover td.idx { background: var(--bg-hover); } .dash-grid.is-report .dash-tile { min-height: 300px; } } @media (max-width: 520px) { - .connection-chip { display: none; } + .connection-chip { max-width: 132px; padding-inline: 5px; } + .connection-chip .connection-state { + min-width: 0; overflow: hidden; text-overflow: ellipsis; + } + .app-header .user-btn .user-short { display: none; } .app-header .lib-name { max-width: 96px; } } @media (max-width: 520px) { diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index 45290b27..545004da 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import type { Mock } from 'vitest'; +import type { Signal } from '@preact/signals-core'; import dagre from '@dagrejs/dagre'; import { createApp } from '../../src/ui/app.js'; import { createCodeMirrorEditor } from '../../src/editor/codemirror-adapter.js'; @@ -36,6 +37,7 @@ import type { DashboardDocumentV2, SavedQueryV2, StoredWorkspaceV5, } from '../../src/generated/json-schema.types.js'; import type { CompletionItem, AssembledReference } from '../../src/core/completions.js'; +import type { ConnectionLifecycleState } from '../../src/core/connection-lifecycle.js'; import type { QueryResult, ScriptResult, ScriptExportResult, ScriptEntry, ScriptExportEntry, ResultSchemaGraph, } from '../../src/ui/results.js'; @@ -1168,6 +1170,32 @@ describe('renderApp shell', () => { expect(qs(app.root, '.conn-status').dataset.connectionState).toBe('offline'); expect(qs(app.root, '.conn-status').classList.contains('tone-offline')).toBe(true); }); + it('projects all seven lifecycle states into the mounted accessible header chip', () => { + const app = createApp(env()); + app.renderApp(); + const lifecycle = app.conn.connection as Signal; + const cases: Array<[ConnectionLifecycleState, string, string, string]> = [ + [{ kind: 'starting', epoch: 1 }, 'Connecting…', 'connecting', 'tone-warning'], + [{ kind: 'connected', epoch: 1 }, 'Connected', 'connected', 'tone-success'], + [{ + kind: 'refreshing', epoch: 1, resume: { kind: 'connected', epoch: 1 }, + }, 'Refreshing…', 'refreshing', 'tone-warning'], + [{ kind: 'offline', epoch: 1 }, 'Offline', 'offline', 'tone-offline'], + [{ kind: 'auth-required', epoch: 2 }, 'Sign in required', 'authentication required', 'tone-error'], + [{ kind: 'reauthenticating', epoch: 3 }, 'Signing in…', 'reauthenticating', 'tone-warning'], + [{ kind: 'signed-out', epoch: 4 }, 'Signed out', 'signed out', 'tone-neutral'], + ]; + for (const [value, label, ariaState, tone] of cases) { + lifecycle.value = value; + const chip = qs(app.root, '.conn-status'); + expect(chip.dataset.connectionState).toBe(value.kind); + expect(chip.classList.contains(`is-${value.kind}`)).toBe(true); + expect(chip.classList.contains(tone)).toBe(true); + expect(qs(chip, '.connection-state').textContent).toBe(label); + expect(chip.getAttribute('aria-label')).toBe(`ClickHouse connection: ${ariaState}`); + expect(chip.title).toBe('ch.example'); + } + }); it('toggles theme via the header button', () => { const { app } = rendered(); app.dom.themeBtn!.dispatchEvent(new Event('click')); // default light → dark diff --git a/tests/unit/connection-lifecycle-boundaries.test.js b/tests/unit/connection-lifecycle-boundaries.test.js new file mode 100644 index 00000000..816fe4b0 --- /dev/null +++ b/tests/unit/connection-lifecycle-boundaries.test.js @@ -0,0 +1,30 @@ +// Mechanical connection-authority/status contracts (#512 Phase 1). The +// runtime tests prove behavior; this Node-tooling spec makes the architectural +// prohibition and narrow-phone visibility difficult to accidentally bypass. + +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), '../..'); +const read = (file) => readFileSync(join(repoRoot, file), 'utf8'); + +describe('connection lifecycle architecture', () => { + it('never infers lifecycle/readiness from serverVersion in authority or projection modules', () => { + const authorityFiles = [ + 'src/core/connection-lifecycle.ts', + 'src/application/connection-session.ts', + 'src/net/ch-client.ts', + 'src/ui/app-header.ts', + 'src/ui/app-shell.ts', + ]; + const violations = authorityFiles.filter((file) => /\bserverVersion\b/.test(read(file))); + expect(violations).toEqual([]); + }); + + it('does not hide the authoritative connection chip at the narrow-phone breakpoint', () => { + const css = read('src/styles.css'); + expect(css).not.toMatch(/\.connection-chip\s*\{\s*display:\s*none\s*;\s*\}/); + }); +}); diff --git a/tests/unit/connection-lifecycle.test.ts b/tests/unit/connection-lifecycle.test.ts index bcd96bfa..f7f34ad4 100644 --- a/tests/unit/connection-lifecycle.test.ts +++ b/tests/unit/connection-lifecycle.test.ts @@ -51,6 +51,16 @@ describe('connection lifecycle reducer', () => { } }); + it('returns the latest retained transport state after a failed refresh', () => { + const refreshing = reduceConnectionLifecycle(connected(), { type: 'begin-refresh', epoch: 4 }); + const wentOffline = reduceConnectionLifecycle(refreshing, { + type: 'transport-offline', epoch: 4, detail: 'network', + }); + expect(reduceConnectionLifecycle(wentOffline, { + type: 'refresh-failed', epoch: 4, + })).toEqual({ kind: 'offline', epoch: 4, detail: 'network' }); + }); + it('moves current starting/offline transport to connected', () => { expect(reduceConnectionLifecycle(starting(), { type: 'transport-connected', epoch: 4 })) .toEqual({ kind: 'connected', epoch: 4 }); @@ -58,13 +68,57 @@ describe('connection lifecycle reducer', () => { .toEqual({ kind: 'connected', epoch: 4, detail: 'probe' }); }); + it('retains current connected transport evidence while refresh remains in flight', () => { + for (const resume of [starting(), offline()]) { + const refreshing = reduceConnectionLifecycle(resume, { type: 'begin-refresh', epoch: 4 }); + const connectedDuringRefresh = reduceConnectionLifecycle(refreshing, { + type: 'transport-connected', epoch: 4, detail: 'query', + }); + expect(connectedDuringRefresh).toMatchObject({ + kind: 'refreshing', + resume: { kind: 'connected', epoch: 4, detail: 'query' }, + }); + expect(reduceConnectionLifecycle(connectedDuringRefresh, { + type: 'refresh-succeeded', epoch: 4, + })).toEqual({ kind: 'connected', epoch: 4, detail: 'query' }); + } + const alreadyConnected = { + kind: 'refreshing' as const, epoch: 4, resume: connected(), + }; + expect(reduceConnectionLifecycle(alreadyConnected, { + type: 'transport-connected', epoch: 4, + })).toBe(alreadyConnected); + }); + it('moves current non-protected transport state offline', () => { - for (const state of [starting(), connected(), { kind: 'refreshing', epoch: 4, resume: connected() } as ConnectionLifecycleState]) { + for (const state of [starting(), connected()]) { expect(reduceConnectionLifecycle(state, { type: 'transport-offline', epoch: 4, detail: 'network' })) .toEqual({ kind: 'offline', epoch: 4, detail: 'network' }); } }); + it('retains current offline transport evidence while refresh remains in flight', () => { + for (const resume of [starting(), connected()]) { + const refreshing = reduceConnectionLifecycle(resume, { type: 'begin-refresh', epoch: 4 }); + const offlineDuringRefresh = reduceConnectionLifecycle(refreshing, { + type: 'transport-offline', epoch: 4, detail: 'network', + }); + expect(offlineDuringRefresh).toMatchObject({ + kind: 'refreshing', + resume: { kind: 'offline', epoch: 4, detail: 'network' }, + }); + expect(reduceConnectionLifecycle(offlineDuringRefresh, { + type: 'refresh-succeeded', epoch: 4, + })).toEqual({ kind: 'offline', epoch: 4, detail: 'network' }); + } + const alreadyOffline = { + kind: 'refreshing' as const, epoch: 4, resume: offline(), + }; + expect(reduceConnectionLifecycle(alreadyOffline, { + type: 'transport-offline', epoch: 4, + })).toBe(alreadyOffline); + }); + it('does not let transport offline overwrite authentication or sign-out states', () => { for (const state of [authRequired(), signedOut(), reauthenticating()]) { expect(reduceConnectionLifecycle(state, { type: 'transport-offline', epoch: 4 })).toBe(state); @@ -88,12 +142,13 @@ describe('connection lifecycle reducer', () => { const cases: Array<[ConnectionLifecycleState, Parameters[1]]> = [ [connected(), { type: 'begin-refresh', epoch: 3 }], [connected(), { type: 'refresh-succeeded', epoch: 4 }], + [connected(), { type: 'refresh-failed', epoch: 4 }], [connected(), { type: 'transport-connected', epoch: 4 }], [offline(), { type: 'auth-required', epoch: 3 }], [signedOut(), { type: 'auth-required', epoch: 4 }], [reauthenticating(), { type: 'failed-authentication', epoch: 3, prior: signedOut() }], [connected(), { type: 'failed-authentication', epoch: 4, prior: authRequired() }], - [refreshing, { type: 'transport-connected', epoch: 4 }], + [refreshing, { type: 'transport-connected', epoch: 3 }], ]; for (const [state, event] of cases) expect(reduceConnectionLifecycle(state, event)).toBe(state); }); From 0765c685a329f7e44a2af759266ffba0ab8cac40 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 28 Jul 2026 15:54:42 +0200 Subject: [PATCH 3/4] test(#512): enforce exclusive lifecycle projection --- build/check-boundaries.mjs | 21 +++++++++++++++++++ .../connection-lifecycle-boundaries.test.js | 21 ++++++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/build/check-boundaries.mjs b/build/check-boundaries.mjs index 7f900500..2f807eda 100644 --- a/build/check-boundaries.mjs +++ b/build/check-boundaries.mjs @@ -199,6 +199,27 @@ for (const relFile of connectionAuthorityFiles) { } } +// The DOM projection is exclusive too. The retired bridge lived in app.ts: +// catalog version metadata called back into the composition root, which then +// mutated the chip. Keep the handle declaration in app.types.ts, but reject +// every chip selector/projector reference outside its pure projector and +// header renderer. This catches that exact regression without banning +// legitimate server-version capability data or the user-menu version label. +const connectionProjectionOwners = new Set([ + 'src/core/connection-lifecycle.ts', + 'src/ui/app-header.ts', + 'src/ui/app.types.ts', +]); +const connectionProjectionPattern = + /\bconnStatus\b|connection-(?:state|chip)|data-connection-state|\bprojectConnectionLifecycle\b/; +for (const file of collectFiles(path.join(repoRoot, 'src'))) { + const relFile = path.relative(repoRoot, file).split(path.sep).join('/'); + if (connectionProjectionOwners.has(relFile)) continue; + if (connectionProjectionPattern.test(fs.readFileSync(file, 'utf8'))) { + violations.push(`${relFile} → connection-chip projection (issue #512: only app-header may render lifecycle state)`); + } +} + if (violations.length) { console.error('check-boundaries: architecture violations:'); for (const line of violations) console.error(` ${line}`); diff --git a/tests/unit/connection-lifecycle-boundaries.test.js b/tests/unit/connection-lifecycle-boundaries.test.js index 816fe4b0..fe9a8a01 100644 --- a/tests/unit/connection-lifecycle-boundaries.test.js +++ b/tests/unit/connection-lifecycle-boundaries.test.js @@ -3,12 +3,15 @@ // prohibition and narrow-phone visibility difficult to accidentally bypass. import { describe, expect, it } from 'vitest'; -import { readFileSync } from 'node:fs'; +import { readdirSync, readFileSync, statSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), '../..'); const read = (file) => readFileSync(join(repoRoot, file), 'utf8'); +const sourceFiles = (path) => statSync(path).isFile() + ? [path] + : readdirSync(path).flatMap((entry) => sourceFiles(join(path, entry))); describe('connection lifecycle architecture', () => { it('never infers lifecycle/readiness from serverVersion in authority or projection modules', () => { @@ -23,6 +26,22 @@ describe('connection lifecycle architecture', () => { expect(violations).toEqual([]); }); + it('keeps connection-chip rendering exclusive to the lifecycle projector and header', () => { + const owners = new Set([ + 'src/core/connection-lifecycle.ts', + 'src/ui/app-header.ts', + 'src/ui/app.types.ts', + ]); + const projectionPattern = + /\bconnStatus\b|connection-(?:state|chip)|data-connection-state|\bprojectConnectionLifecycle\b/; + const violations = sourceFiles(join(repoRoot, 'src')) + .map((file) => file.slice(repoRoot.length + 1)) + .filter((file) => /\.(?:ts|tsx|js|mjs)$/.test(file)) + .filter((file) => !owners.has(file)) + .filter((file) => projectionPattern.test(read(file))); + expect(violations).toEqual([]); + }); + it('does not hide the authoritative connection chip at the narrow-phone breakpoint', () => { const css = read('src/styles.css'); expect(css).not.toMatch(/\.connection-chip\s*\{\s*display:\s*none\s*;\s*\}/); From 6b7db2ea466524e23c21a56bfc2ca175af82d54d Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 28 Jul 2026 15:56:18 +0200 Subject: [PATCH 4/4] test(#512): cover lifecycle projector selectors --- build/check-boundaries.mjs | 2 +- tests/unit/connection-lifecycle-boundaries.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/build/check-boundaries.mjs b/build/check-boundaries.mjs index 2f807eda..8e26db5b 100644 --- a/build/check-boundaries.mjs +++ b/build/check-boundaries.mjs @@ -211,7 +211,7 @@ const connectionProjectionOwners = new Set([ 'src/ui/app.types.ts', ]); const connectionProjectionPattern = - /\bconnStatus\b|connection-(?:state|chip)|data-connection-state|\bprojectConnectionLifecycle\b/; + /\bconnStatus\b|conn-status|connection-(?:state|chip)|data-connection-state|\bconnectionLifecyclePresentation\b/; for (const file of collectFiles(path.join(repoRoot, 'src'))) { const relFile = path.relative(repoRoot, file).split(path.sep).join('/'); if (connectionProjectionOwners.has(relFile)) continue; diff --git a/tests/unit/connection-lifecycle-boundaries.test.js b/tests/unit/connection-lifecycle-boundaries.test.js index fe9a8a01..9fb21c33 100644 --- a/tests/unit/connection-lifecycle-boundaries.test.js +++ b/tests/unit/connection-lifecycle-boundaries.test.js @@ -33,7 +33,7 @@ describe('connection lifecycle architecture', () => { 'src/ui/app.types.ts', ]); const projectionPattern = - /\bconnStatus\b|connection-(?:state|chip)|data-connection-state|\bprojectConnectionLifecycle\b/; + /\bconnStatus\b|conn-status|connection-(?:state|chip)|data-connection-state|\bconnectionLifecyclePresentation\b/; const violations = sourceFiles(join(repoRoot, 'src')) .map((file) => file.slice(repoRoot.length + 1)) .filter((file) => /\.(?:ts|tsx|js|mjs)$/.test(file))