From d63244fc94aeab180b3d7b36c26dc781d822bd46 Mon Sep 17 00:00:00 2001
From: Boris Tyshkevich <btyshkevich@altinity.com>
Date: Tue, 28 Jul 2026 11:32:28 +0000
Subject: [PATCH 1/3] fix(#502): run mandatory authenticated-session teardown
 on involuntary auth loss too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

app.signOut cancels every in-flight authenticated operation (Workbench
request + KILL QUERY, schema graph, both export modes, catalog) and
closes the doc pane before clearing credentials and rendering login —
but onAuthLost (expired/invalid token, failed refresh, or a concurrent
first-contact auth failure) only rendered login, skipping all of it.
Factor the teardown into one idempotent teardownAuthenticatedSession()
shared by both paths.

chCtx.onSignedOut now notifies onAuthLost before clearing credentials,
not after: the teardown's fire-and-forget KILL QUERY / export-cancel
requests need getToken() to still see a valid token when they reach it,
or they silently no-op instead of reaching the server.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017E2cHgYTiobxSJLnCxgCPj
---
 CHANGELOG.md                          | 16 ++++++++
 src/application/connection-session.ts | 12 +++++-
 src/ui/app.ts                         | 53 +++++++++++++++++----------
 tests/unit/app.test.ts                | 49 +++++++++++++++++++++++++
 4 files changed, 109 insertions(+), 21 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 74f5ff09..c2dd7b52 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,22 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
   flip above a row near the viewport bottom, and oversized menus scroll within
   the viewport instead of placing Add/Cancel off-screen.
 
+### Fixed
+- **An involuntary auth loss (expired/invalid token, failed refresh, or a
+  concurrent first-contact authentication failure) now runs the same
+  mandatory authenticated-session teardown as an explicit sign-out** (#502).
+  Previously only `app.signOut` cancelled the in-flight Workbench
+  request (and issued its server-side `KILL QUERY`), cancelled the schema
+  graph and both export modes, invalidated the catalog, and closed the
+  documentation pane before rendering login; the `onAuthLost` path skipped
+  all of it, so a mid-flight query/export/lineage stream — or a still-completing
+  request that would go on to record History, bound parameters, or a schema
+  reload — could survive past the login screen. The teardown is now a single
+  idempotent `teardownAuthenticatedSession()` helper shared by both paths, and
+  `chCtx.onSignedOut` now notifies it before clearing credentials (not after),
+  so the teardown's requests still carry valid credentials to the server
+  instead of silently no-op'ing on an already-cleared token.
+
 ### Removed
 - **The unused saved-query repair planner has been removed** (#429 phase 6 /
   #500). Direct, ownership-safe Panel and Dashboard trash actions are the
diff --git a/src/application/connection-session.ts b/src/application/connection-session.ts
index 185608fa..3f6a6beb 100644
--- a/src/application/connection-session.ts
+++ b/src/application/connection-session.ts
@@ -278,9 +278,19 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection
     authHeader,
     // detail is set when CH rejects a *valid* login (authorization denial); the
     // no-arg calls (no token / expired + refresh failed) fall back to expiry.
+    // #502: `deps.onAuthLost` runs BEFORE `clearTokens()`, not after — its
+    // shell-owned teardown (aborting the workbench/graph/export requests and
+    // KILLing their server-side queries) is fire-and-forget, so each call's
+    // `authedFetch` only reaches its first `await ctx.getToken()` in THIS
+    // synchronous tick; clearing first (the old order) makes `getToken()`'s
+    // synchronous null check see an already-cleared token and every one of
+    // those best-effort authed calls silently no-op before ever reaching the
+    // server. (A Basic-mode session's later `authHeader()` call still reads
+    // `authMode` post-reset either way — same latent gap `signOut()` already
+    // had — tracked separately, not introduced here.)
     onSignedOut: (detail?: string) => {
-      clearTokens();
       deps.onAuthLost(detail || 'Your session expired — please sign in again.');
+      clearTokens();
     },
   };
 
diff --git a/src/ui/app.ts b/src/ui/app.ts
index cc7fa66f..8084017a 100644
--- a/src/ui/app.ts
+++ b/src/ui/app.ts
@@ -455,6 +455,28 @@ export function createApp(env: CreateAppEnv = {}): App {
     disposeShell();
     renderLogin(app as App & { root: Element }, msg);
   };
+  // #502: cancel/tear down every in-flight authenticated operation BEFORE
+  // credentials are cleared and login renders — required for both an
+  // explicit sign-out AND an involuntary auth loss (expired/invalid token,
+  // failed refresh, or a first-contact auth failure via `onAuthLost` below),
+  // so a mid-flight query/export/lineage stream can never land (or repaint)
+  // after the login screen is showing; invalidating the catalog means a
+  // later sign-in (possibly a different server) never sees stale
+  // schema/reference caches. Idempotent: several concurrent requests may
+  // each report auth loss. The workbench session stays reusable after
+  // destroy() — the next renderApp re-attaches its shell effects.
+  const teardownAuthenticatedSession = (): void => {
+    workbench.destroy();
+    // Plain abort (no clearResult settle) — the login render replaces the
+    // whole DOM next, so settling the visible result would be a wasted paint.
+    graph.cancel();
+    exportService.cancelExport();
+    exportService.cancelExportScript();
+    catalog.invalidate();
+    // #313: pane content must never survive a connection change — closed
+    // alongside the catalog reset, before the login screen renders.
+    closeDocPane(app);
+  };
   // The auth + config + ClickHouse connection lifecycle (#276 Phase 2) — OAuth
   // PKCE login/refresh, Basic probing, and IdP config resolution live in
   // `application/connection-session.ts`,
@@ -464,7 +486,12 @@ export function createApp(env: CreateAppEnv = {}): App {
   const conn = createConnectionSession({
     fetch: fetchFn, storage: ss, location: loc, crypto: cryptoObj,
     queryJson: ch.queryJson,
-    onAuthLost: (detail) => renderLoginApp(detail),
+    // #502: onAuthLost is the involuntary counterpart of signOut() below —
+    // same mandatory teardown ordering, before login replaces the DOM.
+    onAuthLost: (detail) => {
+      teardownAuthenticatedSession();
+      renderLoginApp(detail);
+    },
   });
   app.conn = conn;
   // THE single live ClickHouse context — owned by the session, aliased locally
@@ -478,28 +505,14 @@ export function createApp(env: CreateAppEnv = {}): App {
   // comment) — no flat `App` delegates (#276 Phase 5 deleted them).
   // `showLogin`/`signOut` stay app.ts-owned: they compose rendering, not
   // pure forwards.
-  // Sign-out is the one real end-of-session event in a single-route tab —
-  // the first production wiring of the sessions' teardown surfaces (#276
-  // Phase 5). Order matters: cancel/tear down every in-flight operation
-  // BEFORE clearing credentials and rendering login, so a mid-flight
-  // query/export/lineage stream can never land (or repaint) after the login
-  // screen is showing; invalidate the catalog so a later sign-in (possibly a
-  // different server) never sees stale schema/reference caches. The
-  // workbench session stays reusable after destroy(): the next renderApp
-  // re-attaches its shell effects.
+  // Sign-out is the one real explicit end-of-session event in a
+  // single-route tab (#276 Phase 5); `onAuthLost` above is its involuntary
+  // counterpart, sharing the same `teardownAuthenticatedSession()` (#502) —
+  // both must tear down before clearing credentials and rendering login.
   app.signOut = () => {
     app.closeShortcutDialog();
     resetShortcutChord(app);
-    workbench.destroy();
-    // Plain abort (no clearResult settle) — the login render replaces the
-    // whole DOM next, so settling the visible result would be a wasted paint.
-    graph.cancel();
-    exportService.cancelExport();
-    exportService.cancelExportScript();
-    catalog.invalidate();
-    // #313: pane content must never survive a connection change — closed
-    // alongside the catalog reset, before the login screen renders.
-    closeDocPane(app);
+    teardownAuthenticatedSession();
     conn.signOut();
     // #425: the Dashboard teardown, the surface-generation bump and the
     // main-surface reset live in `renderLoginApp` — the one place that knows the
diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts
index 63eb28df..eb17463c 100644
--- a/tests/unit/app.test.ts
+++ b/tests/unit/app.test.ts
@@ -2142,6 +2142,55 @@ describe('query run', () => {
     expect(result(app.activeTab()).rows).toEqual([['ok']]);
     expect(app.state.running.value).toBe(false);
   });
+  // #502: `onAuthLost` (a 401/expired-token/failed-refresh — routed here via
+  // `chCtx.onSignedOut`) is the involuntary counterpart of `signOut()`
+  // above and must run the same mandatory teardown before login renders,
+  // not just the Dashboard-surface reset `renderLoginApp` already owns.
+  it('chCtx.onSignedOut() (involuntary auth loss) tears down the authenticated session exactly like signOut(), and the request that resolves afterward records nothing', async () => {
+    let resolveRunFetch!: (value: FakeResponse | Promise<FakeResponse>) => void;
+    const fetch = asFetch(vi.fn((_url: string, init?: { body?: string }) => {
+      const sql = (init && init.body) || '';
+      if (/CREATE TABLE t\b/.test(sql)) return new Promise<FakeResponse>((res) => { resolveRunFetch = res; });
+      return Promise.resolve(resp({ json: { data: [] } })); // version/schema/reference background loads
+    }));
+    const { app, e } = appForRun([], { fetch });
+    const invalidateSpy = vi.spyOn(app.catalog, 'invalidate');
+    const loadSchemaSpy = vi.spyOn(app.catalog, 'loadSchema');
+    const graphCancelSpy = vi.spyOn(app.graph, 'cancel');
+    const cancelExportSpy = vi.spyOn(app.exports, 'cancelExport');
+    const cancelExportScriptSpy = vi.spyOn(app.exports, 'cancelExportScript');
+    const recordBoundParamsSpy = vi.spyOn(app.params, 'recordBoundParams');
+    app.openDocEntry({ kind: 'function', name: 'sum' }); // pane opens (lookup resolves unavailable — irrelevant here)
+    expect(document.querySelector('[role="complementary"]')).not.toBeNull();
+    // A schema-mutating statement (#502's own example) so a stray schema
+    // reload after resolution would show up as a spy call.
+    app.activeTab().sqlDraft = 'CREATE TABLE t (x UInt8) ENGINE=Memory';
+    const pending = app.actions.run();
+    await new Promise((r) => setTimeout(r));
+    expect(app.state.running.value).toBe(true);
+    const runCall = asMock(fetch).mock.calls.find((c) => c[1] && /CREATE TABLE t\b/.test(c[1].body || ''))!;
+    app.conn.chCtx.onSignedOut(); // involuntary auth loss — NOT app.signOut()
+    // Teardown fired before login rendered, exactly as signOut()'s does.
+    expect((runCall[1].signal as AbortSignal).aborted).toBe(true);
+    expect(invalidateSpy).toHaveBeenCalled();
+    expect(graphCancelSpy).toHaveBeenCalled();
+    expect(cancelExportSpy).toHaveBeenCalled();
+    expect(cancelExportScriptSpy).toHaveBeenCalled();
+    expect(document.querySelector('[role="complementary"]')).toBeNull(); // doc pane closed
+    expect(qs(app.root!, '.login-card')).toBeTruthy();
+    await new Promise((r) => setTimeout(r));
+    const kill = asMock(e.fetch!).mock.calls.find((c) => /KILL QUERY/.test((c[1] && c[1].body) || ''));
+    expect(kill).toBeTruthy();
+    resolveRunFetch(Promise.reject(Object.assign(new Error('aborted'), { name: 'AbortError' })));
+    await pending;
+    // Resolving afterward must record nothing: no History entry, no bound
+    // params, no schema reload for the DDL that was in flight.
+    expect(app.state.history.length).toBe(0);
+    expect(recordBoundParamsSpy).not.toHaveBeenCalled();
+    expect(loadSchemaSpy).not.toHaveBeenCalled();
+    // Idempotent: a second concurrent report of auth loss must not throw.
+    expect(() => app.conn.chCtx.onSignedOut()).not.toThrow();
+  });
   it('surfaces a query error', async () => {
     const { app } = appForRun([
       [(u, sql) => /bad/.test(sql), resp({ ok: false, status: 500, text: '{"exception":"DB::Exception: nope"}' })],

From 7a05bb9c797c834c7ee1fec5cbe5ac18e05debf8 Mon Sep 17 00:00:00 2001
From: Boris Tyshkevich <btyshkevich@altinity.com>
Date: Tue, 28 Jul 2026 12:00:34 +0000
Subject: [PATCH 2/3] fix(#502): fix the failed-refresh trigger and guard
 teardown re-entrancy

Independent review found the first pass didn't actually cover one of the
three triggers #502 lists: getToken() cleared credentials itself on a
failed proactive refresh, before its caller (which always routes a null
getToken() into onSignedOut/onAuthFailed) ever got a chance to run the
teardown with a still-usable token. Credential clearing is now solely
onSignedOut's job, run after the teardown.

That change makes the teardown's own fire-and-forget authenticated calls
able to rediscover the same dead token and report auth loss again, so
onSignedOut now latches once it starts handling a dead session (reset
only by a genuine fresh sign-in via setTokens/connectBasic) to stop that
recursing. onSignedOut also now clears credentials in a finally, so a
throwing onAuthLost callback can't skip cleanup.

Filed #522 (inbox) for a third, separately-shaped instance of the same
family of bug the review surfaced: results.ts's detached-view refresh
never routes its own auth failure into onSignedOut at all.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017E2cHgYTiobxSJLnCxgCPj
---
 CHANGELOG.md                          | 16 ++++--
 src/application/connection-session.ts | 39 +++++++++++++--
 tests/unit/app.test.ts                | 19 +++++--
 tests/unit/connection-session.test.ts | 72 +++++++++++++++++++++++----
 4 files changed, 125 insertions(+), 21 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c2dd7b52..59658a88 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -30,10 +30,18 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
   all of it, so a mid-flight query/export/lineage stream — or a still-completing
   request that would go on to record History, bound parameters, or a schema
   reload — could survive past the login screen. The teardown is now a single
-  idempotent `teardownAuthenticatedSession()` helper shared by both paths, and
-  `chCtx.onSignedOut` now notifies it before clearing credentials (not after),
-  so the teardown's requests still carry valid credentials to the server
-  instead of silently no-op'ing on an already-cleared token.
+  idempotent `teardownAuthenticatedSession()` helper shared by both paths.
+  Making this actually work required two deeper changes to
+  `connection-session.ts`: `getToken()` no longer clears credentials itself
+  on a failed proactive refresh (it did, previously — before its caller ever
+  got a chance to run the teardown with a still-usable token), and
+  `chCtx.onSignedOut` now (a) notifies the teardown *before* clearing
+  credentials, not after, so its requests still carry valid credentials
+  instead of silently no-op'ing on an already-cleared token, (b) latches
+  once it starts handling a dead session so the teardown's own concurrent
+  authenticated calls rediscovering the same dead token can't recursively
+  re-run it, resetting only on a genuine fresh sign-in, and (c) still clears
+  credentials even if the injected `onAuthLost` callback throws.
 
 ### Removed
 - **The unused saved-query repair planner has been removed** (#429 phase 6 /
diff --git a/src/application/connection-session.ts b/src/application/connection-session.ts
index 3f6a6beb..317838a7 100644
--- a/src/application/connection-session.ts
+++ b/src/application/connection-session.ts
@@ -124,6 +124,14 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection
   let token: string | null = ss.getItem('oauth_id_token');
   let refreshTok: string | null = ss.getItem('oauth_refresh_token');
   let authMode: 'oauth' | 'basic' = ss.getItem('ch_basic_auth') ? 'basic' : 'oauth';
+  // #502: latches once `chCtx.onSignedOut` starts handling a dead session, so
+  // concurrent/nested reports of the SAME auth loss (e.g. the teardown's own
+  // fire-and-forget KILL QUERY/export-cancel calls independently discovering
+  // the same expired-and-unrefreshable token) can't re-run the teardown,
+  // re-render login, or re-attempt `refresh()` against the token endpoint.
+  // Reset only by a genuine fresh sign-in (`setTokens`/`connectBasic`), so a
+  // LATER, distinct auth-loss event in the new session is handled normally.
+  let signedOutHandled = false;
   const basicCreds = (): string | null => ss.getItem('ch_basic_auth');
   const basicUser = (): string => ss.getItem('ch_basic_user') || '';
   const originHost = (o: string): string => { try { return new URL(o).host; } catch { return ''; } };
@@ -182,6 +190,9 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection
     // tokens. (The refresh path also lands here; they're already gone → no-op.)
     ss.removeItem('oauth_verifier');
     ss.removeItem('oauth_state');
+    // #502: a fresh/refreshed token means this is a live session again — a
+    // LATER auth loss is a distinct event `onSignedOut` must handle again.
+    signedOutHandled = false;
   }
   function clearTokens(): void {
     token = null;
@@ -244,7 +255,14 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection
     if (!token) return null;
     if (!isTokenExpired(token)) return token;
     if (await refresh()) return token;
-    clearTokens();
+    // #502: deliberately NOT `clearTokens()` here. Every caller of `getToken`
+    // already does `if (!(await getToken())) { <its own onSignedOut/
+    // onAuthFailed call> }` — that path's authenticated teardown (KILL QUERY,
+    // export cancellation) needs the still-current `token`/`authMode` this
+    // closure holds to actually reach the server; clearing it here, before
+    // any of those callers ever get a chance to run, was making every one of
+    // them silently no-op on an already-missing token. `onSignedOut` clears
+    // once its teardown has run.
     return null;
   }
 
@@ -287,10 +305,21 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection
     // those best-effort authed calls silently no-op before ever reaching the
     // server. (A Basic-mode session's later `authHeader()` call still reads
     // `authMode` post-reset either way — same latent gap `signOut()` already
-    // had — tracked separately, not introduced here.)
+    // had — tracked separately, not introduced here.) `signedOutHandled`
+    // guards re-entrancy: the teardown's own fire-and-forget calls can each
+    // independently rediscover the same dead token and report auth loss
+    // again — only the first report actually runs the teardown/render/clear;
+    // later ones (this tick or a delayed microtask) are no-ops until a fresh
+    // sign-in resets the latch. The `finally` guarantees credentials are
+    // still cleared even if the injected `onAuthLost` throws.
     onSignedOut: (detail?: string) => {
-      deps.onAuthLost(detail || 'Your session expired — please sign in again.');
-      clearTokens();
+      if (signedOutHandled) return;
+      signedOutHandled = true;
+      try {
+        deps.onAuthLost(detail || 'Your session expired — please sign in again.');
+      } finally {
+        clearTokens();
+      }
     },
   };
 
@@ -336,6 +365,8 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection
     ss.setItem('ch_basic_user', user);
     ss.setItem('ch_basic_origin', target);
     chCtx.origin = target;
+    // #502: a fresh Basic session is live again — see `setTokens`'s own reset.
+    signedOutHandled = false;
   }
 
   // --- dashboard (#149 D1) -------------------------------------------------
diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts
index eb17463c..b19764b5 100644
--- a/tests/unit/app.test.ts
+++ b/tests/unit/app.test.ts
@@ -3400,16 +3400,29 @@ 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 () => {
+  // #502: `chCtx.onSignedOut` latches once it starts handling a dead
+  // session (concurrent/nested reports of the SAME auth loss must not
+  // re-render login with a second message) — a fresh `app` per case is the
+  // distinct-session boundary the latch actually gates on.
+  it('onSignedOut shows the given detail — an authorization denial', 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
+  });
+  it('onSignedOut falls back to a session-expired default with no detail', async () => {
+    const app = createApp(env());
+    app.renderApp();
     app.conn.chCtx.onSignedOut();
     expect(qs(app.root, '.login-error').textContent).toContain('session expired');
   });
+  it('a second onSignedOut report for the same dead session does not re-render login with a new message', async () => {
+    const app = createApp(env());
+    app.renderApp();
+    app.conn.chCtx.onSignedOut('ClickHouse denied your account (HTTP 403). Server: nope');
+    app.conn.chCtx.onSignedOut(); // e.g. a concurrent request's own report — no-op
+    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;
     const e = env({
diff --git a/tests/unit/connection-session.test.ts b/tests/unit/connection-session.test.ts
index 716ec854..311004f0 100644
--- a/tests/unit/connection-session.test.ts
+++ b/tests/unit/connection-session.test.ts
@@ -260,7 +260,15 @@ describe('refresh (via getToken)', () => {
     await expect(session.getToken()).resolves.toBe('YWJj');
   });
 
-  it('clears everything when the token endpoint rejects the refresh', async () => {
+  // #502: getToken() itself must NOT clear credentials on a failed refresh —
+  // its caller (every one of them already does
+  // `if (!(await getToken())) { <onSignedOut path> }`) needs the still-
+  // current token/authMode readable when it reports auth loss, so its own
+  // authenticated teardown (KILL QUERY, export cancellation) can actually
+  // reach the server instead of finding a token that's already gone.
+  // Clearing is now `chCtx.onSignedOut`'s job alone (see the 'chCtx.onSignedOut'
+  // describe block below).
+  it('leaves credentials in place when the token endpoint rejects the refresh — clearing is onSignedOut\'s job', async () => {
     const { session, storage } = setup({
       storage: memStorage({ oauth_id_token: expiredToken, oauth_refresh_token: 'r0', oauth_idp: 'g' }),
       routes: [(url) => (url.endsWith('/token') ? jsonResponse(401, {}) : null)],
@@ -268,15 +276,11 @@ describe('refresh (via getToken)', () => {
     session.chCtx.authConfirmed = true;
     const tok = await session.getToken();
     expect(tok).toBeNull();
-    expect(session.token()).toBeNull();
-    expect(session.refreshToken()).toBeNull();
-    expect(session.idpId()).toBeNull();
-    expect(session.authMode()).toBe('oauth');
-    expect(session.chCtx.authConfirmed).toBe(false);
-    for (const k of [
-      'oauth_id_token', 'oauth_refresh_token', 'oauth_verifier', 'oauth_state', 'oauth_idp', 'oauth_origin',
-      'ch_basic_auth', 'ch_basic_user', 'ch_basic_origin',
-    ]) expect(storage.getItem(k)).toBeNull();
+    expect(session.token()).toBe(expiredToken);
+    expect(session.refreshToken()).toBe('r0');
+    expect(session.idpId()).toBe('g');
+    expect(session.chCtx.authConfirmed).toBe(true);
+    expect(storage.getItem('oauth_id_token')).toBe(expiredToken);
   });
 
   it('returns false when the token endpoint yields no usable bearer', async () => {
@@ -487,6 +491,54 @@ describe('chCtx.onSignedOut', () => {
     session.chCtx.onSignedOut();
     expect(onAuthLost).toHaveBeenCalledWith('Your session expired — please sign in again.');
   });
+
+  // #502: onAuthLost must see the still-current credential — its own
+  // authenticated teardown needs it — and only THEN does the token clear.
+  it('notifies onAuthLost while the credential is still current, and only clears once it returns', async () => {
+    const storage = memStorage({ oauth_id_token: expiredToken, oauth_refresh_token: 'r0', oauth_idp: 'g' });
+    let idTokenDuringTeardown: string | null | undefined;
+    const { session } = setup({
+      storage,
+      routes: [(url) => (url.endsWith('/token') ? jsonResponse(401, {}) : null)],
+      onAuthLost: vi.fn(() => { idTokenDuringTeardown = storage.getItem('oauth_id_token'); }),
+    });
+    // Mirrors authedFetch's own `if (!(await getToken())) { ctx.onSignedOut(); }`.
+    if (!(await session.getToken())) session.chCtx.onSignedOut();
+    expect(idTokenDuringTeardown).toBe(expiredToken);
+    expect(session.token()).toBeNull();
+    expect(storage.getItem('oauth_id_token')).toBeNull();
+  });
+
+  // #502: idempotency — several concurrent requests can each independently
+  // report the same dead session (including the teardown's own fire-and-
+  // forget calls rediscovering the same expired-and-unrefreshable token).
+  it('a second report for the same dead session is a no-op', () => {
+    const { session, onAuthLost } = setup({ storage: memStorage({ oauth_id_token: validToken }) });
+    session.chCtx.onSignedOut('first report');
+    session.chCtx.onSignedOut('second report');
+    expect(onAuthLost).toHaveBeenCalledTimes(1);
+    expect(onAuthLost).toHaveBeenCalledWith('first report');
+  });
+
+  it('a fresh sign-in resets the latch, so a later distinct auth loss is handled again', () => {
+    const { session, onAuthLost } = setup({ storage: memStorage({ oauth_id_token: validToken }) });
+    session.chCtx.onSignedOut('first loss');
+    session.setTokens(validToken); // fresh sign-in
+    session.chCtx.onSignedOut('second loss');
+    expect(onAuthLost).toHaveBeenCalledTimes(2);
+    expect(onAuthLost).toHaveBeenNthCalledWith(2, 'second loss');
+  });
+
+  it('still clears every credential even when the injected onAuthLost throws', () => {
+    const { session, storage } = setup({
+      storage: memStorage({ oauth_id_token: validToken, oauth_refresh_token: 'r' }),
+      onAuthLost: () => { throw new Error('shell blew up'); },
+    });
+    expect(() => session.chCtx.onSignedOut()).toThrow('shell blew up');
+    expect(session.token()).toBeNull();
+    expect(storage.getItem('oauth_id_token')).toBeNull();
+    expect(storage.getItem('oauth_refresh_token')).toBeNull();
+  });
 });
 
 // ── ensureFreshToken ─────────────────────────────────────────────────────────

From d0ed98aab904a23430149b6378f8b813e924ae49 Mon Sep 17 00:00:00 2001
From: Boris Tyshkevich <btyshkevich@altinity.com>
Date: Tue, 28 Jul 2026 12:45:47 +0000
Subject: [PATCH 3/3] fix(#502): freeze getToken for the teardown, and report
 auth loss from results.ts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Round 2 stopped getToken() from clearing credentials on a failed proactive
refresh, on the theory that this would let the teardown's own authenticated
calls use the retained token. That didn't work: every one of those calls
still goes through the same getToken(), which discards a token whenever its
own refresh attempt fails, regardless of whether the token itself is still
usable — so a nested call during teardown still saw null and authedFetch
still bailed before reaching the server.

chCtx.onSignedOut now freezes chCtx.getToken to resolve the retained token
directly (no refresh, no expiry check) for its own synchronous duration,
restoring it afterward. This works specifically because authedFetch reads
and calls ctx.getToken synchronously, before its own first await — so a
nested fire-and-forget call captures the frozen value before onSignedOut's
finally ever runs.

Also considered, and deliberately NOT done: freezing ctx.authHeader/
ctx.refresh the same way. Both are read by authedFetch only in the
continuation after its own await (refresh, only after an actual 401
response) — well after onSignedOut has already restored/cleared everything
— so freezing them would be dead code with no real effect. That's why a
Basic-mode session's authHeader-scheme bug (#520) genuinely isn't fixed by
this change, despite an earlier belief that it would be.

Separately: src/ui/results.ts's detached/expanded-view refresh preflight
called ensureFreshToken() but never reported the failure to onSignedOut at
all — unlike every other call site in the app. Before round 2, getToken()'s
own clearTokens() call at least silently wiped storage on this path; after
round 2 removed that, credentials would linger uncleared indefinitely if
this was a user's only live activity. Fixed for real (not filed as inbox):
ResultsApp.conn now also carries chCtx.onSignedOut, and rerun() calls it on
failure.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017E2cHgYTiobxSJLnCxgCPj
---
 CHANGELOG.md                          | 26 ++++++++-----
 src/application/connection-session.ts | 56 ++++++++++++++++++++-------
 src/ui/results.ts                     |  7 +++-
 tests/unit/app.test.ts                | 35 ++++++++++++++++-
 tests/unit/connection-session.test.ts | 32 +++++++++++++++
 tests/unit/results.test.ts            |  7 +++-
 6 files changed, 136 insertions(+), 27 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 59658a88..d2e41c6f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,17 +31,25 @@ auto-generated per-PR notes; this file is the curated, human-readable history.
   request that would go on to record History, bound parameters, or a schema
   reload — could survive past the login screen. The teardown is now a single
   idempotent `teardownAuthenticatedSession()` helper shared by both paths.
-  Making this actually work required two deeper changes to
+  Making this actually work required deeper changes to
   `connection-session.ts`: `getToken()` no longer clears credentials itself
   on a failed proactive refresh (it did, previously — before its caller ever
-  got a chance to run the teardown with a still-usable token), and
-  `chCtx.onSignedOut` now (a) notifies the teardown *before* clearing
-  credentials, not after, so its requests still carry valid credentials
-  instead of silently no-op'ing on an already-cleared token, (b) latches
-  once it starts handling a dead session so the teardown's own concurrent
-  authenticated calls rediscovering the same dead token can't recursively
-  re-run it, resetting only on a genuine fresh sign-in, and (c) still clears
-  credentials even if the injected `onAuthLost` callback throws.
+  got a chance to run the teardown), and `chCtx.onSignedOut` now (a)
+  notifies the teardown *before* clearing credentials, not after, freezing
+  `getToken` for that window to resolve the still-retained credential (even
+  one within the proactive-refresh skew, or already past real expiry — a
+  best-effort `KILL QUERY` costs nothing extra either way) rather than
+  discarding it, so the teardown's `KILL QUERY`/export-cancellation requests
+  reach the server instead of silently no-op'ing on a token that looks gone;
+  (b) latches once it starts handling a dead session so the teardown's own
+  concurrent authenticated calls rediscovering the same dead token can't
+  recursively re-run it, resetting only on a genuine fresh sign-in; and (c)
+  still clears credentials even if the injected `onAuthLost` callback
+  throws. A detached/expanded Data Pane view's own refresh preflight
+  (`src/ui/results.ts`) also now reports auth loss the same way every other
+  caller does — previously it neither cleared nor reported it, so a dead
+  token could linger un-torn-down indefinitely if that was a user's only
+  live activity.
 
 ### Removed
 - **The unused saved-query repair planner has been removed** (#429 phase 6 /
diff --git a/src/application/connection-session.ts b/src/application/connection-session.ts
index 317838a7..a229f6f3 100644
--- a/src/application/connection-session.ts
+++ b/src/application/connection-session.ts
@@ -296,28 +296,54 @@ export function createConnectionSession(deps: ConnectionSessionDeps): Connection
     authHeader,
     // detail is set when CH rejects a *valid* login (authorization denial); the
     // no-arg calls (no token / expired + refresh failed) fall back to expiry.
-    // #502: `deps.onAuthLost` runs BEFORE `clearTokens()`, not after — its
-    // shell-owned teardown (aborting the workbench/graph/export requests and
-    // KILLing their server-side queries) is fire-and-forget, so each call's
-    // `authedFetch` only reaches its first `await ctx.getToken()` in THIS
-    // synchronous tick; clearing first (the old order) makes `getToken()`'s
-    // synchronous null check see an already-cleared token and every one of
-    // those best-effort authed calls silently no-op before ever reaching the
-    // server. (A Basic-mode session's later `authHeader()` call still reads
-    // `authMode` post-reset either way — same latent gap `signOut()` already
-    // had — tracked separately, not introduced here.) `signedOutHandled`
-    // guards re-entrancy: the teardown's own fire-and-forget calls can each
-    // independently rediscover the same dead token and report auth loss
-    // again — only the first report actually runs the teardown/render/clear;
-    // later ones (this tick or a delayed microtask) are no-ops until a fresh
-    // sign-in resets the latch. The `finally` guarantees credentials are
+    // #502: the shell-owned teardown `deps.onAuthLost` runs (aborting the
+    // workbench/graph/export requests and KILLing their server-side queries)
+    // must authenticate its own fire-and-forget calls with the credential
+    // this dying session held — even one `getToken()`'s 60s skew calls
+    // "expiring soon", or one already past real expiry: a best-effort KILL
+    // QUERY costs nothing extra either way, and it's likely still accepted.
+    // The plain `ctx.getToken()` can't be reused as-is for this: it treats a
+    // failed *proactive* refresh as terminal and returns null regardless of
+    // whether the retained token is still good, so every nested
+    // `authedFetch` inside the teardown would see `!token` and bail before
+    // ever reaching the server. `chCtx.getToken` is frozen to resolve this
+    // exact retained token — computed and captured BEFORE the teardown
+    // runs — for the synchronous duration of `deps.onAuthLost`, then
+    // restored. This works because `authedFetch` reads+calls `ctx.getToken`
+    // synchronously, before its own first `await` — every nested call
+    // captures the frozen value before this function's `finally` below ever
+    // runs (fire-and-forget calls only kick off their promises here; they
+    // don't settle until well after this function returns).
+    //
+    // `ctx.authHeader`/`ctx.refresh`, in contrast, are read by `authedFetch`
+    // only in the continuation AFTER its own `await ctx.getToken()` — i.e.
+    // in a later microtask (or later still, `ctx.refresh` only fires on an
+    // actual 401 response, after a real network round-trip) — by which
+    // point this function has already returned and restored/cleared
+    // everything. Freezing them here would be a no-op that only creates a
+    // false sense of protection, so this deliberately does NOT try: for a
+    // Basic-mode session specifically, this means a stray retry can still
+    // build the wrong Authorization scheme once `authMode` has been reset
+    // (tracked separately as `inbox` #520, not fixed here — a real fix needs
+    // the fire-and-forget calls to be awaited, or their credentials captured
+    // and threaded through explicitly, before this function ever restores or
+    // clears anything).
+    //
+    // `signedOutHandled` guards re-entrancy: the teardown's own frozen calls,
+    // if they still fail, report auth loss again but that's a no-op (no
+    // second render/teardown) until a fresh sign-in (`setTokens`/
+    // `connectBasic`) resets it. The `finally` guarantees credentials are
     // still cleared even if the injected `onAuthLost` throws.
     onSignedOut: (detail?: string) => {
       if (signedOutHandled) return;
       signedOutHandled = true;
+      const frozenToken = authMode === 'basic' ? basicCreds() : token;
+      const realGetToken = chCtx.getToken;
+      chCtx.getToken = async () => frozenToken;
       try {
         deps.onAuthLost(detail || 'Your session expired — please sign in again.');
       } finally {
+        chCtx.getToken = realGetToken;
         clearTokens();
       }
     },
diff --git a/src/ui/results.ts b/src/ui/results.ts
index 024f4950..489a6a2e 100644
--- a/src/ui/results.ts
+++ b/src/ui/results.ts
@@ -161,7 +161,7 @@ export interface ResultsApp {
   now(): number;
   elapsedMs(): number;
   wallNow(): number;
-  conn: Pick<ConnectionSession, 'ensureFreshToken'>;
+  conn: Pick<ConnectionSession, 'ensureFreshToken'> & { chCtx: Pick<ConnectionSession['chCtx'], 'onSignedOut'> };
   /** The shared request/stream/normalize service (#276 Phase 1) — this module
    *  only ever needs `executeRead` (the detached Data view's own re-run). */
   exec: Pick<QueryExecutionService, 'executeRead'>;
@@ -1008,6 +1008,11 @@ export function expandDataPane(app: ResultsApp, r: QueryResult): DetachedView {
         if (refreshBtn) refreshBtn.disabled = true;
         if (!(await app.conn.ensureFreshToken())) {
           if (myGen === gen && !closed) settle('Not signed in');
+          // #522: unlike a dashboard tile's own preflight (`onAuthFailed`),
+          // this path never reported the auth loss at all — dead credentials
+          // would otherwise linger un-torn-down until some other action
+          // happened to rediscover the same dead token.
+          app.conn.chCtx.onSignedOut();
           return;
         }
         const execution = panelExecution(savedPanel, mergedSourceSql(src, source.sql), {
diff --git a/tests/unit/app.test.ts b/tests/unit/app.test.ts
index b19764b5..2c342de0 100644
--- a/tests/unit/app.test.ts
+++ b/tests/unit/app.test.ts
@@ -2191,6 +2191,39 @@ describe('query run', () => {
     // Idempotent: a second concurrent report of auth loss must not throw.
     expect(() => app.conn.chCtx.onSignedOut()).not.toThrow();
   });
+  // #502: the "failed proactive refresh" trigger specifically — getToken()
+  // treats a token within its 60s skew as needing refresh, and (unlike a
+  // hard-expired token) discards it on a failed refresh attempt UNLESS the
+  // teardown uses the frozen snapshot connection-session.ts now captures.
+  // Without that, workbench.destroy()'s KILL QUERY would never reach the
+  // server at all for this trigger — it would see a null token and bail.
+  it('KILL QUERY still reaches the server when the retained token needed (and failed) a proactive refresh', async () => {
+    const expiringSoonToken = jwt({ email: 'soon@example.com', exp: Math.floor(Date.now() / 1000) + 30 });
+    let resolveRunFetch!: (value: FakeResponse | Promise<FakeResponse>) => void;
+    const fetch = makeFetch([
+      [(u) => /config\.json/.test(u), resp({ json: { issuer: 'https://accounts.google.com', client_id: 'cid' } })],
+      [(u) => /openid-configuration/.test(u), resp({ json: { authorization_endpoint: 'https://a', token_endpoint: 'https://t' } })],
+      [(u) => u === 'https://t', resp({ ok: false, status: 401, text: '{}' })], // refresh fails
+      [(u, sql) => /SELECT 1\b/.test(sql), () => new Promise<FakeResponse>((res) => { resolveRunFetch = res; })],
+    ]);
+    const { app } = appForRun([], { fetch }); // default sessionStorage: a valid (non-expiring) token — run()'s own preflight needs no refresh
+    app.activeTab().sqlDraft = 'SELECT 1';
+    const pending = app.actions.run();
+    await new Promise((r) => setTimeout(r));
+    expect(app.state.running.value).toBe(true);
+    const runCall = asMock(fetch).mock.calls.find((c) => c[1] && /SELECT 1\b/.test(c[1].body || ''))!;
+    // The live token is now this one (e.g. rotated by an earlier refresh
+    // elsewhere) — expiring soon, and its own refresh attempt fails.
+    app.conn.setTokens(expiringSoonToken, 'rt0');
+    expect(await app.conn.getToken()).toBeNull(); // mirrors any real caller's preflight
+    app.conn.chCtx.onSignedOut(); // mirrors what that caller does next
+    expect((runCall[1].signal as AbortSignal).aborted).toBe(true);
+    await new Promise((r) => setTimeout(r));
+    const kill = asMock(fetch).mock.calls.find((c) => /KILL QUERY/.test((c[1] && c[1].body) || ''));
+    expect(kill).toBeTruthy(); // reached the server using the retained, expiring-soon token
+    resolveRunFetch(Promise.reject(Object.assign(new Error('aborted'), { name: 'AbortError' })));
+    await pending;
+  });
   it('surfaces a query error', async () => {
     const { app } = appForRun([
       [(u, sql) => /bad/.test(sql), resp({ ok: false, status: 500, text: '{"exception":"DB::Exception: nope"}' })],
@@ -3389,7 +3422,7 @@ describe('auth flows', () => {
     expect(ok).toBe(true);
     expect(app.conn.token()).toBe(validToken);
   });
-  it('getToken returns null + clears when refresh fails', async () => {
+  it('getToken returns null when refresh fails', async () => {
     const e = env({
       sessionStorage: memSession({ oauth_id_token: jwt({ exp: 1 }) }),
       fetch: makeFetch([
diff --git a/tests/unit/connection-session.test.ts b/tests/unit/connection-session.test.ts
index 311004f0..4ee73361 100644
--- a/tests/unit/connection-session.test.ts
+++ b/tests/unit/connection-session.test.ts
@@ -509,6 +509,38 @@ describe('chCtx.onSignedOut', () => {
     expect(storage.getItem('oauth_id_token')).toBeNull();
   });
 
+  // #502: the plain `getToken()` can't be reused as-is for the teardown's own
+  // authenticated calls — it would treat an already-expired-or-expiring-soon
+  // token as needing another refresh and return null on failure, discarding
+  // a token that might still be accepted. `onSignedOut` freezes `getToken`
+  // to resolve the retained credential for its own synchronous duration —
+  // this works because `authedFetch` (ch-client.ts) reads+calls `ctx.getToken`
+  // synchronously, before its own first `await`, so a nested fire-and-forget
+  // call captures the frozen value before this function's `finally` restores
+  // it. (`ctx.authHeader`/`ctx.refresh` are read later, in `authedFetch`'s own
+  // async continuation — well after this function has already returned — so
+  // freezing THEM would be a no-op; not attempted here. See #520 for the
+  // narrower, separately-tracked Basic-mode consequence.)
+  it('freezes getToken for the teardown window: a nested call gets the still-current (even already-expired) credential', async () => {
+    let session!: ReturnType<typeof setup>['session'];
+    let getTokenResult: string | null | undefined;
+    const storage = memStorage({ oauth_id_token: expiredToken, oauth_refresh_token: 'r0' });
+    const setupResult = setup({
+      storage,
+      // Mirrors what the teardown's own fire-and-forget authedFetch calls
+      // do: call getToken() WHILE onSignedOut is still synchronously running.
+      onAuthLost: () => {
+        void session.chCtx.getToken().then((t) => { getTokenResult = t; });
+      },
+    });
+    session = setupResult.session;
+    session.chCtx.onSignedOut();
+    await Promise.resolve();
+    await Promise.resolve();
+    expect(getTokenResult).toBe(expiredToken); // the retained credential, not null
+    expect(setupResult.fetchMock.calls.some((u) => u.endsWith('/token'))).toBe(false); // no real refresh attempted
+  });
+
   // #502: idempotency — several concurrent requests can each independently
   // report the same dead session (including the teardown's own fire-and-
   // forget calls rediscovering the same expired-and-unrefreshable token).
diff --git a/tests/unit/results.test.ts b/tests/unit/results.test.ts
index ad4cb7b2..31b14e17 100644
--- a/tests/unit/results.test.ts
+++ b/tests/unit/results.test.ts
@@ -1039,7 +1039,8 @@ describe('expandDataPane', () => {
     // `exec.executeRead` fixture in this suite uses.
     const executeRead = vi.fn(async (result: QueryResult, _opts: ExecuteReadOpts = {} as ExecuteReadOpts) => result);
     const ensureFreshToken = vi.fn(async () => false);
-    const app = makeApp({ conn: { ensureFreshToken }, exec: { executeRead } });
+    const onSignedOut = vi.fn();
+    const app = makeApp({ conn: { ensureFreshToken }, chCtx: { onSignedOut }, exec: { executeRead } });
     app.state.varValues.level = 'X';
     expandDataPane(app, paramResult());
     const overlay = qs(document, '.graph-overlay');
@@ -1048,6 +1049,10 @@ describe('expandDataPane', () => {
     expect(app.exec.executeRead).not.toHaveBeenCalled();
     expect(qs(overlay, '.detached-status').textContent).toBe('Not signed in');
     expect(refreshBtn(overlay)!.disabled).toBe(false); // re-enabled after the blocked attempt
+    // #522: unlike before, this failure now reports auth loss too — dead
+    // credentials must not linger un-torn-down until some other action
+    // happens to rediscover them.
+    expect(onSignedOut).toHaveBeenCalled();
   });
 
   it('shows a status and keeps the previous result when the rerun errors', async () => {