diff --git a/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.svelte.ts b/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.svelte.ts index b8e543a720..bce91c5adf 100644 --- a/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.svelte.ts @@ -1,4 +1,5 @@ import { randomUUID } from '$lib/utils/uuid' +import { broadcastSessionArtifact } from '$lib/components/sessions/sessionSync.svelte' import { currentVersion, deleteArtifact, @@ -115,12 +116,46 @@ export class SessionArtifactsStore { // Insert-or-replace: `update` resolves from the database too, so a write can be the first // this session hears of a plan another tab created. - #reflect(artifact: PersistedArtifact): void { + #place(artifact: PersistedArtifact): void { if (artifact.sessionId !== this.#sessionId) return const rest = this.artifacts.filter((a) => a.id !== artifact.id) this.#applyWrite(sortByUpdatedDesc([artifact, ...rest])) } + // Every local write funnels through here, so the announcement sits here rather than at + // each of them. Announced even when this store holds a different session: the tab that + // does hold it is the one that needs to hear. The remote path calls #place directly — + // re-announcing what another tab just told us would bounce between the two forever. + #reflect(artifact: PersistedArtifact): void { + this.#place(artifact) + broadcastSessionArtifact(artifact.sessionId, artifact.id) + } + + /** + * Catch up on an artifact another tab wrote or removed. Carries only an id, like a session + * 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 + * 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. + */ + async applyRemoteArtifact(artifactId: string): Promise { + const loaded = this.#sessionId + if (!loaded) return + const stored = await getArtifact(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) + return + } + const next = this.artifacts.filter((a) => a.id !== artifactId) + if (next.length !== this.artifacts.length) this.#applyWrite(next) + } + async get(id: string): Promise { // In-memory first: a write whose persist silently failed (quota) is still readable here. return this.artifacts.find((a) => a.id === id) ?? (await getArtifact(id)) @@ -321,10 +356,13 @@ export class SessionArtifactsStore { } async remove(id: string): Promise { + const held = this.artifacts.find((a) => a.id === id) await deleteArtifact(id) // Guard on presence: a no-op remove must not invalidate an in-flight load. const next = this.artifacts.filter((a) => a.id !== id) if (next.length !== this.artifacts.length) this.#applyWrite(next) + const sessionId = held?.sessionId ?? this.#sessionId + if (sessionId) broadcastSessionArtifact(sessionId, id) } } diff --git a/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.test.ts b/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.test.ts index 7efca711a1..29b6e0ef9e 100644 --- a/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.test.ts +++ b/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.test.ts @@ -17,6 +17,16 @@ vi.mock('$lib/stores', async () => { }) vi.mock('$lib/utils', () => ({ getLocalSetting: () => undefined, storeLocalSetting: () => {} })) +// Stands in for the cross-tab channel, so what a write announces is assertable and no real +// BroadcastChannel is opened per resetModules. +const { broadcasts } = vi.hoisted(() => ({ + broadcasts: [] as Array<{ sessionId: string; artifactId: string }> +})) +vi.mock('$lib/components/sessions/sessionSync.svelte', () => ({ + broadcastSessionArtifact: (sessionId: string, artifactId: string) => + void broadcasts.push({ sessionId, artifactId }) +})) + // The DB module memoises its handle at module scope. A fresh IDBFactory per test only // isolates data once the handle is reset, so reset modules and re-import both together. // The DB is namespaced by email, so seed a user. @@ -524,6 +534,43 @@ describe('SessionArtifactsStore', () => { }) }) +// Two stores on one database, standing in for two tabs on the same session. The database is +// already shared; what a tab misses is that anything changed in it. +describe('cross-tab artifact sync', () => { + let watcher: SessionArtifactsStore + beforeEach(async () => { + broadcasts.length = 0 + watcher = new stateMod.SessionArtifactsStore() + await watcher.setSession('s1') + await store.setSession('s1') + }) + + it('picks up a plan written in the other tab, and does not announce it back', async () => { + const plan = await store.savePlan('s1', { name: 'Add retries', content: '# Plan', note: 'n' }) + expect(broadcasts).toEqual([{ sessionId: 's1', artifactId: plan.id }]) + expect(watcher.artifacts).toEqual([]) + + broadcasts.length = 0 + await watcher.applyRemoteArtifact(plan.id) + + // The card offering the plan for approval resolves its document out of this list, so + // until it lands the watching tab is being asked to approve a plan it cannot open. + expect(watcher.artifacts.map((a) => a.id)).toEqual([plan.id]) + // Announcing here would bounce the write between the two tabs forever. + expect(broadcasts).toEqual([]) + }) + + it('drops an artifact the other tab removed', async () => { + const created = await store.create('s1', { name: 'Notes', content: 'x' }) + await watcher.applyRemoteArtifact(created.id) + expect(watcher.artifacts).toHaveLength(1) + + await store.remove(created.id) + await watcher.applyRemoteArtifact(created.id) + expect(watcher.artifacts).toEqual([]) + }) +}) + function mk(over: Partial = {}): db.PersistedArtifact { return { id: 'a1', diff --git a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts index 6a59643a2c..91c7d74cab 100644 --- a/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionRuntime.svelte.ts @@ -1220,7 +1220,12 @@ registerSyncHandlers({ onTurnEnd: (sessionId, chatId, committed) => void applyTurnEnd(sessionId, chatId, committed), // A session deleted in another tab takes its runtime with it, so an open // chat for it stops streaming and releases its editors. - onSessionDelete: (id) => disposeRuntime(id) + onSessionDelete: (id) => disposeRuntime(id), + // Artifacts persist to a store both tabs share, but each keeps its own + // reactive copy, so the tab that did not write one never hears of it. That is + // what leaves a mirrored plan awaiting approval with no document behind it. + onSessionArtifact: (sessionId, artifactId) => + void runtimes.get(sessionId)?.manager.artifacts.applyRemoteArtifact(artifactId) }) export function getOrCreateRuntime(session: Session): SessionRuntime { diff --git a/frontend/src/lib/components/sessions/sessionSync.svelte.ts b/frontend/src/lib/components/sessions/sessionSync.svelte.ts index af9e4a910e..91c723f5bb 100644 --- a/frontend/src/lib/components/sessions/sessionSync.svelte.ts +++ b/frontend/src/lib/components/sessions/sessionSync.svelte.ts @@ -39,6 +39,11 @@ const MIRROR_SILENCE_MS = 10_000 * store actually holds. */ type SessionPutMsg = { kind: 'session-put'; id: string } type SessionDeleteMsg = { kind: 'session-delete'; id: string } +/** An artifact another tab wrote or removed. Same id-only shape, and for the + * same reason: the receiver re-reads, and a read that finds nothing is how a + * removal arrives. `sessionId` is here so a receiver can route to the right + * store without a database round-trip for artifacts it does not hold. */ +type SessionArtifactMsg = { kind: 'session-artifact'; sessionId: string; artifactId: string } /** `committed` distinguishes a turn that landed from one that errored, was * rolled back, or belonged to a tab that vanished. Watchers auto-send what the * user queued only on the first, matching the rule a turn follows locally. */ @@ -86,6 +91,7 @@ type QuestionAnswerMsg = { type SyncMsg = | SessionPutMsg | SessionDeleteMsg + | SessionArtifactMsg | TurnEndMsg | MirrorMsg | ResyncRequestMsg @@ -96,6 +102,7 @@ type SyncMsg = type Handlers = { onSessionPut: (id: string) => void onSessionDelete: (id: string) => void + onSessionArtifact: (sessionId: string, artifactId: string) => void onTurnEnd: (sessionId: string, chatId: string, committed: boolean) => void onMirror: (msg: MirrorMsg) => void onResyncRequest: (sessionId: string) => void @@ -165,6 +172,9 @@ function receive(msg: SyncMsg): void { case 'session-delete': emit('onSessionDelete', msg.id) break + case 'session-artifact': + emit('onSessionArtifact', msg.sessionId, msg.artifactId) + break case 'turn-end': remoteDriven.delete(msg.sessionId) emit('onTurnEnd', msg.sessionId, msg.chatId, msg.committed) @@ -210,6 +220,10 @@ export function broadcastSessionDelete(id: string): void { post({ kind: 'session-delete', id }) } +export function broadcastSessionArtifact(sessionId: string, artifactId: string): void { + post({ kind: 'session-artifact', sessionId, artifactId }) +} + // --------------------------------------------------------------------------- // Ownership // ---------------------------------------------------------------------------