fix(#512): harden OAuth document recovery lifecycle

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

docs/ARCHITECTURE.md

@@ -113,27 +113,35 @@ 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
 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

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
 

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<void> {
     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<typeof attemptKeys[number], string | null>;
     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,24 +342,28 @@ 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
       // 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');
+      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,13 +421,19 @@ 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 {
             const value = priorAttemptValues[key];
             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.