Problem
app.signOut documents and enforces an important ordering invariant: cancel or tear down all in-flight authenticated work before credentials are cleared and the login screen is rendered.
It currently performs:
workbench.destroy()graph.cancel()exportService.cancelExport()exportService.cancelExportScript()catalog.invalidate()closeDocPane(app)conn.signOut()renderLoginApp()
However, the involuntary authentication-loss path is wired as:
onAuthLost: (detail) => renderLoginApp(detail)
renderLoginApp only disposes the Dashboard surface and mounted shell, advances the surface generation, resets mainSurface, and renders login. It does not run the Workbench, graph, export, catalog, or documentation-pane teardown.
The existing comment explains that Dashboard teardown was deliberately moved into renderLoginApp so onAuthLost receives it as well, but the rest of the end-of-session teardown remains exclusive to app.signOut.
Impact
When one concurrent operation triggers authentication loss, other authenticated operations can remain alive after the login screen is shown.
Examples include:
- an active Workbench query continuing to stream or complete;
- a successfully completing Workbench request still recording result metadata, History, bound parameters, or a schema reload;
- graph or export work continuing against a dead session;
- catalog or documentation state surviving into a later sign-in, potentially against a different ClickHouse origin.
disposeShell() is not sufficient: other lifecycle paths call both disposeShell() and workbench.destroy(), and workbench.destroy() owns aborting the active request and issuing KILL QUERY.
This is not triggered by every 401/403. Once authConfirmed is true, those responses are deliberately treated as query-level errors. Relevant triggers include failed token refresh, missing-token preflight, or concurrent first-contact authentication failure.
Suggested fix
Factor the authenticated-session teardown into one idempotent helper and invoke it for both explicit sign-out and involuntary auth loss before rendering login.
For example:
function teardownAuthenticatedSession(): void {
workbench.destroy();
graph.cancel();
exportService.cancelExport();
exportService.cancelExportScript();
catalog.invalidate();
closeDocPane(app);
}Then ensure both app.signOut and the onAuthLost path call it before renderLoginApp.
The helper should be idempotent because several concurrent requests may report authentication loss.
Tests
- Start a deferred Workbench request, invoke
chCtx.onSignedOut(), and assert its signal is aborted and its query ID is killed.
- Resolve the deferred request afterward and assert that no History entry, successful-result columns, bound-parameter recording, or schema reload occurs.
- Assert that graph and both export modes are cancelled, catalog state is invalidated, and the documentation pane is closed.
- Preserve the existing contract that a 401/403 after
authConfirmed === true remains a query error rather than forcing logout.
Problem
app.signOutdocuments and enforces an important ordering invariant: cancel or tear down all in-flight authenticated work before credentials are cleared and the login screen is rendered.It currently performs:
workbench.destroy()graph.cancel()exportService.cancelExport()exportService.cancelExportScript()catalog.invalidate()closeDocPane(app)conn.signOut()renderLoginApp()However, the involuntary authentication-loss path is wired as:
renderLoginApponly disposes the Dashboard surface and mounted shell, advances the surface generation, resetsmainSurface, and renders login. It does not run the Workbench, graph, export, catalog, or documentation-pane teardown.The existing comment explains that Dashboard teardown was deliberately moved into
renderLoginAppsoonAuthLostreceives it as well, but the rest of the end-of-session teardown remains exclusive toapp.signOut.Impact
When one concurrent operation triggers authentication loss, other authenticated operations can remain alive after the login screen is shown.
Examples include:
disposeShell()is not sufficient: other lifecycle paths call bothdisposeShell()andworkbench.destroy(), andworkbench.destroy()owns aborting the active request and issuingKILL QUERY.This is not triggered by every 401/403. Once
authConfirmedis true, those responses are deliberately treated as query-level errors. Relevant triggers include failed token refresh, missing-token preflight, or concurrent first-contact authentication failure.Suggested fix
Factor the authenticated-session teardown into one idempotent helper and invoke it for both explicit sign-out and involuntary auth loss before rendering login.
For example:
Then ensure both
app.signOutand theonAuthLostpath call it beforerenderLoginApp.The helper should be idempotent because several concurrent requests may report authentication loss.
Tests
chCtx.onSignedOut(), and assert its signal is aborted and its query ID is killed.authConfirmed === trueremains a query error rather than forcing logout.