diff --git a/CHANGELOG.md b/CHANGELOG.md index 18f42452..16668b78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,17 +19,21 @@ auto-generated per-PR notes; this file is the curated, human-readable history. then consumes the checkpoint. Results, ClickHouse session ids, credentials, and other execution state are never checkpointed. Failed callbacks retain the drafts for a state-rebound retry; malformed, expired, or - workspace-mismatched payloads fail closed into the normal boot path. The - separate tab-scoped, 15-minute validated-callback marker contains only state - and validation time; a checkpoint alone is never retry authority, and expiry + workspace-mismatched payloads fail closed without deletion until their + matching workspace returns. The separate tab-scoped, 15-minute + validated-callback marker binds state and validation time to a canonical + live-document fingerprint; a checkpoint alone is never retry authority, and expiry is logical eligibility rather than a promise of eager deletion. Unavailable workspaces and prepublication storage/validation failures retain unpublished recovery. Valid pending recovery suppresses legacy shared content and retries - after authoritative workspace load without overwriting newer save-relevant - dirty RAM; its marker is retired before publication. A new OAuth attempt - invalidates older authority. The intentional redirect receives a one-shot - unload bypass only after storage succeeds, and explicit Log out clears the - checkpoint and marker. + after authoritative workspace load without overwriting a newer document + session—even after that work is saved clean. Changed sessions require an + explicit Restore drafts action; the marker is retired before publication. + Callback mismatch does not revoke unrelated pending authority. A new OAuth + attempt invalidates older authority. Redirect preparation is transactional + across checkpoint, marker, IdP, origin, verifier, state, and return route; the + intentional redirect receives a one-shot unload bypass only after storage + succeeds, and explicit Log out clears the checkpoint and marker. - **Temporary ClickHouse authentication loss now suspends only authenticated execution, not the in-memory document session** (#512 phase 2, absorbing #502/#520/#522). A disposable, epoch-fenced execution scope coordinates diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e2d0b533..d492572d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -113,9 +113,14 @@ Spec drafts, editor mode, dirty flags, and saved-query reconciliation metadata. It never contains credentials, results, ClickHouse session ids, result-column metadata, or running/export state. A retained checkpoint is rebound to a new OAuth state on retry without replacing its drafts. Retry authority is kept -separately in a tab-scoped, TTL-bound validated-callback marker containing only -the callback state and validation time; a checkpoint alone never authorizes an -automatic restore. Starting a new OAuth attempt invalidates an older marker. +separately in a tab-scoped, TTL-bound validated-callback marker containing the +callback state, validation time, and a compact fingerprint of the authored +document session at validation. The fingerprint fences automatic publication; +it is a change detector, not authorization material, and a checkpoint alone +never authorizes an automatic restore. Starting a new OAuth attempt invalidates +an older marker. Version-1 markers lack the fingerprint, so they are not +migrated: they are unsupported and cleared, while a retained checkpoint can be +rebound by a later OAuth retry that writes a version-2 marker. After a successful callback, bootstrap restores the return route and loads the committed workspace normally. It then applies a matching checkpoint, reconciles @@ -123,17 +128,20 @@ linked tabs, and revalidates every raw Spec draft before the first signed-in render; invalid in-progress Spec text remains byte-for-byte authored while its diagnostics are rebuilt. Workspace-unavailable and prepublication storage or validator failures retain the recovery unpublished. A valid pending recovery -suppresses legacy shared content and retries only after an authoritative -workspace load; it never replaces newer save-relevant dirty work in RAM. Its -validated-callback marker is retired before publication. The ordinary dirty -unload guard is restored before the checkpoint is consumed. Malformed, -unsupported, logically expired after 15 minutes, or workspace-mismatched -checkpoints are ineligible and cleared best-effort while normal bootstrap -continues; an OAuth-state mismatch is not consumed, so an older callback cannot -apply or destroy a newer retry. TTL expiry controls restore eligibility and -does not promise eager physical deletion from browser storage. The one-shot -unload bypass is armed only after a durable checkpoint write; a write failure -leaves navigation and the normal dirty guard untouched. +suppresses legacy shared content and retries after an authoritative workspace +load only while the document session still has the marker's fingerprint. If it +changed, automatic publication stops and the shell offers an explicit +**Restore drafts** action; automatic recovery never replaces newer work in RAM. +Workspace-mismatched checkpoints and their callback markers are retained for a +later matching workspace rather than cleared. Its validated-callback marker is +retired before publication. The ordinary dirty unload guard is restored before +the checkpoint is consumed. Malformed, unsupported, or logically expired after +15 minutes checkpoints are ineligible and cleared best-effort while normal +bootstrap continues; an OAuth-state mismatch is not consumed, so an older +callback cannot apply or destroy a newer retry. TTL expiry controls restore +eligibility and does not promise eager physical deletion from browser storage. +The one-shot unload bypass is armed only after a durable checkpoint write; a +write failure leaves navigation and the normal dirty guard untouched. Explicit Log out remains the separate destructive policy: it closes the scope, tears down the workbench and Dashboard, clears credentials and any pending diff --git a/docs/LOGIN-SCREEN.md b/docs/LOGIN-SCREEN.md index 3698c674..6c18a1ca 100644 --- a/docs/LOGIN-SCREEN.md +++ b/docs/LOGIN-SCREEN.md @@ -53,9 +53,15 @@ only after that checkpoint write succeeds. If serialization or storage fails, the redirect does not start, the warning remains armed, and the inline controls show the failure so sign-in can be retried. A failed IdP callback keeps the draft checkpoint; a retry binds the same authored payload to its new OAuth -state. A separate tab-scoped, TTL-bound validated-callback marker contains only -that state and its validation time; the checkpoint alone never authorizes an -automatic restore. Starting another OAuth attempt invalidates the older marker. +state. A separate tab-scoped, TTL-bound validated-callback marker contains the +callback state, its validation time, and a compact fingerprint of the authored +document session at that point. The fingerprint is a change detector, not an +authorization secret; the checkpoint alone never authorizes an automatic +restore. Starting another OAuth attempt invalidates the older marker. Markers +written by the previous version-1 format lack that fingerprint, so they are +treated as unsupported and cleared rather than migrated; the retained +checkpoint can be rebound by a later OAuth retry, which writes a version-2 +marker. After a successful callback, the app restores the route, loads the committed workspace, validates the checkpoint against that workspace and callback state, @@ -64,14 +70,17 @@ signed-in render. Invalid JSON drafts survive as authored and regain normal diagnostics. If the workspace is unavailable, or storage/validation fails before publication, recovery remains unpublished and retryable. A valid pending recovery suppresses legacy shared content and retries automatically only after -an authoritative workspace load; it never overwrites newer save-relevant dirty -work in memory, and its marker is retired before publication. Results and other -execution state do not survive an OAuth reload. Malformed, unsupported, -logically expired (after 15 minutes), or workspace-mismatched checkpoints are -ineligible and the normal signed-in flow continues; expiry does not promise -eager physical removal from browser storage. A stale callback cannot consume a -newer retry's checkpoint. Explicit **Log out** clears any pending checkpoint -and validated-callback marker. +an authoritative workspace load while the document session is unchanged. If +that session changed after callback validation, the checkpoint is retained and +the user must choose **Restore drafts** explicitly; automatic recovery never +overwrites newer work in memory. A workspace-mismatched checkpoint is likewise +retained, with its callback authority, for a later matching workspace rather +than discarded. Its marker is retired before publication. Results and other +execution state do not survive an OAuth reload. Malformed, unsupported, or +logically expired (after 15 minutes) checkpoints are ineligible; expiry does +not promise eager physical removal from browser storage. A stale callback +cannot consume a newer retry's checkpoint. Explicit **Log out** clears any +pending checkpoint and validated-callback marker. ### Multiple IdPs diff --git a/src/application/connection-session.ts b/src/application/connection-session.ts index b48f23f1..bf789051 100644 --- a/src/application/connection-session.ts +++ b/src/application/connection-session.ts @@ -48,6 +48,11 @@ export interface SessionStorageLike { * a cast; a test stub only implements two members. */ export type SessionCrypto = PkceCrypto; +export interface OAuthRedirectPreparation { + hasRecoverySnapshot: boolean; + rollback(): void; +} + /** Every side effect this session needs, injected as a narrow bag — mirrors * `query-execution-service.ts`'s own `QueryExecutionDeps` seam. Production * wires the real browser/env objects; tests inject plain stubs. */ @@ -66,7 +71,7 @@ export interface ConnectionSessionDeps { /** Persist an OAuth-state-bound document recovery snapshot after this * session has written its PKCE attempt. `true` means that snapshot is * durable and the intentional redirect may bypass the dirty unload guard. */ - prepareOAuthRedirect?(oauthState: string): boolean; + prepareOAuthRedirect?(oauthState: string): boolean | OAuthRedirectPreparation; /** Arm a one-shot dirty-unload bypass for the immediately following OAuth * navigation. The returned callback undoes the arm if navigation fails. */ armOAuthRedirectUnloadBypass?(): () => void; @@ -314,10 +319,13 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection async function beginOAuth(idpArg?: string, targetOrigin?: string): Promise { const prior = authenticationPrior(); const authenticating = transition({ type: 'start-authentication' }); - const attemptKeys = ['oauth_verifier', 'oauth_state', 'oauth_return_route'] as const; + const attemptKeys = [ + 'oauth_verifier', 'oauth_state', 'oauth_return_route', 'oauth_idp', 'oauth_origin', + ] as const; type AttemptValues = Record; let priorAttemptValues: AttemptValues | null = null; let ownedAttemptValues: AttemptValues | null = null; + let rollbackOAuthPreparation: (() => void) | undefined; let disarmUnloadBypass: (() => void) | undefined; const ownsStoredAttempt = (): boolean => { const current = connectionSignal.value; @@ -334,17 +342,20 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection if (!ownsStoredAttempt()) throw new Error('Authentication attempt superseded'); }; try { - // Keep these three legacy keys as the one canonical OAuth attempt. A + // Keep the OAuth attempt and its selected endpoint as one transaction. A // failed post-PKCE preparation must leave a previous callback/retry // attempt exactly as it was, including absent keys. priorAttemptValues = { oauth_verifier: ss.getItem('oauth_verifier'), oauth_state: ss.getItem('oauth_state'), oauth_return_route: ss.getItem('oauth_return_route'), + oauth_idp: ss.getItem('oauth_idp'), + oauth_origin: ss.getItem('oauth_origin'), }; ownedAttemptValues = { ...priorAttemptValues }; if (idpArg) { selectIdp(idpArg); + ownedAttemptValues.oauth_idp = idpArg; assertAttemptOwnership(); } // A picked saved-connection can target another cluster: stash its origin so @@ -352,6 +363,7 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection // 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'); + ownedAttemptValues.oauth_origin = targetOrigin || null; assertAttemptOwnership(); const cfg = await resolveConfig(); assertCurrentAuthentication(authenticating.epoch); @@ -382,9 +394,11 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection ss.setItem('oauth_return_route', returnRoute); ownedAttemptValues.oauth_return_route = returnRoute; assertAttemptOwnership(); - const hasRecoverySnapshot = deps.prepareOAuthRedirect - ? deps.prepareOAuthRedirect(state) - : false; + const prepared = deps.prepareOAuthRedirect?.(state) ?? false; + const hasRecoverySnapshot = typeof prepared === 'boolean' + ? prepared + : prepared.hasRecoverySnapshot; + rollbackOAuthPreparation = typeof prepared === 'boolean' ? undefined : prepared.rollback; assertAttemptOwnership(); // Deliberately adjacent to navigation: no unrelated asynchronous work // may run after the one-shot bypass is armed. @@ -407,6 +421,11 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection // The original redirect failure is the actionable error. Continue // cleanup even when a host-provided disarm callback misbehaves. } + try { + if (rollbackOAuthPreparation && ownsStoredAttempt()) rollbackOAuthPreparation(); + } catch { + // Preserve the redirect error and continue restoring the OAuth attempt. + } if (priorAttemptValues && ownsStoredAttempt()) { for (const key of attemptKeys) { try { @@ -414,6 +433,7 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection if (value === null) ss.removeItem(key); else ss.setItem(key, value); ownedAttemptValues![key] = value; + if (key === 'oauth_idp') idpId = value; } catch { // A failed restore may have mutated storage before throwing. Stop // rather than risk overwriting a reentrant/newer attempt. diff --git a/src/application/oauth-document-recovery-session.ts b/src/application/oauth-document-recovery-session.ts index 551e5751..a6b29334 100644 --- a/src/application/oauth-document-recovery-session.ts +++ b/src/application/oauth-document-recovery-session.ts @@ -60,7 +60,8 @@ export type OAuthDocumentRecoveryRestoreResult = | { kind: 'workspace-unavailable-retained' } /** Pending callback authority could not be retired safely before publication. */ | { kind: 'retry-deferred-retained' } - | { kind: 'workspace-mismatch-cleared' } + | { kind: 'document-session-changed-retained' } + | { kind: 'workspace-mismatch-retained' } /** The snapshot deliberately remains until the shell has installed its guard. */ | { kind: 'restored' }; @@ -71,6 +72,8 @@ export interface OAuthDocumentRecoverySession { * intentionally allowed to throw: ConnectionSession must then abort redirect. */ prepare(oauthState: string): boolean; + /** Prepare plus an exact rollback for failures before navigation commits. */ + prepareTransaction(oauthState: string): { hasRecoverySnapshot: boolean; rollback(): void }; /** * Restore only after OAuth callback validation and workspace loading. A * restored payload is not consumed automatically; call consume once the @@ -82,7 +85,10 @@ export interface OAuthDocumentRecoverySession { * fresh in-memory callback authority or a strict persisted callback marker. * A checkpoint by itself is never authority and produces `kind: 'absent'`. */ - retryPending(workspace: StoredWorkspaceV5 | null): OAuthDocumentRecoveryRestoreResult; + retryPending( + workspace: StoredWorkspaceV5 | null, + options?: { allowChangedDocumentSession?: boolean }, + ): OAuthDocumentRecoveryRestoreResult; /** Remove the payload after the caller's post-restore work succeeded. */ consume(): void; /** Explicit logout/end-session cleanup. */ @@ -108,6 +114,23 @@ function authoredTab(tab: QueryTab): OAuthDocumentRecoveryTab { }; } +export function oauthDocumentSessionFingerprint(state: OAuthDocumentRecoveryState): string { + const authored = JSON.stringify({ + tabs: state.tabs.value.map(authoredTab), + activeTabId: state.activeTabId.value, + nextTabId: state.nextTabId, + }); + // Persist a compact comparison token, not a second copy of authored SQL/Spec. + // FNV-1a 64 is deterministic across reloads; this is a change detector, not + // an authenticity boundary (callback state remains the authority). + let hash = 0xcbf29ce484222325n; + for (let index = 0; index < authored.length; index += 1) { + hash ^= BigInt(authored.charCodeAt(index)); + hash = BigInt.asUintN(64, hash * 0x100000001b3n); + } + return hash.toString(16).padStart(16, '0'); +} + function freshTab(tab: OAuthDocumentRecoveryTab): QueryTab { // Start from the application's single canonical constructor. In particular, // this keeps result/chSession/column metadata and Spec diagnostics transient. @@ -168,6 +191,7 @@ export function createOAuthDocumentRecoverySession( version: OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION, oauthState, validatedAt, + documentSessionFingerprint: oauthDocumentSessionFingerprint(deps.state), }; pendingValidatedCallback = marker; try { @@ -296,6 +320,32 @@ export function createOAuthDocumentRecoverySession( return true; } + function prepareTransaction( + oauthState: string, + ): { hasRecoverySnapshot: boolean; rollback(): void } { + const checkpoint = deps.storage.getItem(OAUTH_DOCUMENT_RECOVERY_KEY); + const marker = deps.storage.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY); + const pending = pendingValidatedCallback; + const rollback = (): void => { + if (checkpoint === null) deps.storage.removeItem(OAUTH_DOCUMENT_RECOVERY_KEY); + else deps.storage.setItem(OAUTH_DOCUMENT_RECOVERY_KEY, checkpoint); + if (marker === null) deps.storage.removeItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY); + else deps.storage.setItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY, marker); + pendingValidatedCallback = pending; + }; + let hasRecoverySnapshot: boolean; + try { + hasRecoverySnapshot = prepare(oauthState); + } catch (error) { + rollback(); + throw error; + } + return { + hasRecoverySnapshot, + rollback, + }; + } + function restoreCheckpoint( callbackState: string, workspace: StoredWorkspaceV5 | null, @@ -320,8 +370,7 @@ export function createOAuthDocumentRecoverySession( if (workspace === null) return { kind: 'workspace-unavailable-retained' }; if (decoded.value.workspaceId !== workspace.id || decoded.value.workspaceKey !== workspace.key) { - deps.storage.removeItem(OAUTH_DOCUMENT_RECOVERY_KEY); - return { kind: 'workspace-mismatch-cleared' }; + return { kind: 'workspace-mismatch-retained' }; } let candidate: Signal; @@ -361,9 +410,10 @@ export function createOAuthDocumentRecoverySession( const now = deps.now(); const result = restoreCheckpoint(callbackState, workspace, now); if (result.kind === 'workspace-unavailable-retained' + || result.kind === 'workspace-mismatch-retained' || result.kind === 'retry-deferred-retained') { rememberValidatedCallback(callbackState, now); - } else { + } else if (result.kind !== 'callback-mismatch') { // A fresh validated callback supersedes any earlier pending authority. // Publication, absence, invalidity, and proven mismatch are all terminal. clearPendingBestEffort(); @@ -373,17 +423,22 @@ export function createOAuthDocumentRecoverySession( function retryPending( workspace: StoredWorkspaceV5 | null, + { allowChangedDocumentSession = false }: { allowChangedDocumentSession?: boolean } = {}, ): OAuthDocumentRecoveryRestoreResult { const now = deps.now(); const pending = pendingCallback(now); if (pending.kind === 'absent') return { kind: 'absent' }; if (pending.kind === 'deferred') return { kind: 'retry-deferred-retained' }; - // Never let an automatic retry replace save-relevant work authored in RAM - // after the callback was deferred. This gate precedes checkpoint access, - // candidate construction, reconciliation, and authority retirement. - if (deps.state.tabs.value.some(tabSaveDirty)) { + if (!allowChangedDocumentSession && deps.state.tabs.value.some(tabSaveDirty)) { return { kind: 'retry-deferred-retained' }; } + // Dirty flags are not a revision: saving or closing newer work can make the + // session clean again. Compare the complete authored state observed when + // callback authority was deferred before allowing automatic publication. + if (!allowChangedDocumentSession + && oauthDocumentSessionFingerprint(deps.state) !== pending.value.documentSessionFingerprint) { + return { kind: 'document-session-changed-retained' }; + } const marker = pending.value; const result = restoreCheckpoint( marker.oauthState, @@ -392,6 +447,7 @@ export function createOAuthDocumentRecoverySession( () => retirePendingBeforePublish(marker), ); if (result.kind !== 'workspace-unavailable-retained' + && result.kind !== 'workspace-mismatch-retained' && result.kind !== 'retry-deferred-retained' && result.kind !== 'restored') { clearPendingBestEffort(); @@ -399,5 +455,5 @@ export function createOAuthDocumentRecoverySession( return result; } - return { prepare, restore, retryPending, consume, clear }; + return { prepare, prepareTransaction, restore, retryPending, consume, clear }; } diff --git a/src/core/oauth-document-recovery.ts b/src/core/oauth-document-recovery.ts index eae5ec88..b91213c5 100644 --- a/src/core/oauth-document-recovery.ts +++ b/src/core/oauth-document-recovery.ts @@ -7,7 +7,7 @@ export const OAUTH_DOCUMENT_RECOVERY_VERSION = 1; export const OAUTH_DOCUMENT_RECOVERY_TTL_MS = 15 * 60 * 1000; export const OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY = 'oauth_document_recovery_validated_callback'; -export const OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION = 1; +export const OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION = 2; export const OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_TTL_MS = OAUTH_DOCUMENT_RECOVERY_TTL_MS; @@ -51,6 +51,7 @@ export interface OAuthDocumentRecoveryValidatedCallback { version: typeof OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION; oauthState: string; validatedAt: number; + documentSessionFingerprint: string; } export type OAuthDocumentRecoveryInvalidReason = 'malformed' | 'unsupported' | 'expired'; @@ -76,7 +77,9 @@ type ValidatedCallbackShapeResult = const ROOT_KEYS = [ 'version', 'createdAt', 'workspaceId', 'workspaceKey', 'oauthState', 'tabs', 'activeTabId', 'nextTabId', ]; -const VALIDATED_CALLBACK_KEYS = ['version', 'oauthState', 'validatedAt']; +const VALIDATED_CALLBACK_KEYS = [ + 'version', 'oauthState', 'validatedAt', 'documentSessionFingerprint', +]; const TAB_KEYS = [ 'id', 'doc', 'name', 'sqlDraft', 'specText', 'specVersion', 'editorMode', 'dirtySql', 'dirtySpec', 'savedId', ]; @@ -176,7 +179,8 @@ function readValidatedCallbackShape( if (value.version !== OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION) { return { kind: 'invalid', reason: 'unsupported' }; } - if (!nonBlank(value.oauthState) || !finiteInteger(value.validatedAt)) { + if (!nonBlank(value.oauthState) || !finiteInteger(value.validatedAt) + || typeof value.documentSessionFingerprint !== 'string') { return { kind: 'invalid', reason: 'malformed' }; } return { @@ -185,6 +189,7 @@ function readValidatedCallbackShape( version: OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION, oauthState: value.oauthState, validatedAt: value.validatedAt, + documentSessionFingerprint: value.documentSessionFingerprint, }, }; } @@ -273,7 +278,7 @@ export function decodeOAuthDocumentRecovery( return shape; } -/** Serialize only the callback proof's version, OAuth state, and validation time. */ +/** Serialize callback authority together with its live-session publication fence. */ export function encodeOAuthDocumentRecoveryValidatedCallback( marker: OAuthDocumentRecoveryValidatedCallback, ): string { @@ -282,6 +287,7 @@ export function encodeOAuthDocumentRecoveryValidatedCallback( version: value.version, oauthState: value.oauthState, validatedAt: value.validatedAt, + documentSessionFingerprint: value.documentSessionFingerprint, }); } diff --git a/src/ui/app.ts b/src/ui/app.ts index cf1c7c2a..484dd7d8 100644 --- a/src/ui/app.ts +++ b/src/ui/app.ts @@ -508,6 +508,26 @@ export function createApp(env: CreateAppEnv = {}): App { // owns callback precedence, so callers discard the legacy share handoff. return deferOAuthDocumentRecovery(); } + if (pending.kind === 'document-session-changed-retained') { + flashToast( + 'Recovered drafts were kept because this document session changed.', + { + document: doc, + action: { + label: 'Restore drafts', + onClick: () => { + const forced = oauthDocumentRecovery.retryPending( + app.currentWorkspace, + { allowChangedDocumentSession: true }, + ); + finalizeOAuthDocumentRecovery(forced); + app.renderCurrentSurface(); + }, + }, + }, + ); + return pending; + } deferredRecoveryWarningShown = false; return finalizeOAuthDocumentRecovery(pending); }; @@ -668,7 +688,7 @@ export function createApp(env: CreateAppEnv = {}): App { closing?.close(lease); revealAuthenticationRequired(detail); }, - prepareOAuthRedirect: (state) => oauthDocumentRecovery.prepare(state), + prepareOAuthRedirect: (state) => oauthDocumentRecovery.prepareTransaction(state), clearOAuthDocumentRecovery: () => oauthDocumentRecovery.clear(), armOAuthRedirectUnloadBypass: () => armOAuthRedirectUnloadBypass(), }); diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts index 7186f6c1..ab0654af 100644 --- a/tests/unit/app.test.ts +++ b/tests/unit/app.test.ts @@ -841,10 +841,19 @@ describe('createApp basics', () => { expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).toBe(checkpoint); expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY)).not.toBeNull(); - // Saving/cleaning the newer edit makes publication safe on the next - // automatic retry; the retained callback recovery can now take over. + // Saving/cleaning does not prove the session is unchanged. Keep the newer + // clean work and require explicit confirmation before recovery publishes. app.activeTab().dirtySql = false; await app.navigateSqlRoute({ surface: 'workspace', workspaceKey: 'recovery' }, 'replace'); + expect(app.activeTab()).toMatchObject({ + name: 'Untitled', sqlDraft: 'SELECT newer in-memory edit', dirtySql: false, + }); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).toBe(checkpoint); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY)).not.toBeNull(); + const restore = [...document.querySelectorAll('.share-toast button')] + .find((button) => button.textContent === 'Restore drafts'); + expect(restore).toBeDefined(); + restore!.click(); expect(app.activeTab()).toMatchObject({ name: 'Recovered draft', sqlDraft: 'SELECT recovered', dirtySql: true, }); diff --git a/tests/unit/connection-session.test.ts b/tests/unit/connection-session.test.ts index dbc8d8c9..f52656e6 100644 --- a/tests/unit/connection-session.test.ts +++ b/tests/unit/connection-session.test.ts @@ -855,7 +855,9 @@ describe('beginOAuth', () => { const events: string[] = []; const storage = memStorage({ oauth_verifier: 'old-verifier', oauth_state: 'old-state', oauth_return_route: 'old-route', + oauth_idp: 'basicidp', oauth_origin: 'https://old-cluster.example', }); + const rollback = vi.fn(); const location = { origin: 'https://ch.example', pathname: '/sql', search: '', get href() { return 'https://ch.example/sql'; }, @@ -864,7 +866,7 @@ describe('beginOAuth', () => { const { session } = setup({ storage, location, - prepareOAuthRedirect: () => true, + prepareOAuthRedirect: () => ({ hasRecoverySnapshot: true, rollback }), armOAuthRedirectUnloadBypass: () => { events.push('arm'); return () => events.push('disarm'); @@ -877,6 +879,10 @@ describe('beginOAuth', () => { expect(storage.getItem('oauth_verifier')).toBe('old-verifier'); expect(storage.getItem('oauth_state')).toBe('old-state'); expect(storage.getItem('oauth_return_route')).toBe('old-route'); + expect(storage.getItem('oauth_idp')).toBe('basicidp'); + expect(storage.getItem('oauth_origin')).toBe('https://old-cluster.example'); + expect(session.idpId()).toBe('basicidp'); + expect(rollback).toHaveBeenCalledOnce(); }); it('replaces every OAuth attempt key on retry', async () => { diff --git a/tests/unit/main.test.ts b/tests/unit/main.test.ts index 44d5f928..1f487d87 100644 --- a/tests/unit/main.test.ts +++ b/tests/unit/main.test.ts @@ -229,6 +229,7 @@ describe('bootstrap', () => { version: OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION, oauthState: 'pending-state', validatedAt: Date.now(), + documentSessionFingerprint: 'pending-live-session', }), ); env.sessionStorage.setItem('oauth_shared', JSON.stringify({ @@ -460,7 +461,7 @@ describe('bootstrap', () => { it.each([ { kind: 'absent' } as const, { kind: 'invalid-cleared', reason: 'expired' } as const, - { kind: 'workspace-mismatch-cleared' } as const, + { kind: 'workspace-mismatch-retained' } as const, { kind: 'callback-mismatch' } as const, ])('falls back to the legacy shared seed when recovery is $kind', async (result) => { const restore = vi.fn(() => result); diff --git a/tests/unit/oauth-document-recovery-session.test.ts b/tests/unit/oauth-document-recovery-session.test.ts index 5f528700..50a50b99 100644 --- a/tests/unit/oauth-document-recovery-session.test.ts +++ b/tests/unit/oauth-document-recovery-session.test.ts @@ -12,6 +12,7 @@ import { } from '../../src/core/oauth-document-recovery.js'; import { createOAuthDocumentRecoverySession, + oauthDocumentSessionFingerprint, type OAuthDocumentRecoverySessionDeps, type OAuthDocumentRecoveryState, type OAuthDocumentRecoveryStorage, @@ -72,6 +73,7 @@ function saveMarker( store: OAuthDocumentRecoveryStorage, oauthState = 'state-1', validatedAt = NOW, + liveState = state(), ): void { store.setItem( OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY, @@ -79,6 +81,7 @@ function saveMarker( version: OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION, oauthState, validatedAt, + documentSessionFingerprint: oauthDocumentSessionFingerprint(liveState), }), ); } @@ -207,6 +210,25 @@ describe('OAuthDocumentRecoverySession.prepare', () => { expect(() => session.prepare('new')).toThrow('storage full'); expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).toBe(old); }); + + it('rolls checkpoint, marker, and in-memory authority back as one preparation transaction', () => { + const store = storage(); + saveSnapshot(store); + saveMarker(store); + const checkpoint = store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY); + const marker = store.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY); + const session = createOAuthDocumentRecoverySession(deps({ storage: store })); + + const prepared = session.prepareTransaction('new-state'); + expect(prepared.hasRecoverySnapshot).toBe(true); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).not.toBe(checkpoint); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY)).toBeNull(); + + prepared.rollback(); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).toBe(checkpoint); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY)).toBe(marker); + expect(session.retryPending(workspace())).toEqual({ kind: 'restored' }); + }); }); describe('OAuthDocumentRecoverySession.restore', () => { @@ -215,23 +237,26 @@ describe('OAuthDocumentRecoverySession.restore', () => { expect(session.restore('state-1', workspace())).toEqual({ kind: 'absent' }); const before = s.tabs.value; saveSnapshot(store); + saveMarker(store); + const marker = store.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY); expect(session.restore('old-state', workspace())).toEqual({ kind: 'callback-mismatch' }); expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).not.toBeNull(); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY)).toBe(marker); expect(s.tabs.value).toBe(before); }); - it('clears invalid or workspace-mismatched payloads without publishing any state', () => { + it('clears invalid payloads but retains workspace-mismatched recovery without publishing', () => { const s = state(); const initialTabs = s.tabs.value; const store = storage({ [OAUTH_DOCUMENT_RECOVERY_KEY]: '{bad' }); const session = createOAuthDocumentRecoverySession(deps({ state: s, storage: store })); expect(session.restore('state-1', null)).toEqual({ kind: 'invalid-cleared', reason: 'malformed' }); expect(s.tabs.value).toBe(initialTabs); saveSnapshot(store); - expect(session.restore('state-1', { ...workspace(), id: 'other' })).toEqual({ kind: 'workspace-mismatch-cleared' }); - expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).toBeNull(); + expect(session.restore('state-1', { ...workspace(), id: 'other' })).toEqual({ kind: 'workspace-mismatch-retained' }); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).not.toBeNull(); expect(s.tabs.value).toBe(initialTabs); saveSnapshot(store); - expect(session.restore('state-1', { ...workspace(), key: 'other' })).toEqual({ kind: 'workspace-mismatch-cleared' }); - expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).toBeNull(); + expect(session.restore('state-1', { ...workspace(), key: 'other' })).toEqual({ kind: 'workspace-mismatch-retained' }); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).not.toBeNull(); }); it('retains a valid checkpoint and callback authority while unavailable, then retries in-session', () => { @@ -249,6 +274,7 @@ describe('OAuthDocumentRecoverySession.restore', () => { version: OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION, oauthState: 'state-1', validatedAt: NOW, + documentSessionFingerprint: oauthDocumentSessionFingerprint(s), }); expect(s.tabs.value).toBe(before); @@ -422,6 +448,7 @@ describe('OAuthDocumentRecoverySession.restore', () => { version: OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION, oauthState: 'state-1', validatedAt: NOW - OAUTH_DOCUMENT_RECOVERY_TTL_MS - 1, + documentSessionFingerprint: oauthDocumentSessionFingerprint(state()), }), ]) { const store = storage({ @@ -450,7 +477,7 @@ describe('OAuthDocumentRecoverySession.restore', () => { expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).not.toBeNull(); }); - it('clears a state-mismatched marker only, and clears both records on proven workspace mismatch', () => { + it('clears a state-mismatched marker only, and retains both records on workspace mismatch', () => { const stateMismatch = storage(); saveSnapshot(stateMismatch); saveMarker(stateMismatch, 'other'); const staleMarker = createOAuthDocumentRecoverySession(deps({ storage: stateMismatch })); expect(staleMarker.retryPending(workspace())).toEqual({ kind: 'callback-mismatch' }); @@ -460,9 +487,10 @@ describe('OAuthDocumentRecoverySession.restore', () => { const workspaceMismatch = storage(); saveSnapshot(workspaceMismatch); saveMarker(workspaceMismatch); const wrongWorkspace = createOAuthDocumentRecoverySession(deps({ storage: workspaceMismatch })); expect(wrongWorkspace.retryPending({ ...workspace(), id: 'other' })) - .toEqual({ kind: 'workspace-mismatch-cleared' }); - expect(workspaceMismatch.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).toBeNull(); - expect(workspaceMismatch.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY)).toBeNull(); + .toEqual({ kind: 'workspace-mismatch-retained' }); + expect(workspaceMismatch.getItem(OAUTH_DOCUMENT_RECOVERY_KEY)).not.toBeNull(); + expect(workspaceMismatch.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY)).not.toBeNull(); + expect(wrongWorkspace.retryPending(workspace())).toEqual({ kind: 'restored' }); }); it('clears unsupported and expired payloads before any state publication', () => { @@ -577,9 +605,15 @@ describe('OAuthDocumentRecoverySession.restore', () => { live.dirtySql = false; live.dirtySpec = false; - expect(session.retryPending(workspace([saved('q1', 'SELECT latest')]))).toEqual({ kind: 'restored' }); - expect(s.tabs.value).not.toBe(beforeTabs); - expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY)).toBeNull(); + expect(session.retryPending(workspace([saved('q1', 'SELECT latest')]))).toEqual({ + kind: 'document-session-changed-retained', + }); + expect(s.tabs.value).toBe(beforeTabs); + expect(store.getItem(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY)).toBe(marker); + expect(session.retryPending( + workspace([saved('q1', 'SELECT latest')]), + { allowChangedDocumentSession: true }, + )).toEqual({ kind: 'restored' }); }); it('retains fresh callback authority when checkpoint storage access fails, then retries', () => { diff --git a/tests/unit/oauth-document-recovery.test.ts b/tests/unit/oauth-document-recovery.test.ts index 6a867db6..c04c2b0d 100644 --- a/tests/unit/oauth-document-recovery.test.ts +++ b/tests/unit/oauth-document-recovery.test.ts @@ -51,6 +51,7 @@ const validatedCallback = ( version: OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION, oauthState: 'oauth-state', validatedAt: createdAt, + documentSessionFingerprint: 'live-session-fingerprint', ...over, }); @@ -179,7 +180,7 @@ describe('OAuth document recovery validated-callback codec', () => { it('exports a separate stable key/version/TTL and round-trips only callback proof fields', () => { expect(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_KEY) .toBe('oauth_document_recovery_validated_callback'); - expect(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION).toBe(1); + expect(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_VERSION).toBe(2); expect(OAUTH_DOCUMENT_RECOVERY_VALIDATED_CALLBACK_TTL_MS) .toBeLessThanOrEqual(OAUTH_DOCUMENT_RECOVERY_TTL_MS); const input = { @@ -204,7 +205,7 @@ describe('OAuth document recovery validated-callback codec', () => { expect(decodeMarker(null)).toEqual({ kind: 'missing' }); expect(decodeMarker(undefined)).toEqual({ kind: 'missing' }); expect(decodeMarker('{bad')).toEqual({ kind: 'invalid', reason: 'malformed' }); - expect(decodeMarker({ ...validatedCallback(), version: 2 })) + expect(decodeMarker({ ...validatedCallback(), version: 99 })) .toEqual({ kind: 'invalid', reason: 'unsupported' }); for (const value of [ { oauthState: 'oauth-state', validatedAt: createdAt },