fix(ai-sessions): guard runtime-init sends and keep unreadable stores from looking empty

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

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

@@ -612,9 +612,13 @@ 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'> {
+	async reloadChat(id: string): Promise<'loaded' | 'missing' | 'unavailable' | 'no-store'> {
 		const db = await this.dbh.whenReady()
-		if (!db) return 'unavailable'
+		// 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'
 		try {
 			const chat = await db.get('chats', id)
 			if (!chat) return 'missing'

frontend/src/lib/components/copilot/chat/artifacts/artifactsDB.ts

@@ -124,13 +124,27 @@ export async function putArtifact(artifact: PersistedArtifact): Promise<void> {
 }
 
 export async function getArtifact(id: string): Promise<PersistedArtifact | undefined> {
+	const read = await readArtifact(id)
+	return read.state === 'loaded' ? read.artifact : undefined
+}
+
+/** A read whose failure is distinguishable from an absence, for the one caller
+ *  that acts on "the row is gone". Collapsing the two into `undefined` — which
+ *  is all {@link getArtifact} can say — turns a moment of unreadable storage
+ *  into a deletion. */
+export async function readArtifact(
+	id: string
+): Promise<
+	{ state: 'loaded'; artifact: PersistedArtifact } | { state: 'missing' } | { state: 'unavailable' }
+> {
 	const db = await getDB()
-	if (!db) return undefined
+	if (!db) return { state: 'unavailable' }
 	try {
-		return await db.get('items', id)
+		const artifact = await db.get('items', id)
+		return artifact ? { state: 'loaded', artifact } : { state: 'missing' }
 	} catch (err) {
 		console.error('Could not read artifact', err)
-		return undefined
+		return { state: 'unavailable' }
 	}
 }
 

frontend/src/lib/components/copilot/chat/artifacts/artifactsState.svelte.ts

@@ -5,6 +5,7 @@ import {
 	deleteArtifact,
 	getArtifact,
 	getArtifactVersion,
+	readArtifact,
 	isPlanArtifact,
 	listArtifactVersions,
 	listArtifactsForSession,
@@ -136,20 +137,25 @@ export class SessionArtifactsStore {
 	 * record does and for the same reason: delivery order and IndexedDB commit order are
 	 * independent, so a shipped copy could be older than what the store already holds.
 	 *
-	 * A read that comes back empty is how a removal arrives. It can only drop an id this list
+	 * A row that is genuinely absent is how a removal arrives. It can only drop an id this list
 	 * already has, so an artifact held here after a failed persist — which is exactly what the
 	 * same-id resync guard in setSession protects — is never one of them: the other tab has
 	 * nothing to say about a row it has never seen.
+	 *
+	 * Storage that cannot be read says nothing about the row and is left alone. Treating it as
+	 * an absence would take a plan off screen mid-approval because a read blipped; the next
+	 * write to that artifact broadcasts again and converges.
 	 */
 	async applyRemoteArtifact(artifactId: string): Promise<void> {
 		const loaded = this.#sessionId
 		if (!loaded) return
-		const stored = await getArtifact(artifactId)
+		const read = await readArtifact(artifactId)
 		// Read once the await settles: a session switched underneath it must not have the
 		// previous one's row placed into its list, nor one of its own rows dropped.
 		if (this.#sessionId !== loaded) return
-		if (stored) {
-			this.#place(stored)
+		if (read.state === 'unavailable') return
+		if (read.state === 'loaded') {
+			this.#place(read.artifact)
 			return
 		}
 		const next = this.artifacts.filter((a) => a.id !== artifactId)

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

@@ -79,6 +79,7 @@ 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,6 +103,7 @@ export function noteCaughtUp(sessionId: string): void {
 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
@@ -169,6 +171,11 @@ export function setDriverProbe(fn: (sessionId: string) => void): void {
 
 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. */
@@ -181,16 +188,29 @@ export function noteDriverAnswered(sessionId: string): void {
  *  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. */
+ *  duplicate tool calls and lost transcript this whole module exists to stop.
+ *
+ *  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. */
 async function bestEffort<T>(sessionId: string, body: () => Promise<T>): Promise<T | 'busy'> {
-	if (isWatching(sessionId)) return 'busy'
-	if (sendDriverProbe) {
+	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 may also have arrived while the probe was outstanding.
-		if (isWatching(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,17 +1,29 @@
 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']
+const SESSIONS = ['session-throttled-driver', 'session-driver-gone', 'session-never-shared']
 
 // `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.
@@ -23,6 +35,7 @@ afterEach(() => SESSIONS.forEach(clearRunPosition))
 // and whichever save lands last discards the other's transcript.
 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))
 		const body = vi.fn(async () => 'ran')
 
@@ -33,6 +46,7 @@ describe('withSessionRunLock with no lock to take', () => {
 	})
 
 	it('takes the run when nothing answers', async () => {
+		driverWasHereThenWentQuiet('session-driver-gone')
 		setDriverProbe(() => {})
 		const body = vi.fn(async () => 'ran')
 
@@ -41,4 +55,19 @@ describe('withSessionRunLock with no lock to take', () => {
 		expect(outcome).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

@@ -924,13 +924,13 @@ function createRuntime(session: Session): SessionRuntime {
 
 async function initRuntime(runtime: SessionRuntime, session: Session) {
 	const { manager } = runtime
-	await manager.historyManager.init()
-	manager.historyManager.setSessionId(session.id)
-	// Restore linked files persisted for this session (live handles re-grant on send;
-	// snapshots restore directly). Non-transient sessions persist immediately.
-	await manager.attachedFiles.restore(session.id, !session.transient)
-	await ensureChatIdsSeeded(manager.historyManager)
 
+	// Wired before the first await, not after. `getOrCreateRuntime` does not wait
+	// for this function, so the manager is mounted and its composer live while the
+	// restores below are still in flight — and a send in that window would find no
+	// guard and start a turn without ever asking who owns the run. None of these
+	// depend on what the awaits produce.
+	//
 	// Keep the session record's chatId following the manager's active chat: a
 	// "/clear" rotation or a history switch would otherwise leave it pointing at
 	// the previous chat, and the compare-page handoff (`from_session`) would
@@ -981,6 +981,13 @@ async function initRuntime(runtime: SessionRuntime, session: Session) {
 		return true
 	}
 
+	await manager.historyManager.init()
+	manager.historyManager.setSessionId(session.id)
+	// Restore linked files persisted for this session (live handles re-grant on send;
+	// snapshots restore directly). Non-transient sessions persist immediately.
+	await manager.attachedFiles.restore(session.id, !session.transient)
+	await ensureChatIdsSeeded(manager.historyManager)
+
 	if (session.chatId) {
 		manager.historyManager.setCurrentChatId(session.chatId)
 		await manager.historyManager.tagChatWithSession(session.chatId, session.id)
@@ -1181,6 +1188,10 @@ 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