mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
feat(ai-sessions): sync artifacts across tabs so a mirrored plan has its document
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
da4102fe57
commit
3b9322d947
@@ -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<void> {
|
||||
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<PersistedArtifact | undefined> {
|
||||
// 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<void> {
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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> = {}): db.PersistedArtifact {
|
||||
return {
|
||||
id: 'a1',
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user