Workbench run re-entrancy guard races across auth awaits

[BorisTyshkevich]

Problem

run(), runVariableSql(), and runScript() guard re-entry by reading state.running.value, but they do not claim the execution slot until after awaiting authentication/configuration.

For example, run() currently follows this sequence:

if (state.running.value) return;
// prepare source
await deps.ensureConfig();
if (!(await deps.getToken())) { hooks.onAuthFailed(); return; }
// initialise shared bookkeeping
state.running.value = true;

runScript() and the dashboard-variable execution path use the same ordering.

If two invocations enter while ensureConfig() or getToken() is pending, both observe running === false and both proceed to execution after authentication completes.

This is realistic on the first run of a session, during a genuine token refresh, or through programmatic calls made before the first invocation reaches the running write.

Impact

The Workbench session stores active-run bookkeeping in four shared closure slots:

• runT0
• runQueryId
• runTick
• abortController

A second admitted invocation overwrites all four values. The two independent finally blocks then clear whichever operation currently owns the shared slots rather than the resources belonging to that invocation.

Concrete consequences include:

• the first interval handle can become unreachable and continue firing until page unload;
• the first completion can set state.running.value = false while the second request is still streaming;
• the Run button and keyboard entry point re-enable, permitting further overlapping executions;
• cancel() becomes a no-op after the premature running = false, or cancels only the most recently published controller/query ID;
• result and elapsed-time bookkeeping can be corrupted;
• two admitted runScript() calls can execute the same INSERT, DDL, or other side-effecting statements twice.

The existing guard test only covers a call made when running was already true. It does not defer authentication and launch competing calls.

Suggested fix

Claim the execution slot synchronously before the first await, and represent lifecycle state with a per-invocation object rather than shared scalar fields.

For example:

interface ActiveRun {
  controller: AbortController;
  queryId: string | null;
  tick: ReturnType<typeof setInterval> | null;
  startedAt: number | null;
  cancelled: boolean;
}

let activeRun: ActiveRun | null = null;

At each execution entry point:

if (activeRun) return;

const operation: ActiveRun = {
  controller: new AbortController(),
  queryId: null,
  tick: null,
  startedAt: null,
  cancelled: false,
};

activeRun = operation;
state.running.value = true;

All request arguments and cleanup should use the local operation. Global state should be released only when that operation still owns the claim:

finally {
  if (operation.tick) clearInterval(operation.tick);

  if (activeRun === operation) {
    activeRun = null;
    state.running.value = false;
  }
}

The same claim must cover run(), runVariableSql(), and runScript() so different execution modes cannot overlap. cancel() should consult activeRun, not the UI signal, and cancellation while authentication is pending should prevent the query from starting once authentication resolves.

Tests

• Defer ensureConfig(), invoke run() twice, resolve it, and assert only one execution starts.
• Repeat for runScript() and dashboard-variable Run.
• Verify the claim is released after authentication failure and thrown authentication errors.
• Cancel while authentication is pending and assert no request starts after the await resolves.
• Use fake timers and deferred executions to assert no elapsed-time interval leaks.
• Verify a stale finaliser cannot clear a newer operation's running state, controller, query ID, or timer.

[BorisTyshkevich]

Current status after #523/#524

This issue remains reproducible on current main.

The authenticated execution scope added by #524 fences work across authentication epochs, but it does not serialize workbench runs within one open epoch. The scope accepts multiple concurrent registrations, and run(), dashboard-variable Run, and runScript() still claim state.running only after ensureConfig() and getToken() complete.

Two calls admitted while either preflight await is pending therefore both remain current and both start execution when preflight resolves.

The cleanup behaviour has changed slightly since this issue was filed:

settleWave(controller) now prevents an old wave from clearing bookkeeping owned by a newer controller.
Each run finaliser still sets state.running.value = false before that ownership check.
The first completion can therefore re-enable Run and make cancel() a no-op while the second execution remains active.
The first interval can still leak because the shared runTick reference has already been replaced.
Shared controller/query-id ownership still means Cancel can target only the most recently published wave.
Duplicate script invocations can still execute side-effecting statements twice.

The fix should add a workbench-local active-wave claim before the first authentication await. Authenticated-scope registration and active-workbench-run ownership are separate concerns: the former fences credential epochs; the latter must enforce one workbench execution at a time.