fix(ai-sessions): drop the driver probe and state the lockless limit plainly

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

frontend/src/lib/components/copilot/chat/HistoryManager.svelte.ts

@@ -612,13 +612,9 @@ export default class HistoryManager {
 	 *  'unavailable' is a fact about this browser. Callers act on the first and
 	 *  must not act on the second — treating a closed database as an empty chat
 	 *  would throw away a transcript that is merely unreadable right now. */
-	async reloadChat(id: string): Promise<'loaded' | 'missing' | 'unavailable' | 'no-store'> {
+	async reloadChat(id: string): Promise<'loaded' | 'missing' | 'unavailable'> {
 		const db = await this.dbh.whenReady()
-		// No database at all (disabled, private mode, a failed open) rather than a
-		// read that happened to fail. Worth distinguishing: a caller waiting for a
-		// readable store can wait forever on this one, and there is nothing stored
-		// for it to be out of step with either.
-		if (!db) return 'no-store'
+		if (!db) return 'unavailable'
 		try {
 			const chat = await db.get('chats', id)
 			if (!chat) return 'missing'

frontend/src/lib/components/sessions/sessionRunOwner.svelte.ts

@@ -79,7 +79,6 @@ export function noteDriverAlive(sessionId: string, planMode: boolean): void {
 	// A tab mid-turn is the authority on its own session; a frame reaching it can
 	// only be an echo of the run it is itself driving.
 	if (isDriving(sessionId)) return
-	seenDriver.add(sessionId)
 	positions.set(sessionId, { state: 'watching', lastHeardAt: Date.now(), planMode })
 	ensureReaper()
 }
@@ -102,8 +101,6 @@ export function noteCaughtUp(sessionId: string): void {
  *  down tab can't leave a position behind for a session nothing is watching. */
 export function clearRunPosition(sessionId: string): void {
 	positions.delete(sessionId)
-	probeAnswers.delete(sessionId)
-	seenDriver.delete(sessionId)
 }
 
 /** True from the driver's turn-end until this tab has re-read what it left
@@ -157,61 +154,26 @@ export async function withSessionRunLock<T>(
 	}
 }
 
-/** How long a probe waits for the driver to answer before its silence counts.
- *  Long enough to survive a busy main thread, short enough that the pre-flight
- *  is not felt; only origins without Web Locks ever pay it. */
-const PROBE_GRACE_MS = 750
-
-let sendDriverProbe: ((sessionId: string) => void) | undefined
-
-/** Registered by sessionSync, which owns the channel this is asked over. */
-export function setDriverProbe(fn: (sessionId: string) => void): void {
-	sendDriverProbe = fn
-}
-
-const probeAnswers = new Set<string>()
-
-/** Sessions this tab has ever seen driven elsewhere. Only these are worth
- *  probing before taking a lockless run; it is never cleared, because "there was
- *  another tab once" stays the reason to ask. */
-const seenDriver = new Set<string>()
-
-/** A driver answered a probe. Recorded rather than folded into the position:
- *  the answer proves a run is alive, but carries none of the frame state a
- *  `watching` position is made of. */
-export function noteDriverAnswered(sessionId: string): void {
-	probeAnswers.add(sessionId)
-}
-
-/** Exclusion without a lock. Silence is not enough to conclude a driver is
- *  gone — a hidden tab's heartbeat is throttled to as little as once a minute
- *  while its turn runs on perfectly well, and the reaper will have retired it
- *  long before that. So ask, and treat only an unanswered probe as absence.
- *  Getting this wrong runs a second turn against the same chat id, which is the
- *  duplicate tool calls and lost transcript this whole module exists to stop.
+/** What exclusion amounts to with no lock to take: refuse while another tab's
+ *  run is visibly on screen, and otherwise go.
  *
- *  This narrows the window rather than closing it, and it cannot close it: a tab
- *  the browser has frozen runs no script at all, so it answers a probe exactly
- *  the way a closed one does and nothing over this channel can tell them apart.
- *  Mutual exclusion needs Web Locks; where there is none, the honest guarantee
- *  is best-effort — still strictly more than the nothing a session had before
- *  any of this, and the reason the lock is what secure origins rely on. */
+ *  This is deliberately not mutual exclusion, and cannot be made into it. A
+ *  driver whose timers have been throttled in a hidden tab stops sending frames
+ *  long before its turn ends, so it is reaped as dead and a send from here can
+ *  start a second turn against the same chat id. Nothing over the channel fixes
+ *  that: a probe distinguishes a throttled tab from a closed one, but not a
+ *  frozen tab from a closed one, and the browser freezes hidden tabs on much the
+ *  same schedule as it throttles them.
+ *
+ *  Accepted rather than solved, because the only origins that land here are
+ *  served over plain HTTP and are not localhost — a shape used for local testing
+ *  rather than for running Windmill. Every HTTPS deployment, and localhost, is a
+ *  secure context and takes the real lock above. `isMirroring` and not
+ *  `isWatching`: a tab that has moved on to `catchingUp` is still owed its
+ *  re-read, and driving from here would send the pre-run history that re-read
+ *  exists to replace. */
 async function bestEffort<T>(sessionId: string, body: () => Promise<T>): Promise<T | 'busy'> {
 	if (isMirroring(sessionId)) return 'busy'
-	// Only worth asking where a driver has actually been seen. A session this tab
-	// has had to itself has nobody to answer, and paying the grace on every send
-	// would tax the single-tab case — the common one — for a race it cannot be in.
-	if (sendDriverProbe && seenDriver.has(sessionId)) {
-		probeAnswers.delete(sessionId)
-		sendDriverProbe(sessionId)
-		await new Promise((resolve) => setTimeout(resolve, PROBE_GRACE_MS))
-		if (probeAnswers.delete(sessionId)) return 'busy'
-		// A frame or a turn-end may also have landed while the probe was out.
-		// `isMirroring`, not `isWatching`: a tab that has moved on to `catchingUp`
-		// is owed a re-read, and driving from here would drop it and send the
-		// pre-run history the re-read exists to replace.
-		if (isMirroring(sessionId)) return 'busy'
-	}
 	return await drive(sessionId, body)
 }
 

frontend/src/lib/components/sessions/sessionRunOwner.test.ts

@@ -1,73 +1,45 @@
 import { afterEach, describe, expect, it, vi } from 'vitest'
 import {
 	clearRunPosition,
-	noteCaughtUp,
 	noteDriverAlive,
-	noteDriverAnswered,
 	noteRemoteTurnEnded,
-	setDriverProbe,
 	withSessionRunLock
 } from './sessionRunOwner.svelte'
 
-const SESSIONS = ['session-throttled-driver', 'session-driver-gone', 'session-never-shared']
+const SESSIONS = ['session-watching', 'session-catching-up', 'session-idle']
 
 // `positions` is module state and a watching entry keeps the reaper interval
 // armed, so leave nothing behind for the next suite.
 afterEach(() => SESSIONS.forEach(clearRunPosition))
 
-/** The sequence a watcher goes through when its driver stops sending frames:
- *  seen driving, then reaped back to idle. The tab is now free to send, and
- *  whether it may is the question the probe answers. */
-function driverWasHereThenWentQuiet(sessionId: string) {
-	noteDriverAlive(sessionId, false)
-	noteRemoteTurnEnded(sessionId)
-	noteCaughtUp(sessionId)
-}
-
-// Under the test env there is no Web Locks API, which is the same footing a
-// self-hosted instance served over plain HTTP runs on — so these exercise the
-// path that has nothing but the channel to arbitrate with.
-//
-// A driving tab that is hidden has its timers throttled to as little as once a
-// minute, so its heartbeat stops long before its turn does and the reaper
-// retires it. Concluding from that silence that the run is over starts a second
-// turn against the same chat id: duplicate tool calls against the workspace,
-// and whichever save lands last discards the other's transcript.
+// The test env has no Web Locks API, which is the footing an origin served over
+// plain HTTP runs on, so these exercise the fallback rather than the lock.
 describe('withSessionRunLock with no lock to take', () => {
-	it('refuses the run when a driver answers the probe', async () => {
-		driverWasHereThenWentQuiet('session-throttled-driver')
-		setDriverProbe((sessionId) => noteDriverAnswered(sessionId))
+	it('refuses while another tab is driving', async () => {
+		noteDriverAlive('session-watching', false)
 		const body = vi.fn(async () => 'ran')
 
-		const outcome = await withSessionRunLock('session-throttled-driver', body)
-
-		expect(outcome).toBe('busy')
+		expect(await withSessionRunLock('session-watching', body)).toBe('busy')
 		expect(body).not.toHaveBeenCalled()
 	})
 
-	it('takes the run when nothing answers', async () => {
-		driverWasHereThenWentQuiet('session-driver-gone')
-		setDriverProbe(() => {})
+	// The window after the driver's turn ends and before this tab has re-read what
+	// it left behind. The run is over, so a check for "someone is driving" says
+	// go — but the transcript on screen is still paired with the history from
+	// before that turn, and sending would put that pair to the model.
+	it('refuses while still catching up on a finished turn', async () => {
+		noteDriverAlive('session-catching-up', false)
+		noteRemoteTurnEnded('session-catching-up')
 		const body = vi.fn(async () => 'ran')
 
-		const outcome = await withSessionRunLock('session-driver-gone', body)
+		expect(await withSessionRunLock('session-catching-up', body)).toBe('busy')
+		expect(body).not.toHaveBeenCalled()
+	})
 
-		expect(outcome).toBe('ran')
+	it('takes a session no other tab holds', async () => {
+		const body = vi.fn(async () => 'ran')
+
+		expect(await withSessionRunLock('session-idle', body)).toBe('ran')
 		expect(body).toHaveBeenCalledTimes(1)
 	})
-
-	// The single-tab case, which is the common one: nobody has ever driven this
-	// session from anywhere else, so there is no one to answer and waiting out
-	// the grace on every send would be a tax paid for a race that cannot happen.
-	it('does not wait on a probe for a session it has never shared', async () => {
-		const probe = vi.fn()
-		setDriverProbe(probe)
-		const started = Date.now()
-
-		const outcome = await withSessionRunLock('session-never-shared', async () => 'ran')
-
-		expect(outcome).toBe('ran')
-		expect(probe).not.toHaveBeenCalled()
-		expect(Date.now() - started).toBeLessThan(300)
-	})
 })

frontend/src/lib/components/sessions/sessionRuntime.svelte.ts

@@ -1188,10 +1188,6 @@ async function applyTurnEnd(sessionId: string, chatId: string, attempt = 0): Pro
 			scheduleCatchUpRetry(sessionId, chatId, attempt)
 			return
 		}
-		// There is no store to catch up from, and no retry that would change that.
-		// Nothing was saved for this tab to be out of step with either, so holding
-		// the composer shut would cost the session for no gain.
-		if (found === 'no-store') caughtUp = true
 		if (found === 'loaded') {
 			// `refresh`: the same conversation caught up from the store, so whatever
 			// this manager still holds for it — a queued message, the background-job

frontend/src/lib/components/sessions/sessionSync.svelte.ts

@@ -1,13 +1,7 @@
 import { BROWSER } from 'esm-env'
 import { onUserChange, scopedKey } from '$lib/userScopedStorage'
 import type { DisplayMessage } from '$lib/components/copilot/chat/shared'
-import {
-	isDriving,
-	noteDriverAlive,
-	noteDriverAnswered,
-	noteRemoteTurnEnded,
-	setDriverProbe
-} from './sessionRunOwner.svelte'
+import { noteDriverAlive, noteRemoteTurnEnded } from './sessionRunOwner.svelte'
 
 // The channel between tabs open on the same AI session. Everything a session is
 // made of — the record list, the chat transcript, the run itself — lives in the
@@ -87,13 +81,6 @@ type QuestionAnswerMsg = {
 	choices: string[]
 }
 
-/** "Is anyone still running this?", asked by a tab about to drive where no lock
- *  can answer for it. Delivery is a task rather than a timer, so a driver whose
- *  timers have been throttled to a crawl in a hidden tab still receives it and
- *  still answers — which is exactly the case silence alone gets wrong. */
-type DriverProbeMsg = { kind: 'driver-probe'; sessionId: string }
-type DriverAliveMsg = { kind: 'driver-alive'; sessionId: string }
-
 type SyncMsg =
 	| SessionPutMsg
 	| SessionDeleteMsg
@@ -104,8 +91,6 @@ type SyncMsg =
 	| CancelRequestMsg
 	| ToolConfirmationMsg
 	| QuestionAnswerMsg
-	| DriverProbeMsg
-	| DriverAliveMsg
 
 type Handlers = {
 	onSessionPut: (id: string) => void
@@ -203,15 +188,6 @@ function receive(msg: SyncMsg): void {
 		case 'question-answer':
 			emit('onQuestionAnswer', msg.sessionId, msg.toolId, msg.choices)
 			break
-		case 'driver-probe':
-			// Answered from inside the handler, never off a timer: a throttled tab
-			// still runs tasks, and routing the answer through a timeout would make
-			// it look dead for exactly the reason the probe exists to rule out.
-			if (isDriving(msg.sessionId)) post({ kind: 'driver-alive', sessionId: msg.sessionId })
-			break
-		case 'driver-alive':
-			noteDriverAnswered(msg.sessionId)
-			break
 	}
 }
 
@@ -245,10 +221,6 @@ export function broadcastSessionArtifact(sessionId: string, artifactId: string):
 // Live mirroring
 // ---------------------------------------------------------------------------
 
-// sessionRunOwner decides when a probe is worth sending; posting one is this
-// module's job, and registering it here is what keeps the two out of a cycle.
-setDriverProbe((sessionId) => post({ kind: 'driver-probe', sessionId }))
-
 export function broadcastTurnEnd(sessionId: string, chatId: string): void {
 	post({ kind: 'turn-end', sessionId, chatId })
 }