diff --git a/frontend/src/lib/components/copilot/chat/artifacts/artifactsDB.ts b/frontend/src/lib/components/copilot/chat/artifacts/artifactsDB.ts index e937b170bf..5462058017 100644 --- a/frontend/src/lib/components/copilot/chat/artifacts/artifactsDB.ts +++ b/frontend/src/lib/components/copilot/chat/artifacts/artifactsDB.ts @@ -1,6 +1,6 @@ // Scoped by sessionId (fixed for the session's life), not chatId: a session follows its // active chat's rotation, so chatId-keying would drop artifacts on each new conversation. -import { type DBSchema as IDBSchema, type IDBPObjectStore } from 'idb' +import { type DBSchema as IDBSchema, type IDBPObjectStore, type IDBPTransaction } from 'idb' import { userScopedDb } from '$lib/userScopedDb' export type ArtifactKind = 'md' | 'html' @@ -135,6 +135,11 @@ export async function listArtifactsForSession(sessionId: string): Promise { + await items.put(edit.artifact) + for (const entry of edit.snapshots) await versions.put(entry) + const newest = edit.snapshots.at(-1) + if (newest) await pruneVersionsIn(versions, edit.artifact.id, newest.content.length) +} + +/** + * Read an artifact and write it back in one transaction. `mutate` returns the edit to + * write, or undefined to leave the artifact alone and resolve to undefined. A store that + * fails is reported rather than thrown, so the edited row resolves either way — persisted + * where it could be, and usable for the session where it could not. + */ +export async function mutateArtifact( + id: string, + mutate: (existing: PersistedArtifact | undefined) => ArtifactEdit | undefined +): Promise { + const db = await getDB() + // `transaction()` throws on a connection closed since `getDB()` answered — another tab + // upgrading the schema, or a user switch releasing the handle. + let opened: IDBPTransaction | undefined + try { + opened = db?.transaction(['items', 'versions'], 'readwrite') + } catch (err) { + console.error('Could not open an artifact write transaction', err) + } + // Not `return undefined`: `create` can hand out an artifact the store never took, and it + // stays revisable only if the edit is computed anyway. + if (!opened) return mutate(undefined)?.artifact + const tx = opened + // Attached before the first await: idb builds `done` eagerly and rejects it on abort, so + // attaching later would leave an unhandled rejection. Cleared before each deliberate + // abort below, whose own site reports the failure when there was one. + let reportFailure = true + const settled = tx.done.catch((err) => { + if (reportFailure) console.error('Could not persist artifact', err) + }) + const abort = () => { + try { + tx.abort() + } catch {} + } + const items = tx.objectStore('items') + const versions = tx.objectStore('versions') + let existing: PersistedArtifact | undefined + try { + // Read outside this transaction, two tabs both see version N, both stamp N+1, and the + // later write silently replaces the earlier one — content and snapshot alike. + existing = await items.get(id) + } catch (err) { + console.error('Could not read the artifact being written', err) + reportFailure = false + abort() + await settled + return mutate(undefined)?.artifact + } + // Kept out of the store's own error handling: a mutator that fails is not the store + // failing, so its error is neither reported as one nor swallowed. + let edit: ArtifactEdit | undefined + try { + edit = mutate(existing) + } catch (err) { + reportFailure = false + abort() + await settled + throw err + } + if (!edit) { + reportFailure = false + abort() + await settled + return undefined + } + try { + await writeEdit(items, versions, edit) + } catch (err) { + // A request that errors aborts the transaction itself; one that throws before creating + // a request (DataCloneError) would otherwise commit the row without its snapshot. + // Logged here because `settled` sees only a cause-less AbortError. + console.error('Could not persist artifact', err) + reportFailure = false + abort() + } + await settled + return edit.artifact +} + async function pruneVersionsIn( store: VersionsStore, artifactId: string, @@ -242,6 +334,8 @@ export async function deleteArtifactsForSession(sessionId: string): Promise + type VersionsStore = IDBPObjectStore< ArtifactsSchema, ('items' | 'versions')[], 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 9a54fd8f83..855b657ed4 100644 --- a/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.svelte.ts +++ b/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.svelte.ts @@ -6,6 +6,7 @@ import { getArtifactVersion, listArtifactVersions, listArtifactsForSession, + mutateArtifact, putArtifactWithVersions, versionKey, type ArtifactKind, @@ -113,31 +114,38 @@ export class SessionArtifactsStore { input: UpdateArtifactInput, opts?: { sessionId?: string } ): Promise { - const existing = this.artifacts.find((a) => a.id === id) ?? (await getArtifact(id)) - if (!existing) return undefined - if (opts?.sessionId !== undefined && existing.sessionId !== opts.sessionId) return undefined - // Only a content change earns a version: a rename or an identical rewrite would - // otherwise fill the picker with entries the user cannot tell apart. - const contentChanged = input.content !== undefined && input.content !== existing.content - const version = currentVersion(existing) + (contentChanged ? 1 : 0) - const updated: PersistedArtifact = { - ...existing, - name: input.name ?? existing.name, - content: input.content ?? existing.content, - updatedAt: Date.now(), - version - } - const snapshots: ArtifactVersion[] = [] - // An artifact written before history existed has no snapshot of its current content, - // so capture one on *any* update, not just a content change: this write stamps - // `version`, and nothing afterwards would recognise it as pre-history. - if (existing.version === undefined) { - snapshots.push(snapshotOf(existing, currentVersion(existing))) - } - if (contentChanged) { - snapshots.push(snapshotOf(updated, version, input.note)) - } - await putArtifactWithVersions(updated, snapshots) + const updated = await mutateArtifact(id, (stored) => { + // Read inside the mutator: hoisted out, it would weigh a stale copy against a fresh one. + const existing = furtherAlong( + stored, + this.artifacts.find((a) => a.id === id) + ) + if (!existing) return undefined + if (opts?.sessionId !== undefined && existing.sessionId !== opts.sessionId) return undefined + // Only a content change earns a version: a rename or an identical rewrite would + // otherwise fill the picker with entries the user cannot tell apart. + const contentChanged = input.content !== undefined && input.content !== existing.content + const version = currentVersion(existing) + (contentChanged ? 1 : 0) + const artifact: PersistedArtifact = { + ...existing, + name: input.name ?? existing.name, + content: input.content ?? existing.content, + updatedAt: Date.now(), + version + } + const snapshots: ArtifactVersion[] = [] + // An artifact written before history existed has no snapshot of its current content, + // so capture one on *any* update, not just a content change: this write stamps + // `version`, and nothing afterwards would recognise it as pre-history. + if (existing.version === undefined) { + snapshots.push(snapshotOf(existing, currentVersion(existing))) + } + if (contentChanged) { + snapshots.push(snapshotOf(artifact, version, input.note)) + } + return { artifact, snapshots } + }) + if (!updated) return undefined if (updated.sessionId === this.#sessionId) { this.#applyWrite(sortByUpdatedDesc(this.artifacts.map((a) => (a.id === id ? updated : a)))) } @@ -187,6 +195,24 @@ export class SessionArtifactsStore { } } +/** + * Neither copy is authoritative: only the store carries an edit another tab made, and only + * memory carries one the store refused (quota) and `update` handed back unpersisted. + * Always preferring one side reverts the other's text on the next edit, so the later wins. + */ +function furtherAlong( + stored: PersistedArtifact | undefined, + held: PersistedArtifact | undefined +): PersistedArtifact | undefined { + if (!stored || !held) return stored ?? held + const heldVersion = currentVersion(held) + const storedVersion = currentVersion(stored) + // A rename earns no version, so at equal versions only the clock separates the two. + // Both are written by the same browser, which is what makes the stamps comparable. + if (heldVersion === storedVersion) return held.updatedAt > stored.updatedAt ? held : stored + return heldVersion > storedVersion ? held : stored +} + function snapshotOf(a: PersistedArtifact, version: number, note?: string): ArtifactVersion { return { key: versionKey(a.id, version), 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 7036c16f2f..aae69fdfff 100644 --- a/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.test.ts +++ b/frontend/src/lib/components/copilot/chat/artifacts/artifactsState.test.ts @@ -143,6 +143,55 @@ describe('SessionArtifactsStore', () => { expect(await store.update('nope', { content: 'x' })).toBeUndefined() }) + it('gives two tabs editing at once distinct versions, keeping both snapshots', async () => { + // Without the transactional read both tabs stamp the same next version, and one edit + // and its snapshot vanish under the other. + const other = new (await import('./artifactsState.svelte')).SessionArtifactsStore() + await store.setSession('s1') + const created = await store.create('s1', { name: 'Doc', content: 'v1' }) + await other.setSession('s1') + + await Promise.all([ + store.update(created.id, { content: 'from A', note: 'A' }), + other.update(created.id, { content: 'from B', note: 'B' }) + ]) + + expect((await dbMod.listArtifactVersions(created.id)).map((v) => v.version)).toEqual([3, 2, 1]) + expect((await dbMod.getArtifact(created.id))?.version).toBe(3) + }) + + it('keeps an edit the store refused when the next update lands', async () => { + await store.setSession('s1') + const created = await store.create('s1', { name: 'Doc', content: 'v1' }) + + // Refused, so v2 lives only in memory while the stored row stays behind at v1. + const quiet = vi.spyOn(console, 'error').mockImplementation(() => {}) + const put = vi.spyOn(IDBObjectStore.prototype, 'put').mockImplementationOnce(() => { + throw new DOMException('quota', 'QuotaExceededError') + }) + await store.update(created.id, { content: 'v2' }) + put.mockRestore() + quiet.mockRestore() + expect((await dbMod.getArtifact(created.id))?.content).toBe('v1') + + // Computed from the stored row, this rename would put v1's text back under the new name. + expect(await store.update(created.id, { name: 'Renamed' })).toMatchObject({ + name: 'Renamed', + content: 'v2' + }) + }) + + it('creates and then revises an artifact when there is no store to write to', async () => { + // No scoped user, so the database never opens. `create` hands the id out either way, so + // the document it named has to stay revisable rather than come back as an unknown one. + ;(await import('$lib/stores')).userStore.set(undefined as never) + await store.setSession('s1') + const created = await store.create('s1', { name: 'A', content: 'x' }) + + expect((await store.update(created.id, { content: 'y' }))?.content).toBe('y') + expect(store.artifacts.map((a) => a.content)).toEqual(['y']) + }) + it('get resolves from the in-memory list even when the DB lacks the record', async () => { await store.setSession('s1') const created = await store.create('s1', { name: 'A', content: 'x' }) @@ -213,7 +262,7 @@ describe('SessionArtifactsStore', () => { await store.setSession('s1') const created = await store.create('s1', { name: 'Doc', content: 'c1' }) - const spy = vi.spyOn(dbMod, 'putArtifactWithVersions') + const spy = vi.spyOn(dbMod, 'mutateArtifact') await store.update(created.id, { content: 'c2', note: 'second' }) // Split into two writes, a stamped version can outlive the snapshot that failed to @@ -221,8 +270,8 @@ describe('SessionArtifactsStore', () => { // synthesizes the current version from the row — right up until the next edit // overwrites the row, which is the only copy of that content left. expect(spy).toHaveBeenCalledTimes(1) - expect(spy.mock.calls[0][1].map((v) => v.version)).toEqual([2]) spy.mockRestore() + expect((await store.listVersions(created.id)).map((v) => v.version)).toEqual([2, 1]) const row = await dbMod.getArtifact(created.id) const stored = await dbMod.listArtifactVersions(created.id)