From 393eb7f51e1fb03c52b39caad35a5f1c64223c1c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 4 Oct 2026 16:29:14 -0700 Subject: [PATCH] fix(native-chat): early appends are read-modify-writes, and the load can't be skipped - Before the load lands, an append (text or images given back, a paste) reads the stored draft and writes it back with the append in one storage change, so a load that failed and was retried can never lose the saved draft. On landing, only appends that load could not have read (made after it began reading, or never written) are applied again, by in-run order rather than by comparing clocks. - The editor saves a value set from the store when this window has a change of its own behind it, so a mention accepted next to a skill chip keeps the chip; another window's draft or a late load still isn't saved back. - A refused draft is journaled again; the 256,000-character cap bounds it. - The draft load starts before the startup chain, and the first write starts it in a window whose startup never did; startup still waits for it before the session's tabs mount. --- src/renderer/src/App.tsx | 3 + .../app-shell/native-chat-draft-startup.ts | 12 +- .../app-shell/use-app-startup-hydration.ts | 4 +- .../native-chat/NativeChatPromptEditor.tsx | 13 ++- .../native-chat-composer-draft-indexeddb.ts | 19 +++- .../native-chat-composer-draft-load.ts | 30 +++-- .../native-chat-composer-draft-memory.ts | 16 ++- .../native-chat-composer-draft-persistence.ts | 60 +++++++--- .../native-chat-composer-draft-storage.ts | 22 +++- .../native-chat-composer-draft-store.test.ts | 104 +++++++++++++----- .../native-chat-composer-draft-store.ts | 28 ++++- ...ve-chat-prompt-editor-store-value.test.tsx | 99 +++++++++++------ 12 files changed, 308 insertions(+), 102 deletions(-) diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx index 903c8b81447..1c1f72e5db8 100644 --- a/src/renderer/src/App.tsx +++ b/src/renderer/src/App.tsx @@ -24,6 +24,7 @@ import { useAppChromeLayout } from './app-shell/use-app-chrome-layout' import { useAppSessionPersistence } from './app-shell/use-app-session-persistence' import { useAppShellServices } from './app-shell/use-app-shell-services' import { useAppStartupHydration } from './app-shell/use-app-startup-hydration' +import { startNativeChatDraftLoad } from './app-shell/native-chat-draft-startup' import { useDocumentAppearance } from './app-shell/use-document-appearance' import { useFloatingWorkspacePanel } from './app-shell/use-floating-workspace-panel' import { useGlobalKeybindings } from './app-shell/use-global-keybindings' @@ -43,6 +44,8 @@ function App(): React.JSX.Element { useAppShellServices({ floatingPanelVisible: floatingWorkspace.enabled && floatingWorkspace.open }) + // Why before the startup chain: its effect runs first, and no startup step can skip the load. + useEffect(startNativeChatDraftLoad, []) useAppStartupHydration(onboardingGate.applyStartupOnboardingState) useAppSessionPersistence() useRuntimeGraphSync() diff --git a/src/renderer/src/app-shell/native-chat-draft-startup.ts b/src/renderer/src/app-shell/native-chat-draft-startup.ts index 1bbdf6fd6f2..4d64bdd708f 100644 --- a/src/renderer/src/app-shell/native-chat-draft-startup.ts +++ b/src/renderer/src/app-shell/native-chat-draft-startup.ts @@ -1,6 +1,7 @@ import { useAppStore } from '../store' import { resolveNativeChatDraftOwner } from '../lib/native-chat-draft-owner' import { + hydrateNativeChatComposerDrafts, setNativeChatComposerDraftOwnerResolver, waitForNativeChatComposerDrafts } from '@/components/native-chat/native-chat-composer-draft-store' @@ -9,11 +10,16 @@ import { // fills in every draft not edited meanwhile when it lands. const DRAFT_LOAD_WAIT_MS = 1_500 -/** Startup waits for the drafts alongside the session read, so a composer shows its draft from - * its first frame. */ -export function loadNativeChatDraftsForStartup(): Promise { +/** Before any startup step, so a step that fails can't leave the drafts unloaded. */ +export function startNativeChatDraftLoad(): void { setNativeChatComposerDraftOwnerResolver((scopeKey) => resolveNativeChatDraftOwner(useAppStore.getState(), scopeKey) ) + void hydrateNativeChatComposerDrafts() +} + +/** Startup waits for the drafts alongside the session read, so a composer shows its draft from + * its first frame. */ +export function waitForNativeChatDraftsAtStartup(): Promise { return waitForNativeChatComposerDrafts(DRAFT_LOAD_WAIT_MS) } diff --git a/src/renderer/src/app-shell/use-app-startup-hydration.ts b/src/renderer/src/app-shell/use-app-startup-hydration.ts index feae3b5b08c..0c0ab0dd543 100644 --- a/src/renderer/src/app-shell/use-app-startup-hydration.ts +++ b/src/renderer/src/app-shell/use-app-startup-hydration.ts @@ -5,7 +5,7 @@ import { installCodexDetachedPaneRestartExecutor } from '@/components/terminal-p import { useAppStore } from '../store' import { reconcileHydratedWorkspaceTabModels } from './reconcile-hydrated-workspace-tab-models' import { useStartupActions } from './use-app-startup-actions' -import { loadNativeChatDraftsForStartup } from './native-chat-draft-startup' +import { waitForNativeChatDraftsAtStartup } from './native-chat-draft-startup' import { WORKTREE_REFRESH_CONCURRENCY } from '../store/slices/worktrees' import { sweepRestoredCodexPanesForStaleAccounts } from '../lib/codex-stale-pane-sweep' import { fetchWorkspaceSessionWithRuntimeHostOwners } from '../lib/workspace-session-host-hydration' @@ -182,7 +182,7 @@ export function useAppStartupHydration(onOnboardingLoaded: (state: OnboardingSta const [sessionOutcome, catalogOutcome] = await Promise.allSettled([ hydrationSessionChain, localCatalogChain, - timeRendererStartupStep('native-chat-drafts', loadNativeChatDraftsForStartup) + timeRendererStartupStep('native-chat-drafts', waitForNativeChatDraftsAtStartup) ]) if (sessionOutcome.status === 'rejected') { throw sessionOutcome.reason diff --git a/src/renderer/src/components/native-chat/NativeChatPromptEditor.tsx b/src/renderer/src/components/native-chat/NativeChatPromptEditor.tsx index 0d9342df2a7..66d5870e1e1 100644 --- a/src/renderer/src/components/native-chat/NativeChatPromptEditor.tsx +++ b/src/renderer/src/components/native-chat/NativeChatPromptEditor.tsx @@ -2,6 +2,7 @@ import { readNativeChatDraftDocument, writeNativeChatDraftDocument } from './native-chat-draft-cache' +import { hasUnsavedNativeChatComposerDraftChange } from './native-chat-composer-draft-store' import { closeHistory } from '@tiptap/pm/history' import { Slice } from '@tiptap/pm/model' import { @@ -104,9 +105,15 @@ export function NativeChatPromptEditor({ ) }, onTransaction: ({ editor: current, transaction }) => { - // Why: a value set from outside (the store's own draft, another window's, a late load) - // is already the store's; saving it back would make it a local change of this window. - if (scopeKey && transaction.docChanged && !transaction.getMeta('preventUpdate')) { + // Why: a value set from the store with no change of this window behind it (another + // window's draft, a late load) is already saved; saving it back would make it one. A + // value this window just set keeps its document, skill chips included. + if ( + scopeKey && + transaction.docChanged && + (!transaction.getMeta('preventUpdate') || + hasUnsavedNativeChatComposerDraftChange(scopeKey)) + ) { writeNativeChatDraftDocument( scopeKey, promptTextMap(current.state.doc).text, diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-indexeddb.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-indexeddb.ts index 9e9a09bd7b9..32864af94ce 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-indexeddb.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-indexeddb.ts @@ -92,6 +92,23 @@ export function createIndexedDbNativeChatComposerDraftStorage( for (const scopeKey of scopeKeys) { drafts.delete(scopeKey) } - }) + }), + update: async (scopeKey, apply) => { + const transaction = (await database()).transaction(DRAFTS, 'readwrite') + const done = committed(transaction) + const drafts = transaction.objectStore(DRAFTS) + const read = drafts.get(scopeKey) + // Why inside the read's callback: the write must land in the same transaction as the read. + read.onsuccess = () => { + const next = apply(read.result) + if (next) { + drafts.put(next, scopeKey) + } else { + drafts.delete(scopeKey) + } + transaction.commit?.() + } + await done + } } } diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-load.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-load.ts index f77d31a301c..a04d225dcb2 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-load.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-load.ts @@ -33,19 +33,25 @@ let hydration: Promise | null = null let failedLoads = 0 let retryTimer: ReturnType | null = null -function withAppends(scopeKey: string, loaded: StoredNativeChatComposerDraft): DraftLoadResult { - const pending = load.appendsBeforeLoad.get(scopeKey) - // Why the time check: a load that read after the append's own write already holds it. - if (!pending || loaded.savedAt >= pending.firstWrittenAt) { +/** The appends this load could not have read: made after it began reading, or never written. */ +function withAppends( + scopeKey: string, + loaded: StoredNativeChatComposerDraft, + readAtSequence: number +): DraftLoadResult { + const missing = (load.appendsBeforeLoad.get(scopeKey) ?? []).filter( + (entry) => !entry.committed || entry.sequence > readAtSequence + ) + if (missing.length === 0) { return { draft: loaded, changed: false } } - const merged = pending.appends.reduce((draft, append) => append(draft), loaded) + const merged = missing.reduce((draft, entry) => entry.append(draft), loaded) return { draft: { ...merged, savedAt: nextSavedAt() }, changed: true } } type DraftLoadResult = { draft: StoredNativeChatComposerDraft; changed: boolean } -function applyLoaded(loaded: ReadonlyMap): void { +function applyLoaded(loaded: ReadonlyMap, readAtSequence: number): void { const drafts = new Map() for (const [scopeKey, value] of loaded) { drafts.set(scopeKey, parseStoredNativeChatComposerDraft(value)) @@ -59,6 +65,12 @@ function applyLoaded(loaded: ReadonlyMap): void { drafts.set(scopeKey, change.draft) dirtyScopes.add(scopeKey) } + for (const [scopeKey, appends] of load.appendsBeforeLoad) { + // Appended with nothing saved before: an empty draft is what the load would have read. + if (!drafts.has(scopeKey) && appends.some((entry) => !entry.committed)) { + drafts.set(scopeKey, { text: '', images: [], savedAt: 0 }) + } + } for (const [scopeKey, stored] of drafts) { if (load.editedBeforeLoad.has(scopeKey)) { continue @@ -68,7 +80,7 @@ function applyLoaded(loaded: ReadonlyMap): void { dirtyScopes.add(scopeKey) continue } - const { draft, changed } = withAppends(scopeKey, stored) + const { draft, changed } = withAppends(scopeKey, stored, readAtSequence) records.set(scopeKey, draft) unverifiedScopes.add(scopeKey) if (changed) { @@ -109,7 +121,9 @@ export function hydrateNativeChatComposerDrafts(): Promise { hydration ??= (async () => { removeLegacyLocalStorageNativeChatComposerDrafts() installNativeChatComposerDraftBroadcast() - applyLoaded(await nativeChatComposerDraftStorage().loadAll()) + // Why read here: storage applies changes in order, so this load reads every append made so far. + const readAtSequence = load.appendSequence + applyLoaded(await nativeChatComposerDraftStorage().loadAll(), readAtSequence) })().catch(retryLater) return hydration } diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-memory.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-memory.ts index 97f03366772..96ba8bc26af 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-memory.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-memory.ts @@ -27,11 +27,21 @@ export const scopeListeners = new Map void>>() * append (text or images given back, a paste) is applied again on top of it. */ export type DraftAppend = (draft: StoredNativeChatComposerDraft) => StoredNativeChatComposerDraft +/** An append made before the load landed: written onto the stored draft in one change, and made + * again on the loaded draft when that load could not have read it. */ +export type AppendBeforeLoad = { + /** In-run order, compared with the order at which a load attempt read storage. */ + readonly sequence: number + readonly append: DraftAppend + committed: boolean +} + type LoadBookkeeping = { hydrated: boolean + /** Sequence of the last append made before the load landed. */ + appendSequence: number readonly editedBeforeLoad: Set - /** Per scope: the appends, and when the first of them was written. */ - readonly appendsBeforeLoad: Map + readonly appendsBeforeLoad: Map readonly deletionsBeforeLoad: (( scopeKey: string, draft: StoredNativeChatComposerDraft @@ -41,6 +51,7 @@ type LoadBookkeeping = { // The startup load's bookkeeping, emptied once it lands. export const load: LoadBookkeeping = { hydrated: false, + appendSequence: 0, editedBeforeLoad: new Set(), appendsBeforeLoad: new Map(), deletionsBeforeLoad: [] @@ -71,6 +82,7 @@ export function clearDraftMemoryForTests(): void { unverifiedScopes.clear() lastSavedAt = 0 load.hydrated = false + load.appendSequence = 0 load.editedBeforeLoad.clear() load.appendsBeforeLoad.clear() load.deletionsBeforeLoad.length = 0 diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-persistence.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-persistence.ts index 2aef8d06c24..06fc7c1eed8 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-persistence.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-persistence.ts @@ -13,6 +13,7 @@ import { refusedScopes, unconfirmed, unverifiedScopes, + type AppendBeforeLoad, type DraftAppend, type DraftRecord, type UnconfirmedDraftChange @@ -116,13 +117,10 @@ export function flushNativeChatComposerDrafts(): void { } /** Why: storage commits after this task, so what it has not confirmed when the window goes away - * is also written synchronously and replayed by the next load. A draft storage already refused - * is left out: it is shown as not saved and retried instead. */ + * is also written synchronously and replayed by the next load, a refused draft included. */ function journalUnconfirmed(): void { flushNativeChatComposerDrafts() - journalNativeChatComposerDraftChanges( - new Map([...unconfirmed].filter(([scopeKey]) => !refusedScopes.has(scopeKey))) - ) + journalNativeChatComposerDraftChanges(unconfirmed) } function journalWhenHidden(): void { @@ -146,6 +144,42 @@ function installFlushOnHide(): void { document.addEventListener('visibilitychange', journalWhenHidden) } +/** Before the load lands, memory may not hold the saved draft, so an append is written onto what + * storage holds, read and written as one change. The loaded draft gets it again on landing. */ +function appendBeforeLoad(scopeKey: string, append: DraftAppend): void { + load.appendSequence += 1 + const entry: AppendBeforeLoad = { sequence: load.appendSequence, append, committed: false } + load.appendsBeforeLoad.set(scopeKey, [...(load.appendsBeforeLoad.get(scopeKey) ?? []), entry]) + const owner = records.get(scopeKey)?.owner + const empty: StoredNativeChatComposerDraft = { text: '', images: [], savedAt: 0 } + const written = nativeChatComposerDraftStorage() + .update(scopeKey, (stored) => { + const base = parseStoredNativeChatComposerDraft(stored) ?? empty + return savedForm({ + ...append({ ...base, ...(base.owner || !owner ? {} : { owner }) }), + savedAt: nextSavedAt() + }) + }) + .then( + () => { + entry.committed = true + channel?.postMessage({ scopeKey }) + }, + (error: unknown) => { + if (!refusedScopes.has(scopeKey)) { + console.warn( + '[native-chat-drafts] a draft could not be saved; it is kept in memory', + error + ) + refusedScopes.add(scopeKey) + notifyScope(scopeKey) + } + } + ) + inFlight.add(written) + void written.finally(() => inFlight.delete(written)) +} + /** Marks a scope changed in memory: `deferred` coalesces typing into one write, `immediate` * writes now. Before the startup load lands, an edit wins over the loaded draft and an append * is applied again on top of it. */ @@ -154,19 +188,15 @@ export function persistNativeChatComposerDraft( persist: 'immediate' | 'deferred', append?: DraftAppend ): void { + installFlushOnHide() + if (!load.hydrated && append && !load.editedBeforeLoad.has(scopeKey)) { + appendBeforeLoad(scopeKey, append) + return + } dirtyScopes.add(scopeKey) if (!load.hydrated) { - if (append) { - const pending = load.appendsBeforeLoad.get(scopeKey) - load.appendsBeforeLoad.set(scopeKey, { - firstWrittenAt: pending?.firstWrittenAt ?? records.get(scopeKey)?.savedAt ?? 0, - appends: [...(pending?.appends ?? []), append] - }) - } else { - load.editedBeforeLoad.add(scopeKey) - } + load.editedBeforeLoad.add(scopeKey) } - installFlushOnHide() if (persist === 'immediate') { flushNativeChatComposerDrafts() return diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-storage.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-storage.ts index e90ca4641cb..83323315c83 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-storage.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-storage.ts @@ -40,6 +40,11 @@ export type NativeChatComposerDraftStorage = { read(scopeKey: string): Promise write(scopeKey: string, draft: StoredNativeChatComposerDraft): Promise remove(scopeKeys: readonly string[]): Promise + /** Reads the stored record and writes what `apply` makes of it, as one change. */ + update( + scopeKey: string, + apply: (stored: unknown) => StoredNativeChatComposerDraft | null + ): Promise } /** Drafts kept for this run only: the storage of an environment without IndexedDB (unit tests), @@ -66,6 +71,21 @@ export function createMemoryNativeChatComposerDraftStorage(): NativeChatComposer drafts.delete(scopeKey) } return Promise.resolve() + }, + update: ( + scopeKey: string, + apply: (stored: unknown) => StoredNativeChatComposerDraft | null + ) => { + if (storage.refuseWrites) { + return Promise.reject(new DOMException('refused', 'QuotaExceededError')) + } + const next = apply(drafts.get(scopeKey)) + if (next) { + drafts.set(scopeKey, next) + } else { + drafts.delete(scopeKey) + } + return Promise.resolve() } } return storage @@ -82,7 +102,7 @@ export function nativeChatComposerDraftStorage(): NativeChatComposerDraftStorage } export function setNativeChatComposerDraftStorageForTests( - next: NativeChatComposerDraftStorage + next: NativeChatComposerDraftStorage | null ): void { storage = next } diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.test.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.test.ts index cddedca13a3..5f07f1e9636 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.test.ts @@ -399,24 +399,74 @@ describe('native-chat composer draft store', () => { }) }) - it('does not add an early append twice when the load already read its write', async () => { - storage.drafts.set('agent-session:s1', { text: 'saved earlier', images: [], savedAt: 1 }) - let land: () => void = () => {} - // This load reads only when it lands, after the append's own write. - const late = { + it('keeps the saved draft when a load that failed once reads after text was given back', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + storage.drafts.set('agent-session:s1', { text: 'saved typing', images: [], savedAt: 1 }) + let failures = 1 + const flaky = { ...storage, loadAll: () => - new Promise>((resolve) => { - land = () => resolve(new Map(storage.drafts)) - }) + failures-- > 0 ? Promise.reject(new Error('backing store')) : storage.loadAll() } - const reloaded = await reload({ using: late, hydrate: false }) + const reloaded = await reload({ using: flaky, hydrate: false }) await reloaded.store.waitForNativeChatComposerDrafts(1) - reloaded.drafts.appendNativeChatDraftCache('agent-session:s1', 'returned by Stop') + reloaded.drafts.appendNativeChatDraftCache('agent-session:s1', 'given back') + await reloaded.store.nativeChatComposerDraftWritesSettled() + await new Promise((resolve) => setTimeout(resolve, 1_200)) + await reloaded.store.hydrateNativeChatComposerDrafts() + await reloaded.store.nativeChatComposerDraftWritesSettled() + + expect(reloaded.drafts.readNativeChatDraftCache('agent-session:s1')).toBe( + 'saved typing\n\ngiven back' + ) + expect(storedDraft('agent-session:s1')?.text).toBe('saved typing\n\ngiven back') + warn.mockRestore() + }) + + it('keeps an append made after a retried load began reading', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + let failures = 1 + let land: () => void = () => {} + const flaky = { + ...storage, + loadAll: () => { + if (failures-- > 0) { + return Promise.reject(new Error('backing store')) + } + const snapshot = new Map(storage.drafts) + return new Promise>((resolve) => { + land = () => resolve(snapshot) + }) + } + } + const reloaded = await reload({ using: flaky, hydrate: false }) + await reloaded.store.waitForNativeChatComposerDrafts(1) + reloaded.drafts.appendNativeChatDraftCache('agent-session:s1', 'first') + await reloaded.store.nativeChatComposerDraftWritesSettled() + await new Promise((resolve) => setTimeout(resolve, 1_200)) + reloaded.drafts.appendNativeChatDraftCache('agent-session:s1', 'second') + await reloaded.store.nativeChatComposerDraftWritesSettled() land() await reloaded.store.hydrateNativeChatComposerDrafts() - expect(reloaded.drafts.readNativeChatDraftCache('agent-session:s1')).toBe('returned by Stop') + await reloaded.store.nativeChatComposerDraftWritesSettled() + expect(reloaded.drafts.readNativeChatDraftCache('agent-session:s1')).toBe('first\n\nsecond') + expect(storedDraft('agent-session:s1')?.text).toBe('first\n\nsecond') + warn.mockRestore() + }) + + it('starts the load on its first write when startup never did', async () => { + storage.drafts.set('agent-session:other', { text: 'saved elsewhere', images: [], savedAt: 1 }) + const counted = { ...storage, loadAll: vi.fn(() => storage.loadAll()) } + const reloaded = await reload({ using: counted, hydrate: false }) + reloaded.drafts.writeNativeChatDraftCache('tab-1:pane', 'typed') + + expect(counted.loadAll).toHaveBeenCalledTimes(1) + await vi.waitFor(() => + expect(reloaded.drafts.readNativeChatDraftCache('agent-session:other')).toBe( + 'saved elsewhere' + ) + ) }) it('retries a failed load a few times, warns once, and then stops', async () => { @@ -494,31 +544,27 @@ describe('native-chat composer draft store', () => { expect(localStorage.getItem('orca:nativeChatComposerDraftJournal:v1')).toBeNull() }) - it('keeps the journal small: never a refused draft, and never past its cap', async () => { + it('keeps a refused draft in the journal for the next run, and the journal under its cap', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) storage.refuseWrites = true + modules.drafts.writeNativeChatDraftCache('agent-session:s1', 'typed while storage refuses') for (let index = 0; index < 10; index += 1) { - modules.drafts.appendNativeChatDraftCache( - `agent-session:refused-${index}`, - 'x'.repeat(200_000) - ) + modules.drafts.appendNativeChatDraftCache(`agent-session:big-${index}`, 'x'.repeat(200_000)) } + modules.store.flushNativeChatComposerDrafts() await modules.store.nativeChatComposerDraftWritesSettled() window.dispatchEvent(new Event('pagehide')) - expect(localStorage.getItem('orca:nativeChatComposerDraftJournal:v1')).toBeNull() + await modules.store.nativeChatComposerDraftWritesSettled() + const journal = localStorage.getItem('orca:nativeChatComposerDraftJournal:v1') ?? '' + expect(journal.length).toBeLessThanOrEqual(256_000) + modules.store.clearNativeChatComposerDraftsForTests() storage.refuseWrites = false - const hanging = { ...storage, write: () => new Promise(() => {}) } - const unconfirmedTab = await reload({ using: hanging }) - for (let index = 0; index < 10; index += 1) { - unconfirmedTab.drafts.appendNativeChatDraftCache( - `agent-session:big-${index}`, - 'y'.repeat(60_000) - ) - } - window.dispatchEvent(new Event('pagehide')) - const journal = localStorage.getItem('orca:nativeChatComposerDraftJournal:v1') ?? '' - expect(journal.length).toBeGreaterThan(0) - expect(journal.length).toBeLessThanOrEqual(256_000) + const next = await reload() + expect(next.drafts.readNativeChatDraftCache('agent-session:s1')).toBe( + 'typed while storage refuses' + ) + warn.mockRestore() }) it('keeps a restored image checked when another window saves the draft with the same image', async () => { diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts index f797d928c92..ac891bafa60 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts @@ -11,6 +11,8 @@ import { import { clearDraftMemoryForTests, dirtyScopes, + hasLocalChange, + load, nextSavedAt, notifyScope, records, @@ -25,12 +27,16 @@ import { persistNativeChatComposerDraft, resetNativeChatComposerDraftPersistenceForTests } from './native-chat-composer-draft-persistence' -import { resetNativeChatComposerDraftLoadForTests } from './native-chat-composer-draft-load' -import type { - NativeChatComposerDraft, - NativeChatComposerDraftImage, - NativeChatComposerDraftOwner, - StoredNativeChatComposerDraft +import { + hydrateNativeChatComposerDrafts, + resetNativeChatComposerDraftLoadForTests +} from './native-chat-composer-draft-load' +import { + setNativeChatComposerDraftStorageForTests, + type NativeChatComposerDraft, + type NativeChatComposerDraftImage, + type NativeChatComposerDraftOwner, + type StoredNativeChatComposerDraft } from './native-chat-composer-draft-storage' export { @@ -44,6 +50,11 @@ export { waitForNativeChatComposerDrafts } from './native-chat-composer-draft-load' +/** This window has changed the draft and storage has not confirmed it yet. */ +export function hasUnsavedNativeChatComposerDraftChange(scopeKey: string): boolean { + return hasLocalChange(scopeKey) +} + export type NativeChatComposerDraftChange = { text?: string /** Present, even as undefined, to replace the document. */ @@ -150,6 +161,10 @@ export function updateNativeChatComposerDraft( records.set(scopeKey, record) notifyScope(scopeKey) persistNativeChatComposerDraft(scopeKey, persist, append) + if (!load.hydrated) { + // Why: a window whose startup never started the load still gets its saved drafts. + void hydrateNativeChatComposerDrafts() + } } /** @@ -260,5 +275,6 @@ export function clearNativeChatComposerDraftsForTests(): void { clearDraftMemoryForTests() resetNativeChatComposerDraftPersistenceForTests() resetNativeChatComposerDraftLoadForTests() + setNativeChatComposerDraftStorageForTests(null) resolveOwner = null } diff --git a/src/renderer/src/components/native-chat/native-chat-prompt-editor-store-value.test.tsx b/src/renderer/src/components/native-chat/native-chat-prompt-editor-store-value.test.tsx index 3f118cc7e43..4aa15f40350 100644 --- a/src/renderer/src/components/native-chat/native-chat-prompt-editor-store-value.test.tsx +++ b/src/renderer/src/components/native-chat/native-chat-prompt-editor-store-value.test.tsx @@ -15,7 +15,7 @@ const SCOPE = 'agent-session:s1' let storage = createMemoryNativeChatComposerDraftStorage() const loaded: Renderer[] = [] -async function open(): Promise { +async function open(options: { hydrate?: boolean } = {}): Promise { vi.resetModules() const storageModule = await import('./native-chat-composer-draft-storage') storageModule.setNativeChatComposerDraftStorageForTests(storage) @@ -24,15 +24,19 @@ async function open(): Promise { store: await import('./native-chat-composer-draft-store') } loaded.push(renderer) - await renderer.store.hydrateNativeChatComposerDrafts() + if (options.hydrate !== false) { + await renderer.store.hydrateNativeChatComposerDrafts() + } return renderer } /** The editor, plus the field's effect that sets the store's draft on it. */ -async function mountEditor(renderer: Renderer): Promise<() => void> { +async function mountEditor( + renderer: Renderer +): Promise<{ stopSync: () => void; input: () => NativeChatComposerInput; unmount: () => void }> { const { NativeChatPromptEditor } = await import('./NativeChatPromptEditor') const inputRef = createRef() - render( + const view = render( createElement(NativeChatPromptEditor, { scopeKey: SCOPE, inputRef, @@ -44,12 +48,29 @@ async function mountEditor(renderer: Renderer): Promise<() => void> { }) ) await vi.waitFor(() => expect(inputRef.current).not.toBeNull()) - return renderer.store.subscribeToNativeChatComposerDraft(SCOPE, () => { - const draft = renderer.drafts.readNativeChatDraftCache(SCOPE) - if (inputRef.current && inputRef.current.value !== draft) { - inputRef.current.value = draft - } + const stopSync = renderer.store.subscribeToNativeChatComposerDraft(SCOPE, () => { + // Like the field's layout effect, after the store change returns. + queueMicrotask(() => { + const draft = renderer.drafts.readNativeChatDraftCache(SCOPE) + if (inputRef.current && inputRef.current.value !== draft) { + inputRef.current.value = draft + } + }) }) + return { stopSync, input: () => inputRef.current!, unmount: view.unmount } +} + +const SKILL_DOCUMENT = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { type: 'nativeChatSkill', attrs: { token: '$review' } }, + { type: 'text', text: ' this' } + ] + } + ] } const pause = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) @@ -67,24 +88,49 @@ afterEach(() => { }) describe('the editor and the draft store', () => { - it('saves nothing when the store’s draft is set on the editor', async () => { - const renderer = await open() - const stopSync = await mountEditor(renderer) - await act(async () => renderer.drafts.appendNativeChatDraftCache(SCOPE, 'hello')) - await renderer.store.nativeChatComposerDraftWritesSettled() - const savedAt = storage.drafts.get(SCOPE)?.savedAt + it('saves nothing when a draft that loads late is set on the editor', async () => { + storage.drafts.set(SCOPE, { text: 'saved before the restart', images: [], savedAt: 1 }) + const renderer = await open({ hydrate: false }) + const { stopSync, input } = await mountEditor(renderer) + const write = vi.spyOn(storage, 'write') + await act(async () => renderer.store.hydrateNativeChatComposerDrafts()) await act(async () => pause(400)) await renderer.store.nativeChatComposerDraftWritesSettled() stopSync() - expect(storage.drafts.get(SCOPE)?.savedAt).toBe(savedAt) - // An echo would have saved the editor's own document over the store's text-only draft. - expect(storage.drafts.get(SCOPE)?.document).toBeUndefined() + expect(input().value).toBe('saved before the restart') + expect(write).not.toHaveBeenCalled() + }) + + it('keeps the skill chip when the composer itself changes the text, across a remount', async () => { + const renderer = await open() + renderer.drafts.writeNativeChatDraftDocument(SCOPE, '$review this', SKILL_DOCUMENT) + renderer.store.flushNativeChatComposerDrafts() + const first = await mountEditor(renderer) + await vi.waitFor(() => + expect(window.document.querySelector('[data-native-chat-skill]')).not.toBeNull() + ) + // A mention accepted: the composer writes the text, then the field sets it on the editor. + const next = '$review this @src/a.ts ' + act(() => { + renderer.drafts.writeNativeChatDraftCache(SCOPE, next) + first.input().value = next + }) + await act(async () => pause(400)) + await renderer.store.nativeChatComposerDraftWritesSettled() + expect(JSON.stringify(storage.drafts.get(SCOPE)?.document ?? null)).toContain('nativeChatSkill') + + first.stopSync() + first.unmount() + const second = await mountEditor(renderer) + await pause(50) + second.stopSync() + expect(window.document.querySelector('[data-native-chat-skill]')).not.toBeNull() }) it('lets another window’s send stand, instead of echoing back the draft it showed', async () => { const receiving = await open() - const stopSync = await mountEditor(receiving) + const { stopSync } = await mountEditor(receiving) const sending = await open() sending.drafts.writeNativeChatDraftCache(SCOPE, 'hello') sending.store.flushNativeChatComposerDrafts() @@ -105,20 +151,9 @@ describe('the editor and the draft store', () => { }) it('shows a draft that arrives later with its skill chip, from its saved document', async () => { - const document = { - type: 'doc', - content: [ - { - type: 'paragraph', - content: [ - { type: 'nativeChatSkill', attrs: { token: '$review' } }, - { type: 'text', text: ' this' } - ] - } - ] - } + const document = SKILL_DOCUMENT const receiving = await open() - const stopSync = await mountEditor(receiving) + const { stopSync } = await mountEditor(receiving) const sending = await open() sending.drafts.writeNativeChatDraftDocument(SCOPE, '$review this', document) sending.store.flushNativeChatComposerDrafts()