From b93e56c3df0b880359c5da993aec10882f2ba9ef Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 18 Jun 2026 17:07:32 +1000 Subject: [PATCH 01/15] feat: cache unsent session drafts Signed-off-by: Matt Toohey --- .../lib/features/branches/BranchCard.svelte | 10 +++ .../BranchCardSessionManager.svelte.ts | 52 ++++++++++-- .../features/projects/ProjectSection.svelte | 53 ++++++++++-- .../features/sessions/NewSessionModal.svelte | 72 ++++++++++++++-- .../features/sessions/SessionChatPane.svelte | 37 ++++++++ apps/staged/src/lib/shared/draftCache.test.ts | 54 ++++++++++++ apps/staged/src/lib/shared/draftCache.ts | 85 +++++++++++++++++++ 7 files changed, 344 insertions(+), 19 deletions(-) create mode 100644 apps/staged/src/lib/shared/draftCache.test.ts create mode 100644 apps/staged/src/lib/shared/draftCache.ts diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index af5f47051..259ecf596 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -2359,6 +2359,16 @@ remote={isRemote} willQueue={sessionMgr.willQueue} willQueueForMode={(mode) => sessionMgr.willQueueForMode(mode)} + loadDraftForMode={(mode) => sessionMgr.loadNewSessionDraftForMode(mode)} + onDraftChange={(draft, opts) => { + // Don't persist prefilled text as a draft — it should be re-evaluated + // each time the dialog opens based on the current timeline state. + const prompt = + draft.prompt.trim() === commitPrefill.trim() || draft.prompt.trim() === notePrefill.trim() + ? '' + : draft.prompt; + sessionMgr.handleNewSessionDraftChange({ ...draft, prompt }, opts); + }} onClose={(draft) => { // Don't persist prefilled text as a draft — it should be re-evaluated // each time the dialog opens based on the current timeline state. diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index 9a16c5b1e..37bff2821 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -16,6 +16,7 @@ import type { BranchSessionType, } from '../../types'; import { buildReferringPrompt } from '../../shared/buildReferringPrompt'; +import { createDraftWriteScheduler, loadDraft } from '../../shared/draftCache'; import { shouldQueueBranchSession } from './branchSessionQueue'; import { getPendingSessionItems, @@ -36,6 +37,8 @@ export default class BranchCardSessionManager { private loadTimeline: (opts?: { timelineKey?: string | null; force?: boolean }) => void = undefined!; private getTimeline: () => BranchTimelineData | null = () => null; + private draftWriter = createDraftWriteScheduler(); + private openNewSessionRequest = 0; // New session modal state showNewSession = $state(false); @@ -98,6 +101,10 @@ export default class BranchCardSessionManager { }); } + private draftKeyForMode(mode: BranchSessionType): string { + return `draft:branch-session:${this.getBranch().id}:${mode}`; + } + async startOrQueueSession( mode: BranchSessionType, prompt: string, @@ -120,22 +127,38 @@ export default class BranchCardSessionManager { }); } - openNewSession(mode: BranchSessionType, e?: MouseEvent) { + async openNewSession(mode: BranchSessionType, e?: MouseEvent) { if (mode === 'review' && e?.altKey) { void this.startReviewSessionWithoutDialog(); return; } + const request = ++this.openNewSessionRequest; this.newSessionMode = mode; + const draft = await this.loadNewSessionDraftForMode(mode); + if (request !== this.openNewSessionRequest) return; + this.draftPrompt = draft ?? ''; this.showNewSession = true; } - openNewSessionReferring(hashtagRef: string) { - const hadContent = this.showNewSession && this.draftPrompt.trim(); + async openNewSessionReferring(hashtagRef: string) { + const request = ++this.openNewSessionRequest; + const wasOpen = this.showNewSession; + const hadContent = wasOpen && this.draftPrompt.trim(); + if (!wasOpen) { + this.newSessionMode = 'commit'; + const draft = await this.loadNewSessionDraftForMode('commit'); + if (request !== this.openNewSessionRequest) return; + this.draftPrompt = draft ?? this.draftPrompt; + } this.draftPrompt = buildReferringPrompt(this.draftPrompt, hashtagRef); - if (!hadContent) { + if (!wasOpen || !hadContent) { this.newSessionMode = 'commit'; this.showNewSession = true; } + this.handleNewSessionDraftChange( + { prompt: this.draftPrompt, mode: this.newSessionMode, imageIds: this.draftImageIds }, + { immediate: true } + ); } async startReviewSessionWithoutDialog() { @@ -143,15 +166,33 @@ export default class BranchCardSessionManager { this.showNewSession = false; this.draftPrompt = ''; this.draftImageIds = []; + void this.draftWriter.clearNow(this.draftKeyForMode('review')); const reviewPrompt = 'Review the code changes on this branch.'; await this.startOrQueueSession('review', reviewPrompt); } - handleNewSessionClose(draft: { prompt: string; mode: BranchSessionType; imageIds: string[] }) { + async loadNewSessionDraftForMode(mode: BranchSessionType): Promise { + return loadDraft(this.draftKeyForMode(mode)); + } + + handleNewSessionDraftChange( + draft: { prompt: string; mode: BranchSessionType; imageIds: string[] }, + opts: { immediate?: boolean } = {} + ) { this.draftPrompt = draft.prompt; this.newSessionMode = draft.mode; this.draftImageIds = draft.imageIds; + const key = this.draftKeyForMode(draft.mode); + if (opts.immediate) { + void this.draftWriter.saveNow(key, draft.prompt); + } else { + this.draftWriter.scheduleSave(key, draft.prompt); + } + } + + handleNewSessionClose(draft: { prompt: string; mode: BranchSessionType; imageIds: string[] }) { + this.handleNewSessionDraftChange(draft, { immediate: true }); this.showNewSession = false; } @@ -166,6 +207,7 @@ export default class BranchCardSessionManager { this.showNewSession = false; this.draftPrompt = ''; this.draftImageIds = []; + void this.draftWriter.clearNow(this.draftKeyForMode(data.mode)); const prompt = data.prompt.trim() === '' && data.mode === 'review' diff --git a/apps/staged/src/lib/features/projects/ProjectSection.svelte b/apps/staged/src/lib/features/projects/ProjectSection.svelte index cf72510e2..adb4769bf 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.svelte +++ b/apps/staged/src/lib/features/projects/ProjectSection.svelte @@ -65,6 +65,7 @@ type ReferenceDiffContext, type ReferenceHistoryEntry, } from '../references/referenceHistory.svelte'; + import { createDraftWriteScheduler, loadDraft } from '../../shared/draftCache'; interface Props { project: Project; @@ -121,10 +122,16 @@ let showProjectSessionModal = $state(false); let draftProjectPrompt = $state(''); let draftProjectImageIds = $state([]); + const projectDraftWriter = createDraftWriteScheduler(); + let projectDraftLoadGeneration = 0; let preferredProvider = $derived(getPreferredAgent(agentState.providers) ?? undefined); /** Session IDs for running project sessions (all produce notes). */ let activeSessionIds = $state>(new Set()); + function projectDraftKey(projectId: string): string { + return `draft:project-session:${projectId}`; + } + // ── Live session hints/titles (latest agent message + ACP title for running notes) ── let liveSessionHints = $state>({}); let liveSessionTitles = $state>({}); @@ -173,6 +180,7 @@ onDestroy(() => { liveSessionHintPoller.destroy(); + projectDraftWriter.cancel(); }); // ── In-flight repo sessions spawned by running project sessions ──────── @@ -309,11 +317,40 @@ void ensureHashtagItems(); }); - function openProjectSessionModal() { + async function loadProjectSessionDraft() { + if (draftProjectPrompt.trim()) return; + + const projectId = project.id; + const generation = ++projectDraftLoadGeneration; + try { + const draft = await loadDraft(projectDraftKey(projectId)); + if (generation !== projectDraftLoadGeneration || project.id !== projectId) return; + draftProjectPrompt = draft ?? ''; + } catch (err) { + console.error('[ProjectSection] Failed to load project draft:', err); + } + } + + async function openProjectSessionModal() { + await loadProjectSessionDraft(); showProjectSessionModal = true; void ensureHashtagItems(); } + function handleProjectSessionDraftChange( + draft: { prompt: string; imageIds: string[] }, + opts: { immediate?: boolean } = {} + ) { + draftProjectPrompt = draft.prompt; + draftProjectImageIds = draft.imageIds; + const key = projectDraftKey(project.id); + if (opts.immediate) { + void projectDraftWriter.saveNow(key, draft.prompt); + } else { + projectDraftWriter.scheduleSave(key, draft.prompt); + } + } + async function handleSubmitProjectSession(data: { prompt: string; imageIds: string[]; @@ -325,6 +362,7 @@ if (!text || !provider) return; const imageIdsToSend = data.imageIds.length > 0 ? [...data.imageIds] : undefined; + void projectDraftWriter.clearNow(projectDraftKey(project.id)); try { const response = await commands.startProjectSession( project.id, @@ -482,9 +520,12 @@ return `project-note-${note.id}`; } - function handleProjectNoteNewSessionReferring(ref: string) { + async function handleProjectNoteNewSessionReferring(ref: string) { + await loadProjectSessionDraft(); draftProjectPrompt = buildReferringPrompt(draftProjectPrompt, ref); - openProjectSessionModal(); + showProjectSessionModal = true; + void ensureHashtagItems(); + void projectDraftWriter.saveNow(projectDraftKey(project.id), draftProjectPrompt); } function projectNoteToOpenState(note: ProjectNote, chatOpen = false): OpenProjectNoteState { @@ -802,9 +843,11 @@ initialImageIds={draftProjectImageIds} {hashtagItems} submitDisabledReason={preferredProvider ? null : 'No AI agent available'} + onDraftChange={(draft, opts) => { + handleProjectSessionDraftChange(draft, opts); + }} onClose={(draft) => { - draftProjectPrompt = draft.prompt; - draftProjectImageIds = draft.imageIds; + handleProjectSessionDraftChange(draft, { immediate: true }); showProjectSessionModal = false; }} onSubmit={(data) => { diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte index 8e3de83b1..c31a0052c 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte @@ -89,6 +89,11 @@ provider?: string; acpConfigSelection?: AcpConfigSelection | null; }) => void; + onDraftChange?: ( + draft: { prompt: string; mode: BranchSessionType; imageIds: string[] }, + opts?: { immediate?: boolean } + ) => void | Promise; + loadDraftForMode?: (mode: BranchSessionType) => Promise; } let { @@ -110,6 +115,8 @@ willQueueForMode, onClose, onSubmit, + onDraftChange, + loadDraftForMode, }: Props = $props(); let prompt = $state(''); @@ -121,6 +128,8 @@ providerId: null, acpConfigSelection: null, }); + let modeSwitchGeneration = 0; + let switchingMode = false; // Text-snippet attachment state (modal-local — folded into the prompt on // submit or on close, never persisted to the backend). @@ -264,7 +273,20 @@ // Platform-aware modifier label for the ⌘1/2/3 mode-switch hints. let modKey = $derived(isMac() ? '⌘' : 'Ctrl '); - function switchMode(newMode: BranchSessionType) { + function focusPrompt(selection: 'all' | 'last-line' | null) { + tick().then(() => + focusAfterDialogEnter(textareaEl, (el) => { + el.focus(); + if (selection && el.textContent) { + selectPromptContent(el, selection); + } else { + placeCursorAtEnd(el); + } + }) + ); + } + + async function switchMode(newMode: BranchSessionType) { if (isProjectNote) return; modeMenuOpen = false; if (newMode === currentMode) return; @@ -275,25 +297,52 @@ const isPromptPrefill = (commitPrefill && prompt === commitPrefill) || (notePrefill && prompt === notePrefill); const userDraft = isPromptPrefill ? '' : prompt; + const previousMode = currentMode; + const generation = ++modeSwitchGeneration; + await onDraftChange?.({ prompt: userDraft, mode: previousMode, imageIds }, { immediate: true }); + if (generation !== modeSwitchGeneration) return; + + switchingMode = true; currentMode = newMode; + if (loadDraftForMode) { + let cachedDraft: string | null = null; + try { + cachedDraft = await loadDraftForMode(newMode); + } catch (err) { + console.error('[NewSessionModal] Failed to load draft:', err); + } + if (generation !== modeSwitchGeneration) return; + + const modePrefill = + newMode === 'commit' ? commitPrefill : newMode === 'note' ? notePrefill : ''; + if (cachedDraft) { + prompt = cachedDraft; + focusPrompt(null); + } else if (modePrefill) { + prompt = modePrefill; + focusPrompt(modePrefill.includes('\n') ? 'last-line' : prefillSelection); + } else { + prompt = ''; + focusPrompt(null); + } + switchingMode = false; + onDraftChange?.({ prompt, mode: currentMode, imageIds }); + return; + } + // Apply prefill when switching to commit or note mode with no user draft const modePrefill = newMode === 'commit' ? commitPrefill : newMode === 'note' ? notePrefill : ''; if (modePrefill && !userDraft) { prompt = modePrefill; - const modeSelection = modePrefill.includes('\n') ? 'last-line' : prefillSelection; - tick().then(() => - focusAfterDialogEnter(textareaEl, (el) => { - if (!el.textContent) return; - selectPromptContent(el, modeSelection); - el.focus(); - }) - ); + focusPrompt(modePrefill.includes('\n') ? 'last-line' : prefillSelection); } else { prompt = userDraft; } + switchingMode = false; + onDraftChange?.({ prompt, mode: currentMode, imageIds }); } // Close mode menu on outside click @@ -380,6 +429,11 @@ textSnippets = textSnippets.filter((s) => s.id !== id); } + $effect(() => { + if (!initialized || switchingMode) return; + onDraftChange?.({ prompt, mode: currentMode, imageIds }); + }); + // Focus textarea on mount (one-time). // We await tick() so the DOM reflects the prompt value set by the init effect above. $effect(() => { diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index eaf68f8d3..f5ad00eae 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -201,6 +201,7 @@ import { renderMarkdown as renderSharedMarkdown } from '../../shared/markdown/renderMarkdown'; import { loadPikchrRenderer, type PikchrRenderer } from '../../shared/markdown/pikchrRendering'; import { getNoteFollowupLabel, noteActivityLabel, type LinkedNoteContext } from './noteFreshness'; + import { createDraftWriteScheduler, loadDraft } from '../../shared/draftCache'; import NoteActivityCard from './NoteActivityCard.svelte'; import { splitAtNoteIndicator } from './noteIndicators'; import type { HashtagClickInfo } from '../references/referenceHistory.svelte'; @@ -333,6 +334,14 @@ let expandedTools = $state>(new Set()); let displayRoots = $state([]); let currentDisplayRootKey = ''; + + const replyDraftWriter = createDraftWriteScheduler(); + let loadedDraftSessionId: string | null = null; + let draftLoadGeneration = 0; + + function replyDraftKey(id: string): string { + return `draft:session-reply:${id}`; + } let discoveredFollowupConfig = $state(null); let modelSpecificFollowupConfig = $state(null); let modelSpecificFollowupModelValue = $state(null); @@ -738,6 +747,7 @@ stopPolling(); unlistenStatus?.(); unlistenBackgroundHold?.(); + replyDraftWriter.cancel(); }); // This pane is mounted once and reused across opens (`sessionPanePool` @@ -802,6 +812,32 @@ onSessionChange?.(session); }); + $effect(() => { + const isActive = active; + const id = sessionId; + if (!isActive || !id) { + loadedDraftSessionId = null; + return; + } + + const generation = ++draftLoadGeneration; + loadedDraftSessionId = null; + loadDraft(replyDraftKey(id)).then((draft) => { + if (generation !== draftLoadGeneration || !active || sessionId !== id) return; + inputText = draft ?? ''; + loadedDraftSessionId = id; + tick().then(() => autoResize()); + }); + }); + + $effect(() => { + const isActive = active; + const id = sessionId; + const text = inputText; + if (!isActive || !id || loadedDraftSessionId !== id) return; + replyDraftWriter.scheduleSave(replyDraftKey(id), text); + }); + $effect(() => { const isActive = active; const id = sessionId; @@ -1328,6 +1364,7 @@ if (sending) return; const text = inputText.trim(); if (!text) return; + void replyDraftWriter.clearNow(replyDraftKey(sessionId)); inputText = ''; const imageIdsToSend = replyImageIds.length > 0 ? [...replyImageIds] : undefined; replyImageIds = []; diff --git a/apps/staged/src/lib/shared/draftCache.test.ts b/apps/staged/src/lib/shared/draftCache.test.ts new file mode 100644 index 000000000..d81ce1a0b --- /dev/null +++ b/apps/staged/src/lib/shared/draftCache.test.ts @@ -0,0 +1,54 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const store = vi.hoisted(() => new Map()); + +vi.mock('./persistentStore', () => ({ + getStoreValue: vi.fn(async (key: string) => store.get(key)), + setStoreValue: vi.fn(async (key: string, value: unknown) => { + store.set(key, value); + }), + deleteStoreValue: vi.fn(async (key: string) => { + store.delete(key); + }), +})); + +import { DRAFT_TTL_MS, loadDraft, saveDraft } from './draftCache'; + +describe('draft cache', () => { + beforeEach(() => { + store.clear(); + }); + + it('restores a saved draft before expiry', async () => { + const now = 1_000; + + await saveDraft('draft:one', 'hello', now); + + await expect(loadDraft('draft:one', now + DRAFT_TTL_MS - 1)).resolves.toBe('hello'); + }); + + it('deletes and ignores expired drafts', async () => { + const now = 2_000; + + await saveDraft('draft:expired', 'old', now); + + await expect(loadDraft('draft:expired', now + DRAFT_TTL_MS)).resolves.toBeNull(); + expect(store.has('draft:expired')).toBe(false); + }); + + it('deletes entries for empty values', async () => { + await saveDraft('draft:empty', 'existing', 3_000); + + await saveDraft('draft:empty', '', 4_000); + + expect(store.has('draft:empty')).toBe(false); + }); + + it('keeps separate keys isolated', async () => { + await saveDraft('draft:first', 'first', 5_000); + await saveDraft('draft:second', 'second', 5_000); + + await expect(loadDraft('draft:first', 5_001)).resolves.toBe('first'); + await expect(loadDraft('draft:second', 5_001)).resolves.toBe('second'); + }); +}); diff --git a/apps/staged/src/lib/shared/draftCache.ts b/apps/staged/src/lib/shared/draftCache.ts new file mode 100644 index 000000000..7748d0e32 --- /dev/null +++ b/apps/staged/src/lib/shared/draftCache.ts @@ -0,0 +1,85 @@ +import { deleteStoreValue, getStoreValue, setStoreValue } from './persistentStore'; + +export type DraftEntry = { + value: string; + updatedAt: number; + expiresAt: number; +}; + +export const DRAFT_TTL_MS = 7 * 24 * 60 * 60 * 1000; +export const DRAFT_WRITE_DEBOUNCE_MS = 350; + +function isDraftEntry(value: unknown): value is DraftEntry { + if (!value || typeof value !== 'object') return false; + const entry = value as Partial; + return ( + typeof entry.value === 'string' && + typeof entry.updatedAt === 'number' && + typeof entry.expiresAt === 'number' + ); +} + +export async function loadDraft(key: string, now = Date.now()): Promise { + const entry = await getStoreValue(key); + if (!isDraftEntry(entry)) return null; + + if (entry.expiresAt <= now) { + await deleteStoreValue(key); + return null; + } + + return entry.value; +} + +export async function saveDraft(key: string, value: string, now = Date.now()): Promise { + if (value.length === 0) { + await deleteStoreValue(key); + return; + } + + await setStoreValue(key, { + value, + updatedAt: now, + expiresAt: now + DRAFT_TTL_MS, + }); +} + +export async function clearDraft(key: string): Promise { + await deleteStoreValue(key); +} + +export function createDraftWriteScheduler(delayMs = DRAFT_WRITE_DEBOUNCE_MS) { + let timer: ReturnType | null = null; + let writeQueue: Promise = Promise.resolve(); + + function enqueue(write: () => Promise): Promise { + writeQueue = writeQueue.catch(() => {}).then(write); + return writeQueue; + } + + function cancelPending() { + if (timer) { + clearTimeout(timer); + timer = null; + } + } + + return { + scheduleSave(key: string, value: string) { + cancelPending(); + timer = setTimeout(() => { + timer = null; + void enqueue(() => saveDraft(key, value)); + }, delayMs); + }, + saveNow(key: string, value: string): Promise { + cancelPending(); + return enqueue(() => saveDraft(key, value)); + }, + clearNow(key: string): Promise { + cancelPending(); + return enqueue(() => clearDraft(key)); + }, + cancel: cancelPending, + }; +} From 9373d1234946df3ea363f88dccacefca9fac4761 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 22 Jun 2026 16:20:18 +1000 Subject: [PATCH 02/15] fix: preserve typed project prompt during draft load Signed-off-by: Matt Toohey --- .../staged/src/lib/features/projects/ProjectSection.svelte | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/staged/src/lib/features/projects/ProjectSection.svelte b/apps/staged/src/lib/features/projects/ProjectSection.svelte index adb4769bf..a57e7c894 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.svelte +++ b/apps/staged/src/lib/features/projects/ProjectSection.svelte @@ -318,14 +318,17 @@ }); async function loadProjectSessionDraft() { - if (draftProjectPrompt.trim()) return; + const promptWhenLoadStarted = draftProjectPrompt; + if (promptWhenLoadStarted.trim()) return; const projectId = project.id; const generation = ++projectDraftLoadGeneration; try { const draft = await loadDraft(projectDraftKey(projectId)); if (generation !== projectDraftLoadGeneration || project.id !== projectId) return; - draftProjectPrompt = draft ?? ''; + if (draftProjectPrompt === promptWhenLoadStarted) { + draftProjectPrompt = draft ?? ''; + } } catch (err) { console.error('[ProjectSection] Failed to load project draft:', err); } From bbcf084003c4d6f1846538ccc2374fe55e1e2b85 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 22 Jun 2026 16:23:31 +1000 Subject: [PATCH 03/15] fix: preserve typed session reply during draft load Signed-off-by: Matt Toohey --- apps/staged/src/lib/features/sessions/SessionChatPane.svelte | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index f5ad00eae..f7aa28d6c 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -821,10 +821,13 @@ } const generation = ++draftLoadGeneration; + const inputTextWhenLoadStarted = untrack(() => inputText); loadedDraftSessionId = null; loadDraft(replyDraftKey(id)).then((draft) => { if (generation !== draftLoadGeneration || !active || sessionId !== id) return; - inputText = draft ?? ''; + if (inputText === inputTextWhenLoadStarted) { + inputText = draft ?? ''; + } loadedDraftSessionId = id; tick().then(() => autoResize()); }); From 879d9fc7c4c2a33bc9a87144c8071f7648c79435 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 14 Jul 2026 17:15:43 +1000 Subject: [PATCH 04/15] fix: flush pending draft writes on key switch and teardown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve code review warnings on the unsent-draft caching feature: - draftCache: the write scheduler kept a single debounce timer, so scheduling a save for a new key silently cancelled a pending save for the previous key (e.g. switching sessions in the chat pane dropped the prior session's latest keystrokes). Track the pending key and flush — rather than cancel — when a write targets a different key. - draftCache: replace the scheduler's cancel() with flush(), and call it from SessionChatPane/ProjectSection onDestroy so text typed within the debounce window of teardown is persisted instead of dropped. - BranchCardSessionManager: guard loadNewSessionDraftForMode with try/catch so a failed store read degrades to an empty draft instead of leaving the new-session modal unopened via an unhandled rejection, matching the other draft-load call sites. Adds scheduler unit tests for debouncing, cross-key flushing, and flush-on-close. Signed-off-by: Matt Toohey --- .../BranchCardSessionManager.svelte.ts | 9 +- .../features/projects/ProjectSection.svelte | 2 +- .../features/sessions/SessionChatPane.svelte | 4 +- apps/staged/src/lib/shared/draftCache.test.ts | 88 ++++++++++++++++++- apps/staged/src/lib/shared/draftCache.ts | 34 ++++++- 5 files changed, 129 insertions(+), 8 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index 37bff2821..2c05c974f 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -173,7 +173,14 @@ export default class BranchCardSessionManager { } async loadNewSessionDraftForMode(mode: BranchSessionType): Promise { - return loadDraft(this.draftKeyForMode(mode)); + // Never reject: openNewSession callers fire-and-forget, so a failed store + // read should degrade to an empty draft instead of blocking the modal. + try { + return await loadDraft(this.draftKeyForMode(mode)); + } catch (e) { + console.error('[BranchCard] Failed to load session draft:', e); + return null; + } } handleNewSessionDraftChange( diff --git a/apps/staged/src/lib/features/projects/ProjectSection.svelte b/apps/staged/src/lib/features/projects/ProjectSection.svelte index a57e7c894..26f904582 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.svelte +++ b/apps/staged/src/lib/features/projects/ProjectSection.svelte @@ -180,7 +180,7 @@ onDestroy(() => { liveSessionHintPoller.destroy(); - projectDraftWriter.cancel(); + void projectDraftWriter.flush(); }); // ── In-flight repo sessions spawned by running project sessions ──────── diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index f7aa28d6c..3f1104cb7 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -747,7 +747,9 @@ stopPolling(); unlistenStatus?.(); unlistenBackgroundHold?.(); - replyDraftWriter.cancel(); + // Flush (not cancel) so a reply typed within the debounce window of + // teardown still gets persisted. + void replyDraftWriter.flush(); }); // This pane is mounted once and reused across opens (`sessionPanePool` diff --git a/apps/staged/src/lib/shared/draftCache.test.ts b/apps/staged/src/lib/shared/draftCache.test.ts index d81ce1a0b..8282e0679 100644 --- a/apps/staged/src/lib/shared/draftCache.test.ts +++ b/apps/staged/src/lib/shared/draftCache.test.ts @@ -12,7 +12,13 @@ vi.mock('./persistentStore', () => ({ }), })); -import { DRAFT_TTL_MS, loadDraft, saveDraft } from './draftCache'; +import { + createDraftWriteScheduler, + DRAFT_TTL_MS, + DRAFT_WRITE_DEBOUNCE_MS, + loadDraft, + saveDraft, +} from './draftCache'; describe('draft cache', () => { beforeEach(() => { @@ -52,3 +58,83 @@ describe('draft cache', () => { await expect(loadDraft('draft:second', 5_001)).resolves.toBe('second'); }); }); + +describe('draft write scheduler', () => { + beforeEach(() => { + store.clear(); + vi.useFakeTimers(); + return () => vi.useRealTimers(); + }); + + async function drainWrites() { + await vi.advanceTimersByTimeAsync(DRAFT_WRITE_DEBOUNCE_MS); + } + + it('debounces writes for the same key, keeping only the latest value', async () => { + const scheduler = createDraftWriteScheduler(); + + scheduler.scheduleSave('draft:a', 'hel'); + scheduler.scheduleSave('draft:a', 'hello'); + await drainWrites(); + + expect(store.get('draft:a')).toMatchObject({ value: 'hello' }); + }); + + it('flushes a pending write when scheduling for a different key', async () => { + const scheduler = createDraftWriteScheduler(); + + scheduler.scheduleSave('draft:a', 'from session a'); + scheduler.scheduleSave('draft:b', 'from session b'); + await drainWrites(); + + expect(store.get('draft:a')).toMatchObject({ value: 'from session a' }); + expect(store.get('draft:b')).toMatchObject({ value: 'from session b' }); + }); + + it('flushes a pending write when saveNow/clearNow target a different key', async () => { + const scheduler = createDraftWriteScheduler(); + + scheduler.scheduleSave('draft:a', 'pending a'); + await scheduler.saveNow('draft:b', 'immediate b'); + scheduler.scheduleSave('draft:c', 'pending c'); + await scheduler.clearNow('draft:b'); + await drainWrites(); + + expect(store.get('draft:a')).toMatchObject({ value: 'pending a' }); + expect(store.get('draft:c')).toMatchObject({ value: 'pending c' }); + expect(store.has('draft:b')).toBe(false); + }); + + it('supersedes a pending write when saveNow/clearNow target the same key', async () => { + const scheduler = createDraftWriteScheduler(); + + scheduler.scheduleSave('draft:a', 'stale'); + await scheduler.saveNow('draft:a', 'fresh'); + scheduler.scheduleSave('draft:b', 'stale'); + await scheduler.clearNow('draft:b'); + await drainWrites(); + + expect(store.get('draft:a')).toMatchObject({ value: 'fresh' }); + expect(store.has('draft:b')).toBe(false); + }); + + it('flush persists a pending write without waiting for the debounce', async () => { + const scheduler = createDraftWriteScheduler(); + + scheduler.scheduleSave('draft:a', 'typed right before close'); + await scheduler.flush(); + + expect(store.get('draft:a')).toMatchObject({ value: 'typed right before close' }); + + await drainWrites(); + expect(store.get('draft:a')).toMatchObject({ value: 'typed right before close' }); + }); + + it('flush with nothing pending resolves without writing', async () => { + const scheduler = createDraftWriteScheduler(); + + await scheduler.flush(); + + expect(store.size).toBe(0); + }); +}); diff --git a/apps/staged/src/lib/shared/draftCache.ts b/apps/staged/src/lib/shared/draftCache.ts index 7748d0e32..2d04b47c2 100644 --- a/apps/staged/src/lib/shared/draftCache.ts +++ b/apps/staged/src/lib/shared/draftCache.ts @@ -50,6 +50,8 @@ export async function clearDraft(key: string): Promise { export function createDraftWriteScheduler(delayMs = DRAFT_WRITE_DEBOUNCE_MS) { let timer: ReturnType | null = null; + let pendingKey: string | null = null; + let pendingValue = ''; let writeQueue: Promise = Promise.resolve(); function enqueue(write: () => Promise): Promise { @@ -62,24 +64,48 @@ export function createDraftWriteScheduler(delayMs = DRAFT_WRITE_DEBOUNCE_MS) { clearTimeout(timer); timer = null; } + pendingKey = null; + } + + function flushPending(): Promise { + if (pendingKey === null) return writeQueue; + const key = pendingKey; + const value = pendingValue; + cancelPending(); + return enqueue(() => saveDraft(key, value)); + } + + // One scheduler instance can be reused across targets (e.g. the session + // modal keeps one writer while switching sessions). A pending debounced + // write for a *different* key must be flushed rather than cancelled, or + // that key's latest keystrokes would be dropped. + function settleBeforeWrite(key: string) { + if (pendingKey !== null && pendingKey !== key) { + void flushPending(); + } else { + cancelPending(); + } } return { scheduleSave(key: string, value: string) { - cancelPending(); + settleBeforeWrite(key); + pendingKey = key; + pendingValue = value; timer = setTimeout(() => { timer = null; + pendingKey = null; void enqueue(() => saveDraft(key, value)); }, delayMs); }, saveNow(key: string, value: string): Promise { - cancelPending(); + settleBeforeWrite(key); return enqueue(() => saveDraft(key, value)); }, clearNow(key: string): Promise { - cancelPending(); + settleBeforeWrite(key); return enqueue(() => clearDraft(key)); }, - cancel: cancelPending, + flush: flushPending, }; } From 85aa6ebe59d8f42e68eac558633365b573efb280 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 3 Sep 2026 15:18:43 +1000 Subject: [PATCH 05/15] fix: close draft misattribution races and expire orphaned drafts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the remaining code review comments on unsent-draft caching: - SessionChatPane: bouncing between sessions faster than the write debounce (A → B → A) read A's stale stored value while its newest keystrokes were still pending, restored that stale text, and then let the save effect write it back — superseding the pending newer write, so the newest text reached neither the composer nor the store. Flush the writer before reading. The load now also catches store failures instead of leaving an unhandled rejection, and re-enables saving when it fails so further keystrokes still persist. - NewSessionModal: two mode switches in quick succession (⌘1→⌘2→⌘3) could file one mode's draft under another mode's key — while switch #1 awaited its draft load, `prompt` still held the outgoing mode's text but `currentMode` had already advanced, so switch #2 saved the former under the latter. Skip the outgoing save when a switch is already in flight; that switch has already saved the correct owner's draft. - BranchCardSessionManager: `draft ?? ''` blanked the draft still held in memory when the store read failed, and a close-then-instant-reopen could read the store before the close's immediate save landed. Flush before reading, and fall back to the held draft — but only when it belongs to the mode being opened, which also stops openNewSessionReferring from leaking another mode's text into commit mode. - draftCache: loadDraft only expires the key it reads, and draft keys are scoped to a session/branch/project id, so drafts outliving their owner were never read again and sat in ~/.staged/preferences.json forever. Add sweepExpiredDrafts(), run once at startup, backed by a new listStoreKeys() (with a matching web-mode `list_preference_keys` arm). Adds sweep unit tests covering expiry, namespace isolation, and dropping unparseable entries. Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/web_server.rs | 4 ++ apps/staged/src/App.svelte | 8 ++++ .../BranchCardSessionManager.svelte.ts | 30 +++++++++++++- .../features/sessions/NewSessionModal.svelte | 15 ++++++- .../features/sessions/SessionChatPane.svelte | 39 ++++++++++++++---- apps/staged/src/lib/shared/draftCache.test.ts | 41 +++++++++++++++++++ apps/staged/src/lib/shared/draftCache.ts | 30 +++++++++++++- apps/staged/src/lib/shared/persistentStore.ts | 19 +++++++++ 8 files changed, 173 insertions(+), 13 deletions(-) diff --git a/apps/staged/src-tauri/src/web_server.rs b/apps/staged/src-tauri/src/web_server.rs index b25fe5b58..8a73f520c 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -4150,6 +4150,10 @@ async fn dispatch(command: &str, args: Value, state: &WebAppState) -> Result { + let store = preferences_store(app_handle)?; + Ok(serde_json::to_value(store.keys()).unwrap()) + } "delete_preference" => { let key: String = arg(&args, "key")?; let store = preferences_store(app_handle)?; diff --git a/apps/staged/src/App.svelte b/apps/staged/src/App.svelte index 70e6cd079..ee8c1395f 100644 --- a/apps/staged/src/App.svelte +++ b/apps/staged/src/App.svelte @@ -64,6 +64,7 @@ import { projectsDataStore } from './lib/stores/projectsData.svelte'; import { projectRunActionsStore } from './lib/stores/projectRunActions.svelte'; import { initBloxEnv } from './lib/stores/bloxEnv.svelte'; + import { sweepExpiredDrafts } from './lib/shared/draftCache'; import { listenForSessionStatus } from './lib/listeners/sessionStatusListener'; import { listenForCacheInvalidation } from './lib/listeners/cacheInvalidationListener'; import { listenForPageLifecycle } from './lib/listeners/pageLifecycleListener'; @@ -474,6 +475,13 @@ // Cache the BLOX_ENV value once at startup (process-level, never changes). initBloxEnv(); + // Expire cached unsent drafts whose owning session/branch/project is gone — + // nothing ever reads those keys again, so only this sweep can clear them. + // Fire-and-forget: no view waits on it. + void sweepExpiredDrafts().catch((e) => { + console.error('Failed to sweep expired drafts:', e); + }); + // Restore persisted unread state and sync the dock badge. try { await projectStateStore.initFromStore(); diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index 2c05c974f..b08f3dd34 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -133,10 +133,12 @@ export default class BranchCardSessionManager { return; } const request = ++this.openNewSessionRequest; + const heldMode = this.newSessionMode; + const heldPrompt = this.draftPrompt; this.newSessionMode = mode; const draft = await this.loadNewSessionDraftForMode(mode); if (request !== this.openNewSessionRequest) return; - this.draftPrompt = draft ?? ''; + this.draftPrompt = this.resolveOpenPrompt(mode, draft, heldMode, heldPrompt); this.showNewSession = true; } @@ -145,10 +147,12 @@ export default class BranchCardSessionManager { const wasOpen = this.showNewSession; const hadContent = wasOpen && this.draftPrompt.trim(); if (!wasOpen) { + const heldMode = this.newSessionMode; + const heldPrompt = this.draftPrompt; this.newSessionMode = 'commit'; const draft = await this.loadNewSessionDraftForMode('commit'); if (request !== this.openNewSessionRequest) return; - this.draftPrompt = draft ?? this.draftPrompt; + this.draftPrompt = this.resolveOpenPrompt('commit', draft, heldMode, heldPrompt); } this.draftPrompt = buildReferringPrompt(this.draftPrompt, hashtagRef); if (!wasOpen || !hadContent) { @@ -176,6 +180,10 @@ export default class BranchCardSessionManager { // Never reject: openNewSession callers fire-and-forget, so a failed store // read should degrade to an empty draft instead of blocking the modal. try { + // Settle queued writes first. Closing the modal and switching modes both + // save immediately, so reading the store straight away can return the + // value from before that write landed. + await this.draftWriter.flush(); return await loadDraft(this.draftKeyForMode(mode)); } catch (e) { console.error('[BranchCard] Failed to load session draft:', e); @@ -183,6 +191,24 @@ export default class BranchCardSessionManager { } } + /** + * Prompt to show when opening the modal for `mode`. + * + * `loadNewSessionDraftForMode` returns null both when nothing is stored and + * when the store read fails, so fall back to the draft this manager still + * holds in memory rather than blanking it — but only when that draft belongs + * to the mode being opened, or another mode's text would leak in. + */ + private resolveOpenPrompt( + mode: BranchSessionType, + loaded: string | null, + heldMode: BranchSessionType, + heldPrompt: string + ): string { + if (loaded !== null) return loaded; + return mode === heldMode ? heldPrompt : ''; + } + handleNewSessionDraftChange( draft: { prompt: string; mode: BranchSessionType; imageIds: string[] }, opts: { immediate?: boolean } = {} diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte index c31a0052c..1b94a49f6 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte @@ -298,10 +298,21 @@ (commitPrefill && prompt === commitPrefill) || (notePrefill && prompt === notePrefill); const userDraft = isPromptPrefill ? '' : prompt; const previousMode = currentMode; + // A switch already in flight has saved its own outgoing mode's draft and is + // about to replace `prompt` with the incoming mode's text. Until that lands, + // `prompt` still holds *that* switch's outgoing text while `currentMode` has + // already moved on — so saving here would file one mode's draft under + // another mode's key (⌘1→⌘2→⌘3 in quick succession). + const switchInFlight = switchingMode; const generation = ++modeSwitchGeneration; - await onDraftChange?.({ prompt: userDraft, mode: previousMode, imageIds }, { immediate: true }); - if (generation !== modeSwitchGeneration) return; + if (!switchInFlight) { + await onDraftChange?.( + { prompt: userDraft, mode: previousMode, imageIds }, + { immediate: true } + ); + if (generation !== modeSwitchGeneration) return; + } switchingMode = true; currentMode = newMode; diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index 3f1104cb7..9351d7a55 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -342,6 +342,36 @@ function replyDraftKey(id: string): string { return `draft:session-reply:${id}`; } + + /** + * Restore the cached reply draft for `id` into the composer. + * + * Flushes the writer before reading: bouncing between sessions faster than + * the write debounce (A → B → A) leaves A's newest keystrokes pending, and a + * direct store read would restore the stale value — which the save effect + * below would then write back, superseding the pending newer text. + */ + async function loadReplyDraft(id: string, generation: number, textWhenLoadStarted: string) { + try { + await replyDraftWriter.flush(); + const draft = await loadDraft(replyDraftKey(id)); + if (generation !== draftLoadGeneration || !active || sessionId !== id) return; + if (inputText === textWhenLoadStarted) { + inputText = draft ?? ''; + } + loadedDraftSessionId = id; + await tick(); + autoResize(); + } catch (err) { + console.error('[SessionChatPane] Failed to load reply draft:', err); + // Leave the composer as the user left it, but re-enable saving so + // further keystrokes still reach the store. + if (generation === draftLoadGeneration && active && sessionId === id) { + loadedDraftSessionId = id; + } + } + } + let discoveredFollowupConfig = $state(null); let modelSpecificFollowupConfig = $state(null); let modelSpecificFollowupModelValue = $state(null); @@ -825,14 +855,7 @@ const generation = ++draftLoadGeneration; const inputTextWhenLoadStarted = untrack(() => inputText); loadedDraftSessionId = null; - loadDraft(replyDraftKey(id)).then((draft) => { - if (generation !== draftLoadGeneration || !active || sessionId !== id) return; - if (inputText === inputTextWhenLoadStarted) { - inputText = draft ?? ''; - } - loadedDraftSessionId = id; - tick().then(() => autoResize()); - }); + void loadReplyDraft(id, generation, inputTextWhenLoadStarted); }); $effect(() => { diff --git a/apps/staged/src/lib/shared/draftCache.test.ts b/apps/staged/src/lib/shared/draftCache.test.ts index 8282e0679..5d018753f 100644 --- a/apps/staged/src/lib/shared/draftCache.test.ts +++ b/apps/staged/src/lib/shared/draftCache.test.ts @@ -10,6 +10,7 @@ vi.mock('./persistentStore', () => ({ deleteStoreValue: vi.fn(async (key: string) => { store.delete(key); }), + listStoreKeys: vi.fn(async () => [...store.keys()]), })); import { @@ -18,6 +19,7 @@ import { DRAFT_WRITE_DEBOUNCE_MS, loadDraft, saveDraft, + sweepExpiredDrafts, } from './draftCache'; describe('draft cache', () => { @@ -59,6 +61,45 @@ describe('draft cache', () => { }); }); +describe('expired draft sweep', () => { + beforeEach(() => { + store.clear(); + }); + + it('deletes expired drafts whose owner is gone and keeps live ones', async () => { + const now = 10_000; + await saveDraft('draft:session-reply:deleted-session', 'orphaned', now); + await saveDraft('draft:session-reply:live-session', 'still typing', now + DRAFT_TTL_MS); + + const deleted = await sweepExpiredDrafts(now + DRAFT_TTL_MS); + + expect(deleted).toBe(1); + expect(store.has('draft:session-reply:deleted-session')).toBe(false); + expect(store.get('draft:session-reply:live-session')).toMatchObject({ value: 'still typing' }); + }); + + it('leaves keys outside the draft namespace alone', async () => { + await saveDraft('draft:branch-session:b1:commit', 'old', 1_000); + store.set('branch-prefix', 'matt/'); + store.set('app-mode', 'dark'); + + const deleted = await sweepExpiredDrafts(1_000 + DRAFT_TTL_MS); + + expect(deleted).toBe(1); + expect(store.get('branch-prefix')).toBe('matt/'); + expect(store.get('app-mode')).toBe('dark'); + }); + + it('drops unparseable entries under the draft namespace', async () => { + store.set('draft:project-session:p1', 'written by an older format'); + + const deleted = await sweepExpiredDrafts(20_000); + + expect(deleted).toBe(1); + expect(store.has('draft:project-session:p1')).toBe(false); + }); +}); + describe('draft write scheduler', () => { beforeEach(() => { store.clear(); diff --git a/apps/staged/src/lib/shared/draftCache.ts b/apps/staged/src/lib/shared/draftCache.ts index 2d04b47c2..dc834f335 100644 --- a/apps/staged/src/lib/shared/draftCache.ts +++ b/apps/staged/src/lib/shared/draftCache.ts @@ -1,4 +1,4 @@ -import { deleteStoreValue, getStoreValue, setStoreValue } from './persistentStore'; +import { deleteStoreValue, getStoreValue, listStoreKeys, setStoreValue } from './persistentStore'; export type DraftEntry = { value: string; @@ -6,6 +6,8 @@ export type DraftEntry = { expiresAt: number; }; +/** Namespace every draft key shares, so expired entries can be swept. */ +export const DRAFT_KEY_PREFIX = 'draft:'; export const DRAFT_TTL_MS = 7 * 24 * 60 * 60 * 1000; export const DRAFT_WRITE_DEBOUNCE_MS = 350; @@ -48,6 +50,32 @@ export async function clearDraft(key: string): Promise { await deleteStoreValue(key); } +/** + * Delete every expired `draft:*` entry from the store. + * + * `loadDraft` only expires the one key it reads, and draft keys are scoped to a + * session/branch/project id — so once the owner is deleted its key is never + * read again and the user's text would sit in `~/.staged/preferences.json` + * forever, next to real preferences. Sweeping at startup bounds that to the + * TTL. Entries under the namespace that no longer parse as drafts are dropped + * too; `loadDraft` already treats them as absent. + * + * Returns the number of keys deleted. + */ +export async function sweepExpiredDrafts(now = Date.now()): Promise { + const keys = (await listStoreKeys()).filter((key) => key.startsWith(DRAFT_KEY_PREFIX)); + + let deleted = 0; + for (const key of keys) { + const entry = await getStoreValue(key); + if (isDraftEntry(entry) && entry.expiresAt > now) continue; + await deleteStoreValue(key); + deleted += 1; + } + + return deleted; +} + export function createDraftWriteScheduler(delayMs = DRAFT_WRITE_DEBOUNCE_MS) { let timer: ReturnType | null = null; let pendingKey: string | null = null; diff --git a/apps/staged/src/lib/shared/persistentStore.ts b/apps/staged/src/lib/shared/persistentStore.ts index 58381a610..b922fac54 100644 --- a/apps/staged/src/lib/shared/persistentStore.ts +++ b/apps/staged/src/lib/shared/persistentStore.ts @@ -133,6 +133,25 @@ export async function setStoreValue(key: string, value: T): Promise { await invokeCommand('set_preference', { key, value }); } +/** + * List every key currently held in the persistent store. + * + * Needed by maintenance passes (e.g. expiring cached drafts) that must find + * keys they never recorded anywhere else. + */ +export async function listStoreKeys(): Promise { + if (!backend) { + console.warn('[PersistentStore] Store not initialized, call initPersistentStore() first'); + return []; + } + + if (backend.kind === 'tauri') { + return backend.store.keys(); + } + + return (await invokeCommand('list_preference_keys')) ?? []; +} + /** * Delete a value from the persistent store. */ From 3ed9335725fa3484bd0a8071e992ab65a3f56447 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 3 Sep 2026 16:58:37 +1000 Subject: [PATCH 06/15] fix: keep draft autosave alive across mode switches and loads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve all five findings from the code review of commit af4d634: - NewSessionModal: the draft-save $effect guarded on two non-reactive locals before reading any signals. Every mode switch scheduled the effect and then ran it while the guard tripped, so the run read zero reactive values, Svelte truncated its dependency list to empty, and the effect never ran again — after the first ⌘1/⌘2/⌘3 switch typing no longer reached onDraftChange, so debounced autosave was dead for the life of the modal and only close/submit persisted. Read prompt/mode/imageIds into a local before the guard, and make `switchingMode` $state so the effect re-fires when the gate reopens and schedules text typed during a load window. - SessionChatPane: `loadedDraftSessionId` was a plain let, so the save effect never re-ran when a finished draft load opened its gate — a keystroke typed during the load window was never scheduled and was lost if the user then stopped typing. Make it $state. - BranchCardSessionManager: openNewSession awaited a writer flush plus a store read before showing the dialog, lagging the click by up to two IPC round-trips (an HTTP hop in web mode). Open the modal synchronously seeded with the held draft and let the modal fetch the stored draft itself via loadDraftForMode — preserving the flush-before-read and openNewSessionRequest generation checks from af4d634, adding a don't-clobber-typing check, and still clearing the held prompt when it belongs to a different mode. - NewSessionModal: the mount-focus effect tracked `prefilled`, which flips when the first keystroke overwrites a prefill — the re-run yanked the caret to the end mid-edit. Gate it with a one-shot flag. - web_server: list_preference_keys maps serialization errors into the Result like the neighbouring arms instead of unwrapping. Adds jsdom component tests for the modal's draft autosave via a new "component" vitest project (svelte plugin, stubbed leaf components). The regression test was verified to fail against the pre-fix effect; the others cover populating a synchronous open from the stored draft and preserving text typed during the initial load. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/web_server.rs | 3 +- .../BranchCardSessionManager.svelte.ts | 22 +-- .../NewSessionModal.component.test.ts | 142 ++++++++++++++++++ .../features/sessions/NewSessionModal.svelte | 70 ++++++--- .../features/sessions/SessionChatPane.svelte | 4 +- .../sessions/testStubs/EmptyStub.svelte | 1 + .../sessions/testStubs/PassthroughStub.svelte | 20 +++ .../sessions/testStubs/PromptInputStub.svelte | 24 +++ apps/staged/vitest.config.ts | 36 ++++- 9 files changed, 292 insertions(+), 30 deletions(-) create mode 100644 apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts create mode 100644 apps/staged/src/lib/features/sessions/testStubs/EmptyStub.svelte create mode 100644 apps/staged/src/lib/features/sessions/testStubs/PassthroughStub.svelte create mode 100644 apps/staged/src/lib/features/sessions/testStubs/PromptInputStub.svelte diff --git a/apps/staged/src-tauri/src/web_server.rs b/apps/staged/src-tauri/src/web_server.rs index 8a73f520c..6d596b1f8 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -4152,7 +4152,8 @@ async fn dispatch(command: &str, args: Value, state: &WebAppState) -> Result { let store = preferences_store(app_handle)?; - Ok(serde_json::to_value(store.keys()).unwrap()) + serde_json::to_value(store.keys()) + .map_err(|e| format!("Failed to list preference keys: {e}")) } "delete_preference" => { let key: String = arg(&args, "key")?; diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index b08f3dd34..c0197863a 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -127,18 +127,21 @@ export default class BranchCardSessionManager { }); } - async openNewSession(mode: BranchSessionType, e?: MouseEvent) { + openNewSession(mode: BranchSessionType, e?: MouseEvent) { if (mode === 'review' && e?.altKey) { void this.startReviewSessionWithoutDialog(); return; } - const request = ++this.openNewSessionRequest; - const heldMode = this.newSessionMode; - const heldPrompt = this.draftPrompt; + // Supersede any in-flight referring open so it can't clobber this one. + ++this.openNewSessionRequest; + // Open synchronously — the modal fetches the stored draft itself (via + // loadDraftForMode) so the click isn't gated on store IPC. Seed it with + // the draft held in memory only when that draft belongs to the mode being + // opened, or another mode's text would leak in. + if (mode !== this.newSessionMode) { + this.draftPrompt = ''; + } this.newSessionMode = mode; - const draft = await this.loadNewSessionDraftForMode(mode); - if (request !== this.openNewSessionRequest) return; - this.draftPrompt = this.resolveOpenPrompt(mode, draft, heldMode, heldPrompt); this.showNewSession = true; } @@ -177,8 +180,9 @@ export default class BranchCardSessionManager { } async loadNewSessionDraftForMode(mode: BranchSessionType): Promise { - // Never reject: openNewSession callers fire-and-forget, so a failed store - // read should degrade to an empty draft instead of blocking the modal. + // Never reject: the modal calls this fire-and-forget on mount and on mode + // switch, so a failed store read should degrade to keeping the seeded + // draft instead of blocking the modal. try { // Settle queued writes first. Closing the modal and switching modes both // save immediately, so reading the store straight away can return the diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts new file mode 100644 index 000000000..93254d9ad --- /dev/null +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts @@ -0,0 +1,142 @@ +import { flushSync, mount, tick, unmount, type ComponentProps } from 'svelte'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Branch } from '../../types'; + +// Heavy leaf components are stubbed out: the tests exercise the modal's draft +// effect wiring, not the editor/dialog/picker internals. +vi.mock('$lib/components/ui/dialog', async () => { + const { default: Passthrough } = await import('./testStubs/PassthroughStub.svelte'); + return { Root: Passthrough, Content: Passthrough, Title: Passthrough }; +}); +vi.mock('./HashtagInput.svelte', () => import('./testStubs/PromptInputStub.svelte')); +vi.mock('./ImageAttachment.svelte', () => import('./testStubs/EmptyStub.svelte')); +vi.mock('../agents/AcpConfigPicker.svelte', () => import('./testStubs/EmptyStub.svelte')); +vi.mock('../branches/dragDrop', () => ({ subscribeDragDrop: () => () => {} })); + +import NewSessionModal from './NewSessionModal.svelte'; + +const branch = { + id: 'branch-1', + projectId: 'project-1', + branchName: 'feature-branch', + worktreePath: '/tmp/worktree', +} as unknown as Branch; + +function mountModal(props: Partial>) { + const target = document.createElement('div'); + document.body.appendChild(target); + return mount(NewSessionModal, { + target, + props: { + open: true, + branch, + mode: 'commit', + hashtagItems: [], + onClose: () => {}, + onSubmit: () => {}, + ...props, + }, + }); +} + +/** Let the modal's async draft loads and effect flushes run to completion. */ +async function settle() { + for (let i = 0; i < 8; i++) { + await tick(); + } + flushSync(); +} + +function typeInPrompt(text: string) { + const input = document.querySelector('[data-testid="prompt-input"]'); + if (!input) throw new Error('prompt input not rendered'); + input.textContent = text; + input.dispatchEvent(new InputEvent('input', { bubbles: true })); + flushSync(); +} + +/** ⌘/Ctrl+1/2/3 mode-switch shortcut (jsdom is non-Mac, so Ctrl). */ +function pressModeShortcut(key: string) { + window.dispatchEvent(new KeyboardEvent('keydown', { key, ctrlKey: true, cancelable: true })); + flushSync(); +} + +describe('NewSessionModal draft autosave', () => { + beforeEach(() => { + document.body.innerHTML = ''; + }); + + it('keeps firing onDraftChange for keystrokes after a mode switch', async () => { + // Regression: the draft-save $effect used to guard on non-reactive locals + // before reading any signals; a run during a mode switch then read zero + // reactive values, Svelte truncated its dependency list to empty, and + // autosave was dead for the rest of the modal's life. + const onDraftChange = vi.fn(); + const modal = mountModal({ + onDraftChange, + loadDraftForMode: vi.fn(async () => null), + }); + flushSync(); + await settle(); + + pressModeShortcut('1'); // commit → note + await settle(); + expect(onDraftChange).toHaveBeenCalledWith(expect.objectContaining({ mode: 'note' })); + + onDraftChange.mockClear(); + typeInPrompt('typed after switch'); + await settle(); + expect(onDraftChange).toHaveBeenCalledWith( + expect.objectContaining({ prompt: 'typed after switch', mode: 'note' }) + ); + + unmount(modal); + }); + + it('populates the composer from the stored draft without blocking the open', async () => { + const onDraftChange = vi.fn(); + const modal = mountModal({ + onDraftChange, + loadDraftForMode: vi.fn(async () => 'stored draft'), + }); + flushSync(); + // The modal mounts before the store read resolves… + expect(document.querySelector('[data-testid="prompt-input"]')).not.toBeNull(); + + // …and the stored draft lands afterwards. + await settle(); + expect(onDraftChange).toHaveBeenCalledWith( + expect.objectContaining({ prompt: 'stored draft', mode: 'commit' }) + ); + + unmount(modal); + }); + + it('lets text typed during the initial draft load win, and still saves it', async () => { + let resolveLoad!: (value: string | null) => void; + const onDraftChange = vi.fn(); + const modal = mountModal({ + onDraftChange, + loadDraftForMode: vi.fn(() => new Promise((r) => (resolveLoad = r))), + }); + flushSync(); + await settle(); + // The save gate stays closed while the stored draft is loading, so the + // stale seed is never written back over a newer pending store write… + expect(onDraftChange).not.toHaveBeenCalled(); + + typeInPrompt('typed during load'); + await settle(); + expect(onDraftChange).not.toHaveBeenCalled(); + + resolveLoad('stored draft'); + await settle(); + // …and once the load lands, the typed text is preserved over the stored + // draft and reaches onDraftChange so it still gets persisted. + const prompts = onDraftChange.mock.calls.map(([draft]) => (draft as { prompt: string }).prompt); + expect(prompts).toContain('typed during load'); + expect(prompts).not.toContain('stored draft'); + + unmount(modal); + }); +}); diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte index 1b94a49f6..738b962ac 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte @@ -129,7 +129,10 @@ acpConfigSelection: null, }); let modeSwitchGeneration = 0; - let switchingMode = false; + // $state: gates the draft-save effect while a mode switch or the initial + // draft load is replacing `prompt`, and re-triggers it when the gate opens + // so text typed during the load window still gets scheduled. + let switchingMode = $state(false); // Text-snippet attachment state (modal-local — folded into the prompt on // submit or on close, never persisted to the backend). @@ -407,9 +410,35 @@ prompt = initialPrompt; currentMode = isProjectNote ? 'note' : mode; imageIds = [...initialImageIds]; + // The caller opens the modal synchronously with whatever draft it holds + // in memory; the authoritative stored draft is fetched here so the open + // never waits on store IPC. + if (loadDraftForMode) { + switchingMode = true; + void loadInitialDraft(currentMode); + } } }); + async function loadInitialDraft(initialMode: BranchSessionType) { + const generation = ++modeSwitchGeneration; + const promptWhenLoadStarted = prompt; + let cachedDraft: string | null = null; + try { + cachedDraft = (await loadDraftForMode?.(initialMode)) ?? null; + } catch (err) { + console.error('[NewSessionModal] Failed to load draft:', err); + } + // A mode switch superseded this load; it owns `prompt` and the save gate now. + if (generation !== modeSwitchGeneration) return; + // Apply the stored draft only if the user hasn't typed in the meantime. + if (cachedDraft !== null && cachedDraft !== prompt && prompt === promptWhenLoadStarted) { + prompt = cachedDraft; + focusPrompt(null); + } + switchingMode = false; + } + function addSnippet(text: string) { const trimmed = text.trim(); if (!trimmed) return; @@ -440,28 +469,35 @@ textSnippets = textSnippets.filter((s) => s.id !== id); } + // Read the signals before the guard: an early return that read zero reactive + // values would truncate this effect's dependency list to empty and it would + // never run again, killing autosave for the rest of the modal's life. $effect(() => { + const draft = { prompt, mode: currentMode, imageIds }; if (!initialized || switchingMode) return; - onDraftChange?.({ prompt, mode: currentMode, imageIds }); + onDraftChange?.(draft); }); - // Focus textarea on mount (one-time). + // Focus textarea on mount (one-time). One-shot flag because `prefilled` is a + // tracked dependency that flips when the first keystroke overwrites a + // prefill — re-running would yank the caret to the end mid-edit. // We await tick() so the DOM reflects the prompt value set by the init effect above. + let didInitialFocus = false; $effect(() => { - if (textareaEl) { - const el = textareaEl; - const shouldSelect = prefilled; - tick().then(() => - focusAfterDialogEnter(el, (target) => { - target.focus(); - if (shouldSelect && target.textContent) { - selectPromptContent(target, prefillSelection); - } else { - placeCursorAtEnd(target); - } - }) - ); - } + if (!textareaEl || didInitialFocus) return; + didInitialFocus = true; + const el = textareaEl; + const shouldSelect = prefilled; + tick().then(() => + focusAfterDialogEnter(el, (target) => { + target.focus(); + if (shouldSelect && target.textContent) { + selectPromptContent(target, prefillSelection); + } else { + placeCursorAtEnd(target); + } + }) + ); }); // Resolving hashtag references can re-render the editor. Keep diff-launched diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index 9351d7a55..a890c3e2c 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -336,7 +336,9 @@ let currentDisplayRootKey = ''; const replyDraftWriter = createDraftWriteScheduler(); - let loadedDraftSessionId: string | null = null; + // $state so the save effect re-runs when a finished load opens the gate — + // otherwise a keystroke typed during the load window is never scheduled. + let loadedDraftSessionId = $state(null); let draftLoadGeneration = 0; function replyDraftKey(id: string): string { diff --git a/apps/staged/src/lib/features/sessions/testStubs/EmptyStub.svelte b/apps/staged/src/lib/features/sessions/testStubs/EmptyStub.svelte new file mode 100644 index 000000000..421afc668 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/testStubs/EmptyStub.svelte @@ -0,0 +1 @@ + diff --git a/apps/staged/src/lib/features/sessions/testStubs/PassthroughStub.svelte b/apps/staged/src/lib/features/sessions/testStubs/PassthroughStub.svelte new file mode 100644 index 000000000..9f2ab9449 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/testStubs/PassthroughStub.svelte @@ -0,0 +1,20 @@ + + + +
+ {@render children?.()} +
diff --git a/apps/staged/src/lib/features/sessions/testStubs/PromptInputStub.svelte b/apps/staged/src/lib/features/sessions/testStubs/PromptInputStub.svelte new file mode 100644 index 000000000..4484f8457 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/testStubs/PromptInputStub.svelte @@ -0,0 +1,24 @@ + + + +
(value = e.currentTarget.textContent ?? '')} +>
diff --git a/apps/staged/vitest.config.ts b/apps/staged/vitest.config.ts index dced63c43..aaa792353 100644 --- a/apps/staged/vitest.config.ts +++ b/apps/staged/vitest.config.ts @@ -1,7 +1,39 @@ -import { defineConfig } from 'vitest/config'; +import { resolve } from 'node:path'; +import { svelte } from '@sveltejs/vite-plugin-svelte'; +import { configDefaults, defineConfig } from 'vitest/config'; export default defineConfig({ test: { - include: ['src/**/*.test.ts', '../../packages/diff-viewer/src/**/*.test.ts'], + projects: [ + { + test: { + name: 'unit', + include: ['src/**/*.test.ts', '../../packages/diff-viewer/src/**/*.test.ts'], + exclude: [...configDefaults.exclude, 'src/**/*.component.test.ts'], + }, + }, + { + plugins: [svelte()], + resolve: { + alias: { + $lib: resolve(import.meta.dirname, 'src/lib'), + }, + // Resolve svelte to its client build so `mount` works under jsdom. + conditions: ['browser'], + }, + test: { + name: 'component', + include: ['src/**/*.component.test.ts'], + environment: 'jsdom', + server: { + deps: { + // Svelte libraries ship uncompiled .svelte files; they must go + // through the svelte plugin rather than be externalized to node. + inline: ['@lucide/svelte', 'bits-ui'], + }, + }, + }, + }, + ], }, }); From a1e696ecd7aadfd39d49f15f895c15d5f535892e Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 3 Sep 2026 17:16:45 +1000 Subject: [PATCH 07/15] fix: attribute drafts to their owner mode and surface write failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the four actionable comments from the code review of bfb7a1d8: - NewSessionModal: closing the modal during an in-flight mode switch — after currentMode advanced but before the load replaced `prompt` — paired the outgoing mode's text with the new mode, so the parent's immediate save clobbered the new mode's stored draft; and nothing superseded the switch, which fired one more onDraftChange against the closed modal. Track `promptMode` (the mode `prompt`'s text belongs to, which lags `currentMode` during a switch) and have close, submit, outgoing switch saves, and the save effect read it; close and submit now also bump modeSwitchGeneration so superseded loads are no-ops. Submit is ignored while promptMode lags currentMode, since it would pair one mode's text with the other mode's session type. - NewSessionModal: the switch-in-flight skip also suppressed the outgoing save when only the *initial* draft load was in flight, where no prior save existed — text typed during the load window and followed by ⌘2 was lost from both the store and the held draft. The owner-mode attribution makes the outgoing save correct mid-flight, so the skip is gone and that save always runs. - ProjectSection: loadProjectSessionDraft was the one draft-load site that read the store without awaiting projectDraftWriter.flush(), so a quick reopen after submit (clearNow) or close-with-cleared-text (saveNow('')) could read before the pending delete landed and resurrect the just-cleared prompt. Flush before reading, matching loadNewSessionDraftForMode and loadReplyDraft. - draftCache: the write scheduler swallowed queue errors for the chain while handing the same rejected promise to callers, several of which void it — teardown failures surfaced as unhandled rejections and mid-session failures were never reported. enqueue now catches and console.errors, so queue promises never reject and every failure is logged, matching the load paths. Adds component regression tests for the close-during-switch owner attribution and the save-during-initial-load scenario (both verified to fail pre-fix), a scheduler test that write failures resolve, log, and don't block later writes (also verified pre-fix), and a flush-waits-for-in-flight-clearNow test covering the reopen read. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- .../features/projects/ProjectSection.svelte | 4 + .../NewSessionModal.component.test.ts | 85 ++++++++++++++++++- .../features/sessions/NewSessionModal.svelte | 62 +++++++++----- apps/staged/src/lib/shared/draftCache.test.ts | 47 ++++++++++ apps/staged/src/lib/shared/draftCache.ts | 8 +- 5 files changed, 181 insertions(+), 25 deletions(-) diff --git a/apps/staged/src/lib/features/projects/ProjectSection.svelte b/apps/staged/src/lib/features/projects/ProjectSection.svelte index 26f904582..2310b1f94 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.svelte +++ b/apps/staged/src/lib/features/projects/ProjectSection.svelte @@ -324,6 +324,10 @@ const projectId = project.id; const generation = ++projectDraftLoadGeneration; try { + // Settle queued writes first. Submit clears the stored draft and close + // saves immediately through the writer, so a quick reopen could read the + // store before that write lands and resurrect the just-cleared prompt. + await projectDraftWriter.flush(); const draft = await loadDraft(projectDraftKey(projectId)); if (generation !== projectDraftLoadGeneration || project.id !== projectId) return; if (draftProjectPrompt === promptWhenLoadStarted) { diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts index 93254d9ad..fe76440bc 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts @@ -1,6 +1,6 @@ import { flushSync, mount, tick, unmount, type ComponentProps } from 'svelte'; import { beforeEach, describe, expect, it, vi } from 'vitest'; -import type { Branch } from '../../types'; +import type { Branch, BranchSessionType } from '../../types'; // Heavy leaf components are stubbed out: the tests exercise the modal's draft // effect wiring, not the editor/dialog/picker internals. @@ -61,6 +61,14 @@ function pressModeShortcut(key: string) { flushSync(); } +/** Dismiss the modal via the header close button (same path as Escape). */ +function clickCloseButton() { + const btn = document.querySelector('[aria-label="Close"]'); + if (!btn) throw new Error('close button not rendered'); + btn.click(); + flushSync(); +} + describe('NewSessionModal draft autosave', () => { beforeEach(() => { document.body.innerHTML = ''; @@ -139,4 +147,79 @@ describe('NewSessionModal draft autosave', () => { unmount(modal); }); + + it('files the draft under its owner mode when closed during a mode switch', async () => { + const pendingNoteLoads: Array<(value: string | null) => void> = []; + const onClose = vi.fn(); + const onDraftChange = vi.fn(); + const modal = mountModal({ + onClose, + onDraftChange, + loadDraftForMode: vi.fn((mode: BranchSessionType) => { + if (mode === 'note') return new Promise((r) => pendingNoteLoads.push(r)); + return Promise.resolve(null); + }), + }); + flushSync(); + await settle(); + + typeInPrompt('commit text'); + await settle(); + + pressModeShortcut('1'); // commit → note; note's draft load stays pending + await settle(); + + // Dismissing mid-switch: the text on screen still belongs to commit mode, + // so the close must pair it with commit — filing it under the + // half-switched-to note mode would clobber note's stored draft. + clickCloseButton(); + expect(onClose).toHaveBeenCalledWith( + expect.objectContaining({ prompt: 'commit text', mode: 'commit' }) + ); + + // The superseded switch must not fire another draft change after close. + onDraftChange.mockClear(); + pendingNoteLoads.forEach((resolve) => resolve('note stored draft')); + await settle(); + expect(onDraftChange).not.toHaveBeenCalled(); + + unmount(modal); + }); + + it('saves text typed during the initial load when a mode switch interrupts it', async () => { + let resolveInitialLoad!: (value: string | null) => void; + const onDraftChange = vi.fn(); + const modal = mountModal({ + onDraftChange, + loadDraftForMode: vi.fn((mode: BranchSessionType) => { + if (mode === 'commit') { + return new Promise((r) => (resolveInitialLoad = r)); + } + return Promise.resolve(null); + }), + }); + flushSync(); + await settle(); + + typeInPrompt('typed during load'); + await settle(); + + // Switching modes before the initial load resolves: unlike a switch-in- + // flight, no prior save exists for this text, so the outgoing save must + // still run — filed under commit, the mode the prompt belongs to. + pressModeShortcut('1'); // commit → note + await settle(); + expect(onDraftChange).toHaveBeenCalledWith( + expect.objectContaining({ prompt: 'typed during load', mode: 'commit' }), + expect.objectContaining({ immediate: true }) + ); + + // The superseded initial load must not clobber the new mode's prompt. + resolveInitialLoad('stale stored commit draft'); + await settle(); + const prompts = onDraftChange.mock.calls.map(([draft]) => (draft as { prompt: string }).prompt); + expect(prompts).not.toContain('stale stored commit draft'); + + unmount(modal); + }); }); diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte index 738b962ac..2cd78c71c 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte @@ -4,8 +4,8 @@ A focused modal with a prompt textarea. The mode (commit/note/review) is switchable via a clickable dropdown in the header for branch sessions. Project note sessions keep a fixed note mode and show a simple "Project note" header. - On close, returns whatever text was typed and the current mode so the caller - can restore state if the user re-opens the modal. + On close, returns whatever text was typed and the mode that text belongs to + so the caller can restore state if the user re-opens the modal. Props: branch — the branch to create a session on, for branch sessions @@ -133,6 +133,12 @@ // draft load is replacing `prompt`, and re-triggers it when the gate opens // so text typed during the load window still gets scheduled. let switchingMode = $state(false); + // The mode `prompt`'s text belongs to. Lags `currentMode` while a mode + // switch's draft load is replacing `prompt`, so anything that pairs the + // prompt with a mode (outgoing saves, close, submit) must read this — + // `currentMode` mid-switch would file one mode's text under another mode's + // key and clobber that mode's stored draft. + let promptMode = $state('commit'); // Text-snippet attachment state (modal-local — folded into the prompt on // submit or on close, never persisted to the backend). @@ -300,22 +306,16 @@ const isPromptPrefill = (commitPrefill && prompt === commitPrefill) || (notePrefill && prompt === notePrefill); const userDraft = isPromptPrefill ? '' : prompt; - const previousMode = currentMode; - // A switch already in flight has saved its own outgoing mode's draft and is - // about to replace `prompt` with the incoming mode's text. Until that lands, - // `prompt` still holds *that* switch's outgoing text while `currentMode` has - // already moved on — so saving here would file one mode's draft under - // another mode's key (⌘1→⌘2→⌘3 in quick succession). - const switchInFlight = switchingMode; + // With a switch or the initial draft load in flight, `prompt` still holds + // `promptMode`'s text while `currentMode` has already moved on. Filing the + // outgoing save under the owner keeps rapid switches (⌘1→⌘2→⌘3) and a + // switch during the initial load from saving one mode's text under another + // mode's key — the save is correct mid-flight, so it never needs skipping. + const outgoingMode = promptMode; const generation = ++modeSwitchGeneration; - if (!switchInFlight) { - await onDraftChange?.( - { prompt: userDraft, mode: previousMode, imageIds }, - { immediate: true } - ); - if (generation !== modeSwitchGeneration) return; - } + await onDraftChange?.({ prompt: userDraft, mode: outgoingMode, imageIds }, { immediate: true }); + if (generation !== modeSwitchGeneration) return; switchingMode = true; currentMode = newMode; @@ -341,8 +341,9 @@ prompt = ''; focusPrompt(null); } + promptMode = newMode; switchingMode = false; - onDraftChange?.({ prompt, mode: currentMode, imageIds }); + onDraftChange?.({ prompt, mode: promptMode, imageIds }); return; } @@ -355,8 +356,9 @@ } else { prompt = userDraft; } + promptMode = newMode; switchingMode = false; - onDraftChange?.({ prompt, mode: currentMode, imageIds }); + onDraftChange?.({ prompt, mode: promptMode, imageIds }); } // Close mode menu on outside click @@ -409,6 +411,7 @@ initialized = true; prompt = initialPrompt; currentMode = isProjectNote ? 'note' : mode; + promptMode = currentMode; imageIds = [...initialImageIds]; // The caller opens the modal synchronously with whatever draft it holds // in memory; the authoritative stored draft is fetched here so the open @@ -473,7 +476,7 @@ // values would truncate this effect's dependency list to empty and it would // never run again, killing autosave for the rest of the modal's life. $effect(() => { - const draft = { prompt, mode: currentMode, imageIds }; + const draft = { prompt, mode: promptMode, imageIds }; if (!initialized || switchingMode) return; onDraftChange?.(draft); }); @@ -522,30 +525,43 @@ e?.preventDefault(); // Review mode allows empty prompts; other modes require text or a snippet. if (!canSubmit) return; + // Mid mode-switch the textarea still shows the outgoing mode's text while + // the header shows the new mode — submitting would pair one mode's text + // with the other mode's session type. Ignore the submit; the switch's load + // lands in milliseconds and ⌘Enter can simply be pressed again. + if (switchingMode && promptMode !== currentMode) return; starting = true; // Fold attached snippets into the prompt; the onSubmit signature is unchanged. const finalPrompt = foldSnippetsIntoPrompt(prompt.trim(), textSnippets); + // Supersede any in-flight draft load so it can't touch state after close. + modeSwitchGeneration++; onSubmit({ prompt: finalPrompt, - mode: currentMode, + mode: promptMode, imageIds, provider: acpPickerSelection.providerId ?? undefined, acpConfigSelection: acpPickerSelection.acpConfigSelection, }); // Close immediately; parent handles async start + optimistic timeline row. closed = true; - onClose({ prompt: '', mode: currentMode, imageIds: [] }); + onClose({ prompt: '', mode: promptMode, imageIds: [] }); } function handleClose() { + // Supersede any in-flight mode switch or draft load: once the modal is + // closed it must not replace `prompt` or fire another onDraftChange. + modeSwitchGeneration++; // Snippets are modal-local state, so they'd die with the component on an // accidental dismiss. Fold them into the preserved draft prompt instead — - // reopening then shows the same text the submit would have sent. + // reopening then shows the same text the submit would have sent. The draft + // is filed under `promptMode`: mid mode-switch the text on screen still + // belongs to the outgoing mode, and pairing it with `currentMode` would + // clobber the incoming mode's stored draft. closed = true; onClose({ prompt: foldSnippetsIntoPrompt(prompt, textSnippets), - mode: currentMode, + mode: promptMode, imageIds, }); } diff --git a/apps/staged/src/lib/shared/draftCache.test.ts b/apps/staged/src/lib/shared/draftCache.test.ts index 5d018753f..4729a4244 100644 --- a/apps/staged/src/lib/shared/draftCache.test.ts +++ b/apps/staged/src/lib/shared/draftCache.test.ts @@ -13,6 +13,7 @@ vi.mock('./persistentStore', () => ({ listStoreKeys: vi.fn(async () => [...store.keys()]), })); +import { deleteStoreValue, setStoreValue } from './persistentStore'; import { createDraftWriteScheduler, DRAFT_TTL_MS, @@ -178,4 +179,50 @@ describe('draft write scheduler', () => { expect(store.size).toBe(0); }); + + it('flush waits for an in-flight clearNow so a reopen read cannot resurrect the draft', async () => { + const scheduler = createDraftWriteScheduler(); + await scheduler.saveNow('draft:project-session:p1', 'submitted'); + + // Model a slow store delete (a separate HTTP round-trip in web mode). + vi.mocked(deleteStoreValue).mockImplementationOnce( + (key: string) => + new Promise((resolve) => + setTimeout(() => { + store.delete(key); + resolve(); + }, 50) + ) + ); + + const clearPromise = scheduler.clearNow('draft:project-session:p1'); + // The flush-before-read pattern used by every draft-load site: without the + // flush, this read would land before the delete and resurrect the prompt. + const reopenRead = scheduler.flush().then(() => loadDraft('draft:project-session:p1')); + + await vi.advanceTimersByTimeAsync(50); + await clearPromise; + await expect(reopenRead).resolves.toBeNull(); + }); + + it('logs write failures instead of rejecting, and keeps later writes flowing', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const scheduler = createDraftWriteScheduler(); + vi.mocked(setStoreValue).mockRejectedValueOnce(new Error('store unavailable')); + + // Call sites fire-and-forget these promises (onDestroy flushes, clearNow + // after submit), so a failure must resolve — not reject — and be logged. + await expect(scheduler.saveNow('draft:a', 'dropped')).resolves.toBeUndefined(); + expect(consoleError).toHaveBeenCalledWith( + '[draftCache] Failed to persist draft:', + expect.any(Error) + ); + + await scheduler.saveNow('draft:a', 'second attempt'); + expect(store.get('draft:a')).toMatchObject({ value: 'second attempt' }); + } finally { + consoleError.mockRestore(); + } + }); }); diff --git a/apps/staged/src/lib/shared/draftCache.ts b/apps/staged/src/lib/shared/draftCache.ts index dc834f335..5234135e3 100644 --- a/apps/staged/src/lib/shared/draftCache.ts +++ b/apps/staged/src/lib/shared/draftCache.ts @@ -83,7 +83,13 @@ export function createDraftWriteScheduler(delayMs = DRAFT_WRITE_DEBOUNCE_MS) { let writeQueue: Promise = Promise.resolve(); function enqueue(write: () => Promise): Promise { - writeQueue = writeQueue.catch(() => {}).then(write); + // The queue's promises never reject: several call sites fire-and-forget + // (`void` on onDestroy flushes and clearNow), so a rejected write would + // surface as an unhandled rejection at teardown and be invisible + // mid-session. Log it here instead and keep the chain alive. + writeQueue = writeQueue.then(write).catch((err) => { + console.error('[draftCache] Failed to persist draft:', err); + }); return writeQueue; } From 09be27030c74917137749b90ee3c94eaccbac836 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 3 Sep 2026 17:41:42 +1000 Subject: [PATCH 08/15] fix: stop failed loads and racing opens from crossing draft owners Resolve the three actionable comments from the code review of 1080f849: - SessionChatPane: the loadDraft failure path re-enabled the save gate while the composer could still hold the previous session's text. On a switch A -> B with B's load failing, setting loadedDraftSessionId ($state) re-fired the save effect immediately, scheduling A's text under B's key and overwriting B's stored draft. Suppress exactly that first save-effect run when the composer still holds the text from before the load began (inherited text); text typed during the load window was written while B was displayed, so it still saves, and the success path's write-back (which refreshes the TTL) is untouched. - BranchCardSessionManager: openNewSessionReferring set newSessionMode = 'commit' before its awaited flush+read, so a concurrent openNewSession('commit') click saw mode === newSessionMode and kept a draftPrompt that actually belonged to the held mode, seeding commit mode with e.g. note's text. The assignment was redundant - it's set again after the generation guard, and the branch reads the captured heldMode/heldPrompt - so it's gone, closing the window. - ProjectSection: the project-session modal was the last open path that awaited store IPC (flush + read) before showing the dialog. It now opens synchronously seeded with the held draft and passes loadDraftForMode so NewSessionModal fetches the authoritative stored draft itself, with its existing generation and don't-clobber-typing checks - matching the BranchCard flow from bfb7a1d8. The flush-before- read stays inside the load function, and the referring path reuses it. Adds jsdom component regression tests: SessionChatPane tests that a failed load neither files the previous session's text under the new session's key nor drops text typed during the failing load (plus a reactiveProps test helper for post-mount prop switches), and BranchCardSessionManager tests that a commit open racing the referring load gets a clean composer while an unraced referring open still builds on the stored commit draft. The two misattribution tests were verified to fail against the pre-fix code. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- ...BranchCardSessionManager.component.test.ts | 97 +++++++++++ .../BranchCardSessionManager.svelte.ts | 6 +- .../features/projects/ProjectSection.svelte | 43 +++-- .../SessionChatPane.component.test.ts | 159 ++++++++++++++++++ .../features/sessions/SessionChatPane.svelte | 18 +- .../testStubs/reactiveProps.svelte.ts | 9 + 6 files changed, 316 insertions(+), 16 deletions(-) create mode 100644 apps/staged/src/lib/features/branches/BranchCardSessionManager.component.test.ts create mode 100644 apps/staged/src/lib/features/sessions/SessionChatPane.component.test.ts create mode 100644 apps/staged/src/lib/features/sessions/testStubs/reactiveProps.svelte.ts diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.component.test.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.component.test.ts new file mode 100644 index 000000000..83450a63d --- /dev/null +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.component.test.ts @@ -0,0 +1,97 @@ +// Runs in the "component" vitest project: the manager is a .svelte.ts class +// whose $state/$derived fields need the svelte plugin to compile. +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Branch } from '../../types'; + +const { writer, loadDraftMock } = vi.hoisted(() => ({ + writer: { + scheduleSave: vi.fn(), + saveNow: vi.fn(async () => {}), + clearNow: vi.fn(async () => {}), + flush: vi.fn(async () => {}), + }, + loadDraftMock: vi.fn(async (_key: string): Promise => null), +})); + +vi.mock('../../shared/draftCache', () => ({ + createDraftWriteScheduler: () => writer, + loadDraft: (key: string) => loadDraftMock(key), +})); +vi.mock('./branchSessionLaunch.svelte', () => ({ + getPendingSessionItems: () => [], + hasPendingQueuedSession: () => false, + hasPendingSessionStart: () => false, + startOrQueueBranchSessionWithPending: vi.fn(async () => {}), +})); + +import BranchCardSessionManager from './BranchCardSessionManager.svelte'; + +const branch = { id: 'branch-1', projectId: 'project-1' } as unknown as Branch; + +function createManager() { + return new BranchCardSessionManager({ + getBranch: () => branch, + getIsRemote: () => false, + loadTimeline: () => {}, + getTimeline: () => null, + }); +} + +describe('BranchCardSessionManager referring open', () => { + beforeEach(() => { + writer.scheduleSave.mockClear(); + writer.saveNow.mockClear(); + writer.flush.mockClear(); + loadDraftMock.mockReset(); + loadDraftMock.mockImplementation(async () => null); + }); + + it("does not leak another mode's held draft into a commit open racing the referring load", async () => { + // Regression: openNewSessionReferring used to set newSessionMode='commit' + // before its awaited flush+read, so a concurrent openNewSession('commit') + // saw mode === newSessionMode and kept a draftPrompt that actually + // belonged to the held mode, seeding commit mode with note's text. + const mgr = createManager(); + // A note draft held in memory from a previous close. + mgr.handleNewSessionClose({ prompt: 'note text', mode: 'note', imageIds: [] }); + + // The referring open's store read stays in flight… + let resolveCommitLoad!: (value: string | null) => void; + loadDraftMock.mockImplementation( + () => + new Promise((resolve) => { + resolveCommitLoad = resolve; + }) + ); + const referringOpen = mgr.openNewSessionReferring('#note:abc123'); + + // …while the user clicks "New commit". The commit modal must open with a + // clean composer, not the held note text. + mgr.openNewSession('commit'); + expect(mgr.showNewSession).toBe(true); + expect(mgr.newSessionMode).toBe('commit'); + expect(mgr.draftPrompt).toBe(''); + + // The superseded referring open must not touch the prompt when it lands. + // (Its store read only starts once the writer flush ahead of it settles.) + await vi.waitFor(() => expect(loadDraftMock).toHaveBeenCalled()); + resolveCommitLoad('stored commit draft'); + await referringOpen; + expect(mgr.draftPrompt).toBe(''); + expect(mgr.newSessionMode).toBe('commit'); + }); + + it('still builds the referring prompt from the stored commit draft when unraced', async () => { + const mgr = createManager(); + mgr.handleNewSessionClose({ prompt: 'note text', mode: 'note', imageIds: [] }); + + loadDraftMock.mockImplementation(async () => 'stored commit draft'); + await mgr.openNewSessionReferring('#note:abc123'); + + expect(mgr.showNewSession).toBe(true); + expect(mgr.newSessionMode).toBe('commit'); + expect(mgr.draftPrompt).toContain('stored commit draft'); + expect(mgr.draftPrompt).toContain('#note:abc123'); + expect(mgr.draftPrompt).not.toContain('note text'); + }); +}); diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index c0197863a..b626d4d26 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -152,7 +152,11 @@ export default class BranchCardSessionManager { if (!wasOpen) { const heldMode = this.newSessionMode; const heldPrompt = this.draftPrompt; - this.newSessionMode = 'commit'; + // newSessionMode is only assigned after the generation guard below: + // setting it to 'commit' before the awaited read would let a concurrent + // openNewSession('commit') see mode === newSessionMode and keep a + // draftPrompt that actually belongs to heldMode, seeding commit mode + // with another mode's text. const draft = await this.loadNewSessionDraftForMode('commit'); if (request !== this.openNewSessionRequest) return; this.draftPrompt = this.resolveOpenPrompt('commit', draft, heldMode, heldPrompt); diff --git a/apps/staged/src/lib/features/projects/ProjectSection.svelte b/apps/staged/src/lib/features/projects/ProjectSection.svelte index 2310b1f94..9b2044cb7 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.svelte +++ b/apps/staged/src/lib/features/projects/ProjectSection.svelte @@ -317,29 +317,43 @@ void ensureHashtagItems(); }); + /** + * Read the stored project draft, settling queued writes first: submit + * clears the stored draft and close saves immediately through the writer, + * so reading the store straight away could return the value from before + * that write lands and resurrect a just-cleared prompt. + * + * Never rejects — the modal calls this fire-and-forget on mount, so a + * failed store read degrades to keeping the seeded draft. + */ + async function loadStoredProjectDraft(): Promise { + const projectId = project.id; + try { + await projectDraftWriter.flush(); + return await loadDraft(projectDraftKey(projectId)); + } catch (err) { + console.error('[ProjectSection] Failed to load project draft:', err); + return null; + } + } + async function loadProjectSessionDraft() { const promptWhenLoadStarted = draftProjectPrompt; if (promptWhenLoadStarted.trim()) return; const projectId = project.id; const generation = ++projectDraftLoadGeneration; - try { - // Settle queued writes first. Submit clears the stored draft and close - // saves immediately through the writer, so a quick reopen could read the - // store before that write lands and resurrect the just-cleared prompt. - await projectDraftWriter.flush(); - const draft = await loadDraft(projectDraftKey(projectId)); - if (generation !== projectDraftLoadGeneration || project.id !== projectId) return; - if (draftProjectPrompt === promptWhenLoadStarted) { - draftProjectPrompt = draft ?? ''; - } - } catch (err) { - console.error('[ProjectSection] Failed to load project draft:', err); + const draft = await loadStoredProjectDraft(); + if (generation !== projectDraftLoadGeneration || project.id !== projectId) return; + if (draftProjectPrompt === promptWhenLoadStarted) { + draftProjectPrompt = draft ?? ''; } } - async function openProjectSessionModal() { - await loadProjectSessionDraft(); + function openProjectSessionModal() { + // Open synchronously seeded with the held draft — the modal fetches the + // authoritative stored draft itself (via loadDraftForMode) so the click + // isn't gated on store IPC (a flush plus a read; HTTP hops in web mode). showProjectSessionModal = true; void ensureHashtagItems(); } @@ -850,6 +864,7 @@ initialImageIds={draftProjectImageIds} {hashtagItems} submitDisabledReason={preferredProvider ? null : 'No AI agent available'} + loadDraftForMode={() => loadStoredProjectDraft()} onDraftChange={(draft, opts) => { handleProjectSessionDraftChange(draft, opts); }} diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.component.test.ts b/apps/staged/src/lib/features/sessions/SessionChatPane.component.test.ts new file mode 100644 index 000000000..6ddb36715 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.component.test.ts @@ -0,0 +1,159 @@ +import { flushSync, mount, tick, unmount } from 'svelte'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Session } from '../../types'; + +const { writer, loadDraftMock } = vi.hoisted(() => ({ + writer: { + scheduleSave: vi.fn(), + saveNow: vi.fn(async () => {}), + clearNow: vi.fn(async () => {}), + flush: vi.fn(async () => {}), + }, + loadDraftMock: vi.fn(async (_key: string): Promise => null), +})); + +// Heavy leaf components are stubbed out: the tests exercise the pane's +// reply-draft effect wiring, not the transcript/composer internals. +vi.mock('./ChatComposer.svelte', async () => { + const { default: PromptInputStub } = await import('./testStubs/PromptInputStub.svelte'); + return { default: PromptInputStub, composerControlClass: '' }; +}); +vi.mock('../agents/AcpFixedConfigPicker.svelte', () => import('./testStubs/EmptyStub.svelte')); +vi.mock('./PlanCard.svelte', () => import('./testStubs/EmptyStub.svelte')); +vi.mock('./PipelineSteps.svelte', () => import('./testStubs/EmptyStub.svelte')); +vi.mock('./tool-calls/ToolCallCard.svelte', () => import('./testStubs/EmptyStub.svelte')); +vi.mock('./tool-calls/ToolCallHeader.svelte', () => import('./testStubs/EmptyStub.svelte')); +vi.mock('./NoteActivityCard.svelte', () => import('./testStubs/EmptyStub.svelte')); +vi.mock( + '../../shared/markdown/DiagramViewerModal.svelte', + () => import('./testStubs/EmptyStub.svelte') +); +vi.mock('../branches/dragDrop', () => ({ subscribeDragDrop: () => () => {} })); +vi.mock('../agents/agent.svelte', () => ({ agentState: { providers: [] } })); +vi.mock('../settings/preferences.svelte', () => ({ setAcpConfigPref: vi.fn() })); +vi.mock('../../transport', async (importOriginal) => ({ + ...(await importOriginal()), + listenToEvent: () => () => {}, +})); +vi.mock('../../shared/draftCache', () => ({ + createDraftWriteScheduler: () => writer, + loadDraft: (key: string) => loadDraftMock(key), +})); +vi.mock('../../commands', async (importOriginal) => ({ + ...(await importOriginal()), + getSession: async (id: string) => + ({ id, status: 'completed', provider: null, workingDir: null }) as unknown as Session, + getSessionMessages: async () => ({ data: [] }), + getSessionMessagesSince: async () => [], + getSessionAcpMetadataMessages: async () => [], + getSessionAcpMetadataMessagesSince: async () => [], + listQueuedSessionMessages: async () => [], +})); + +import SessionChatPane from './SessionChatPane.svelte'; +import { reactiveProps } from './testStubs/reactiveProps.svelte'; + +function replyKey(id: string): string { + return `draft:session-reply:${id}`; +} + +function mountPane(sessionId: string) { + const props = reactiveProps({ active: true, sessionId, hashtagItems: [] }); + const target = document.createElement('div'); + document.body.appendChild(target); + const pane = mount(SessionChatPane, { target, props }); + return { pane, props }; +} + +/** Let the pane's async session/draft loads and effect flushes run to completion. */ +async function settle() { + for (let i = 0; i < 8; i++) { + await tick(); + } + flushSync(); +} + +function typeInComposer(text: string) { + const input = document.querySelector('[data-testid="prompt-input"]'); + if (!input) throw new Error('composer input not rendered'); + input.textContent = text; + input.dispatchEvent(new InputEvent('input', { bubbles: true })); + flushSync(); +} + +describe('SessionChatPane reply draft autosave', () => { + beforeEach(() => { + document.body.innerHTML = ''; + writer.scheduleSave.mockClear(); + writer.flush.mockClear(); + loadDraftMock.mockReset(); + loadDraftMock.mockImplementation(async () => null); + vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + it("does not file the previous session's composer text under a session whose draft load failed", async () => { + // Regression: on a switch A → B where B's draft load throws, the composer + // still holds A's text; re-enabling the save gate used to fire the save + // effect immediately and schedule that text under B's key, overwriting + // B's stored draft. + loadDraftMock.mockImplementation(async (key: string) => { + if (key === replyKey('session-a')) return 'draft for A'; + throw new Error('store unavailable'); + }); + + const { pane, props } = mountPane('session-a'); + flushSync(); + await settle(); + // Session A's stored draft lands in the composer and is written back + // under A's own key (the success-path write-back that refreshes the TTL). + expect(writer.scheduleSave).toHaveBeenCalledWith(replyKey('session-a'), 'draft for A'); + + writer.scheduleSave.mockClear(); + props.sessionId = 'session-b'; + flushSync(); + await settle(); + // The failed load must not save A's leftover text under B's key. + expect(writer.scheduleSave).not.toHaveBeenCalled(); + + // The save gate is re-enabled: an edit made while B is displayed is B's + // draft and persists under B's key. + typeInComposer('a reply for B'); + await settle(); + expect(writer.scheduleSave).toHaveBeenCalledWith(replyKey('session-b'), 'a reply for B'); + + unmount(pane); + }); + + it('saves text typed while a draft load was failing', async () => { + let rejectLoad!: (err: Error) => void; + loadDraftMock.mockImplementation((key: string) => { + if (key === replyKey('session-a')) return Promise.resolve(null); + return new Promise((_resolve, reject) => { + rejectLoad = reject; + }); + }); + + const { pane, props } = mountPane('session-a'); + flushSync(); + await settle(); + + props.sessionId = 'session-b'; + flushSync(); + await settle(); + writer.scheduleSave.mockClear(); + + // Typed while B's load is still in flight: the gate is closed, so nothing + // is scheduled yet… + typeInComposer('typed during load'); + await settle(); + expect(writer.scheduleSave).not.toHaveBeenCalled(); + + // …and when the load fails, the text was typed while B was displayed, so + // it saves under B immediately rather than waiting for another keystroke. + rejectLoad(new Error('store unavailable')); + await settle(); + expect(writer.scheduleSave).toHaveBeenCalledWith(replyKey('session-b'), 'typed during load'); + + unmount(pane); + }); +}); diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index a890c3e2c..b046a0adc 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -340,6 +340,10 @@ // otherwise a keystroke typed during the load window is never scheduled. let loadedDraftSessionId = $state(null); let draftLoadGeneration = 0; + // One-shot: set when a failed load re-opens the save gate while the composer + // may still hold the previously shown session's text; the save effect skips + // exactly that run so inherited text isn't filed under this session's key. + let suppressNextReplySave = false; function replyDraftKey(id: string): string { return `draft:session-reply:${id}`; @@ -367,8 +371,15 @@ } catch (err) { console.error('[SessionChatPane] Failed to load reply draft:', err); // Leave the composer as the user left it, but re-enable saving so - // further keystrokes still reach the store. + // further keystrokes still reach the store. Re-enabling fires the save + // effect once immediately: when the composer still holds exactly the + // text from before this load began, that text was inherited from the + // previously shown session (a switch A → B where B's load failed), and + // letting that run through would overwrite B's stored draft with A's + // text. Suppress it — text typed during the load window was written + // while this session was displayed, so it still saves. if (generation === draftLoadGeneration && active && sessionId === id) { + suppressNextReplySave = inputText === textWhenLoadStarted; loadedDraftSessionId = id; } } @@ -856,6 +867,7 @@ const generation = ++draftLoadGeneration; const inputTextWhenLoadStarted = untrack(() => inputText); + suppressNextReplySave = false; loadedDraftSessionId = null; void loadReplyDraft(id, generation, inputTextWhenLoadStarted); }); @@ -865,6 +877,10 @@ const id = sessionId; const text = inputText; if (!isActive || !id || loadedDraftSessionId !== id) return; + if (suppressNextReplySave) { + suppressNextReplySave = false; + return; + } replyDraftWriter.scheduleSave(replyDraftKey(id), text); }); diff --git a/apps/staged/src/lib/features/sessions/testStubs/reactiveProps.svelte.ts b/apps/staged/src/lib/features/sessions/testStubs/reactiveProps.svelte.ts new file mode 100644 index 000000000..a1a3e645c --- /dev/null +++ b/apps/staged/src/lib/features/sessions/testStubs/reactiveProps.svelte.ts @@ -0,0 +1,9 @@ +/** + * Wrap a props object in $state so component tests can mutate props after + * mount() and have the change propagate (plain objects passed to mount() are + * read once and never update). + */ +export function reactiveProps>(initial: T): T { + const props = $state(initial); + return props; +} From d52d4fbe5726b7d493bfdd3c871c35a2393c430d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 3 Sep 2026 17:53:40 +1000 Subject: [PATCH 09/15] fix: keep image drafts per-mode when opening the session modal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the single actionable comment from the code review of a5c74ed0: opening the new-session modal in a different mode than the held draft cleared draftPrompt but not draftImageIds, so images attached while drafting (e.g.) a note seeded the commit modal via initialImageIds — violating the branch's per-mode draft invariant. Images aren't persisted to the store, so the in-memory list is the only copy and can simply be cleared: - BranchCardSessionManager.openNewSession: clear draftImageIds alongside draftPrompt when the requested mode differs from the held one. - openNewSessionReferring needed the same treatment: its fresh-open path targets commit mode while draftImageIds may still hold another mode's images. Clear them when the held mode isn't commit — the same rule resolveOpenPrompt applies to the held prompt. Held commit-mode images still survive a referring open. Extends the manager component tests: different-mode opens (plain and referring) get a clean image list while same-mode/commit-mode opens keep the held images. Both clearing tests were verified to fail against the pre-fix code. Co-Authored-By: Claude Fable 5 Signed-off-by: Matt Toohey --- ...BranchCardSessionManager.component.test.ts | 59 ++++++++++++++++--- .../BranchCardSessionManager.svelte.ts | 8 ++- 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.component.test.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.component.test.ts index 83450a63d..ce9bc9d6d 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.component.test.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.component.test.ts @@ -37,15 +37,15 @@ function createManager() { }); } -describe('BranchCardSessionManager referring open', () => { - beforeEach(() => { - writer.scheduleSave.mockClear(); - writer.saveNow.mockClear(); - writer.flush.mockClear(); - loadDraftMock.mockReset(); - loadDraftMock.mockImplementation(async () => null); - }); +beforeEach(() => { + writer.scheduleSave.mockClear(); + writer.saveNow.mockClear(); + writer.flush.mockClear(); + loadDraftMock.mockReset(); + loadDraftMock.mockImplementation(async () => null); +}); +describe('BranchCardSessionManager referring open', () => { it("does not leak another mode's held draft into a commit open racing the referring load", async () => { // Regression: openNewSessionReferring used to set newSessionMode='commit' // before its awaited flush+read, so a concurrent openNewSession('commit') @@ -83,7 +83,7 @@ describe('BranchCardSessionManager referring open', () => { it('still builds the referring prompt from the stored commit draft when unraced', async () => { const mgr = createManager(); - mgr.handleNewSessionClose({ prompt: 'note text', mode: 'note', imageIds: [] }); + mgr.handleNewSessionClose({ prompt: 'note text', mode: 'note', imageIds: ['note-img'] }); loadDraftMock.mockImplementation(async () => 'stored commit draft'); await mgr.openNewSessionReferring('#note:abc123'); @@ -93,5 +93,46 @@ describe('BranchCardSessionManager referring open', () => { expect(mgr.draftPrompt).toContain('stored commit draft'); expect(mgr.draftPrompt).toContain('#note:abc123'); expect(mgr.draftPrompt).not.toContain('note text'); + // Images aren't persisted, but the held ones belong to note mode and must + // not seed the commit modal. + expect(mgr.draftImageIds).toEqual([]); + }); + + it('keeps held commit-mode images across a referring open', async () => { + const mgr = createManager(); + mgr.handleNewSessionClose({ prompt: 'commit text', mode: 'commit', imageIds: ['commit-img'] }); + + await mgr.openNewSessionReferring('#note:abc123'); + + expect(mgr.newSessionMode).toBe('commit'); + expect(mgr.draftPrompt).toContain('commit text'); + expect(mgr.draftImageIds).toEqual(['commit-img']); + }); +}); + +describe('BranchCardSessionManager per-mode image drafts', () => { + it("clears another mode's held images when opening a different mode", () => { + // Regression: openNewSession cleared draftPrompt when opening a different + // mode but left draftImageIds, so images attached while drafting a note + // seeded the commit modal via initialImageIds. + const mgr = createManager(); + mgr.handleNewSessionClose({ prompt: 'note text', mode: 'note', imageIds: ['note-img'] }); + + mgr.openNewSession('commit'); + + expect(mgr.showNewSession).toBe(true); + expect(mgr.newSessionMode).toBe('commit'); + expect(mgr.draftPrompt).toBe(''); + expect(mgr.draftImageIds).toEqual([]); + }); + + it('keeps held images when reopening the same mode', () => { + const mgr = createManager(); + mgr.handleNewSessionClose({ prompt: 'note text', mode: 'note', imageIds: ['note-img'] }); + + mgr.openNewSession('note'); + + expect(mgr.draftPrompt).toBe('note text'); + expect(mgr.draftImageIds).toEqual(['note-img']); }); }); diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index b626d4d26..443a34dd9 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -137,9 +137,10 @@ export default class BranchCardSessionManager { // Open synchronously — the modal fetches the stored draft itself (via // loadDraftForMode) so the click isn't gated on store IPC. Seed it with // the draft held in memory only when that draft belongs to the mode being - // opened, or another mode's text would leak in. + // opened, or another mode's text or images would leak in. if (mode !== this.newSessionMode) { this.draftPrompt = ''; + this.draftImageIds = []; } this.newSessionMode = mode; this.showNewSession = true; @@ -160,6 +161,11 @@ export default class BranchCardSessionManager { const draft = await this.loadNewSessionDraftForMode('commit'); if (request !== this.openNewSessionRequest) return; this.draftPrompt = this.resolveOpenPrompt('commit', draft, heldMode, heldPrompt); + // Images aren't persisted to the store, so the held ones survive only + // when they already belong to commit mode — same rule as the prompt. + if (heldMode !== 'commit') { + this.draftImageIds = []; + } } this.draftPrompt = buildReferringPrompt(this.draftPrompt, hashtagRef); if (!wasOpen || !hadContent) { From c39d474f85d08bccd748cb5d709892664cc5d66d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Sat, 17 Oct 2026 14:48:49 +1100 Subject: [PATCH 10/15] fix: guard project referring opens and keep text typed mid mode-switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the two remaining comments from the code review of 063330b6: - ProjectSection: handleProjectNoteNewSessionReferring awaited its flush+read with no supersede guard. A plain "New project note" click landing in that window opened the modal synchronously without the ref; when the referring handler resumed it saved the ref-appended prompt to the store, which the already-open modal (it reads initialPrompt once) never displayed and whose next keystroke's save overwrote - the ref was silently lost. Add a projectSessionOpenRequest counter mirroring BranchCardSessionManager.openNewSessionRequest: every open bumps it, and a referring open that finds itself overtaken becomes a no-op, matching the BranchCard flow. - NewSessionModal: the mode-switch load overwrote `prompt` wholesale when it landed, so keystrokes typed between ⌘1/⌘2/⌘3 and the load resolving were dropped from both modes - the outgoing save had already been snapshotted and the save effect is gated by switchingMode. Snapshot the prompt once the gate closes (keystrokes before that were still shown under the outgoing mode and reach its key through the save effect) and, when the load lands, diff it against the live prompt with a new insertedText() helper: the inserted part belongs to the incoming mode, so it is placed after the stored draft, or stands alone when there is none (a prefill would bury it). The existing post-load onDraftChange and the reopened save effect then file it under the new mode's key. The NewSessionModal component suite was failing wholesale at HEAD: main's f9125791 (#956) added Dialog.ResizeHandle to the modal after the dialog mock was written. The mock now stubs it. Adds jsdom component regression tests: a ProjectSection suite (with a TimelineContextMenu stub exposing the referring trigger) covering a plain open racing the referring load - verified to fail pre-fix - plus the unraced referring prompt; and NewSessionModal tests that text typed during a mode-switch load is appended to the stored draft and kept when there is none, both verified to fail pre-fix. insertedText gets unit tests for appends, mid-string inserts, replacements, and deletions. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .../projects/ProjectSection.component.test.ts | 189 ++++++++++++++++++ .../features/projects/ProjectSection.svelte | 10 + .../NewSessionModal.component.test.ts | 80 +++++++- .../features/sessions/NewSessionModal.svelte | 22 +- .../sessions/sessionModalHelpers.test.ts | 25 +++ .../features/sessions/sessionModalHelpers.ts | 22 ++ .../testStubs/TimelineContextMenuStub.svelte | 32 +++ 7 files changed, 377 insertions(+), 3 deletions(-) create mode 100644 apps/staged/src/lib/features/projects/ProjectSection.component.test.ts create mode 100644 apps/staged/src/lib/features/sessions/testStubs/TimelineContextMenuStub.svelte diff --git a/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts b/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts new file mode 100644 index 000000000..d47611005 --- /dev/null +++ b/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts @@ -0,0 +1,189 @@ +import { flushSync, mount, tick, unmount } from 'svelte'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Project, ProjectNote } from '../../types'; + +const { writer, loadDraftMock } = vi.hoisted(() => ({ + writer: { + scheduleSave: vi.fn(), + saveNow: vi.fn(async () => {}), + clearNow: vi.fn(async () => {}), + flush: vi.fn(async () => {}), + }, + loadDraftMock: vi.fn(async (_key: string): Promise => null), +})); + +// Heavy leaf components are stubbed out: the tests exercise the section's +// project-session draft wiring, not the timeline/branch/note internals. The +// real NewSessionModal is mounted (with its own leaves stubbed) so the open +// paths drive its draft load exactly as in the app. +vi.mock('../branches/BranchCard.svelte', () => import('../sessions/testStubs/EmptyStub.svelte')); +vi.mock('../timeline/TimelineRow.svelte', () => import('../sessions/testStubs/EmptyStub.svelte')); +vi.mock( + '../timeline/TimelineContextMenu.svelte', + () => import('../sessions/testStubs/TimelineContextMenuStub.svelte') +); +vi.mock('../notes/NoteModal.svelte', () => import('../sessions/testStubs/EmptyStub.svelte')); +vi.mock('./SuggestedRepos.svelte', () => import('../sessions/testStubs/EmptyStub.svelte')); +vi.mock('./ProjectStatusPicker.svelte', () => import('../sessions/testStubs/EmptyStub.svelte')); +vi.mock('$lib/components/ui/dialog', async () => { + const { default: Passthrough } = await import('../sessions/testStubs/PassthroughStub.svelte'); + const { default: Empty } = await import('../sessions/testStubs/EmptyStub.svelte'); + return { Root: Passthrough, Content: Passthrough, Title: Passthrough, ResizeHandle: Empty }; +}); +vi.mock( + '../sessions/HashtagInput.svelte', + () => import('../sessions/testStubs/PromptInputStub.svelte') +); +vi.mock( + '../sessions/ImageAttachment.svelte', + () => import('../sessions/testStubs/EmptyStub.svelte') +); +vi.mock('../agents/AcpConfigPicker.svelte', () => import('../sessions/testStubs/EmptyStub.svelte')); +vi.mock('../branches/dragDrop', () => ({ subscribeDragDrop: () => () => {} })); +vi.mock('../agents/agent.svelte', () => ({ agentState: { providers: [] } })); +vi.mock('../settings/preferences.svelte', () => ({ getPreferredAgent: () => undefined })); +vi.mock('../timeline/liveSessionHints', () => ({ + createLiveSessionHints: () => ({ syncRunningSessionIds: () => {}, destroy: () => {} }), +})); +vi.mock('../sessions/hashtagItems', () => ({ + buildProjectHashtagItems: async () => [], + projectNotesToHashtagItems: () => [], +})); +vi.mock('../../transport', async (importOriginal) => ({ + ...(await importOriginal()), + listenToEvent: () => () => {}, +})); +vi.mock('../../shared/draftCache', () => ({ + createDraftWriteScheduler: () => writer, + loadDraft: (key: string) => loadDraftMock(key), +})); +vi.mock('../../api/commands', async (importOriginal) => ({ + ...(await importOriginal()), + listProjectNotes: async () => [completedNote], +})); + +import ProjectSection from './ProjectSection.svelte'; + +const project = { + id: 'project-1', + name: 'Project', + githubRepo: null, + location: 'local', + subpath: null, + createdAt: 1, + updatedAt: 1, + statusOverride: null, +} as Project; + +const completedNote: ProjectNote = { + id: 'note-1', + projectId: project.id, + sessionId: 'session-1', + title: 'A finished note', + content: 'Note body', + createdAt: 1, + updatedAt: 1, + completedAt: 1, + suggestedNextCommitStep: null, + suggestedNextNoteStep: null, + sessionStatus: 'completed', + completionReason: null, +}; + +const REF = `#project-note:${completedNote.id}`; +const DRAFT_KEY = `draft:project-session:${project.id}`; + +function mountSection() { + const target = document.createElement('div'); + document.body.appendChild(target); + return mount(ProjectSection, { target, props: { project, branches: [] } }); +} + +/** Let the section's async note/draft loads and effect flushes run to completion. */ +async function settle() { + for (let i = 0; i < 8; i++) { + await tick(); + } + flushSync(); +} + +function promptInput() { + return document.querySelector('[data-testid="prompt-input"]'); +} + +function clickReferring() { + const btn = document.querySelector('[data-testid="new-session-referring"]'); + if (!btn) throw new Error('referring trigger not rendered'); + btn.click(); + flushSync(); +} + +function clickNewProjectNote() { + const btn = document.querySelector('[aria-label="New project note"]'); + if (!btn) throw new Error('new project note button not rendered'); + btn.click(); + flushSync(); +} + +function savedPrompts(): string[] { + return [...writer.saveNow.mock.calls, ...writer.scheduleSave.mock.calls].map( + ([, value]) => value as string + ); +} + +describe('ProjectSection referring open', () => { + beforeEach(() => { + document.body.innerHTML = ''; + writer.scheduleSave.mockClear(); + writer.saveNow.mockClear(); + writer.flush.mockClear(); + loadDraftMock.mockReset(); + loadDraftMock.mockImplementation(async () => null); + }); + + it('is superseded by a plain open racing its draft load instead of saving a ref the modal never shows', async () => { + // Regression: the referring open awaited its flush+read with no supersede + // guard. A plain "New project note" click landing in that window opened + // the modal synchronously without the ref; the referring handler then + // resumed and saved the ref-appended prompt to the store, which the + // already-open modal never displayed and whose next keystroke overwrote. + const section = mountSection(); + await settle(); + + // The referring open's store read stays in flight… + let resolveReferringLoad!: (value: string | null) => void; + loadDraftMock.mockImplementationOnce( + () => new Promise((resolve) => (resolveReferringLoad = resolve)) + ); + clickReferring(); + await vi.waitFor(() => expect(loadDraftMock).toHaveBeenCalledTimes(1)); + expect(promptInput()).toBeNull(); + + // …while the user clicks "New project note". The modal opens at once. + clickNewProjectNote(); + expect(promptInput()).not.toBeNull(); + await settle(); + + // The superseded referring open must be a no-op when its read lands: the + // open modal can't show the ref, so storing it would only be clobbered. + resolveReferringLoad('stored draft'); + await settle(); + expect(savedPrompts().some((prompt) => prompt.includes(REF))).toBe(false); + + unmount(section); + }); + + it('still builds the referring prompt from the stored draft when unraced', async () => { + const section = mountSection(); + await settle(); + + loadDraftMock.mockImplementation(async () => 'stored draft'); + clickReferring(); + await settle(); + + expect(promptInput()).not.toBeNull(); + expect(writer.saveNow).toHaveBeenCalledWith(DRAFT_KEY, `stored draft\n${REF}`); + + unmount(section); + }); +}); diff --git a/apps/staged/src/lib/features/projects/ProjectSection.svelte b/apps/staged/src/lib/features/projects/ProjectSection.svelte index 9b2044cb7..034e990e8 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.svelte +++ b/apps/staged/src/lib/features/projects/ProjectSection.svelte @@ -124,6 +124,8 @@ let draftProjectImageIds = $state([]); const projectDraftWriter = createDraftWriteScheduler(); let projectDraftLoadGeneration = 0; + /** Bumped by every open so a referring open can tell when a plain open overtook it. */ + let projectSessionOpenRequest = 0; let preferredProvider = $derived(getPreferredAgent(agentState.providers) ?? undefined); /** Session IDs for running project sessions (all produce notes). */ let activeSessionIds = $state>(new Set()); @@ -351,6 +353,8 @@ } function openProjectSessionModal() { + // Supersede any in-flight referring open so it can't clobber this one. + ++projectSessionOpenRequest; // Open synchronously seeded with the held draft — the modal fetches the // authoritative stored draft itself (via loadDraftForMode) so the click // isn't gated on store IPC (a flush plus a read; HTTP hops in web mode). @@ -542,7 +546,13 @@ } async function handleProjectNoteNewSessionReferring(ref: string) { + const request = ++projectSessionOpenRequest; await loadProjectSessionDraft(); + // A plain open landed during the read: the modal is already up without + // the ref and only reads initialPrompt once, so saving the ref-appended + // prompt would just be overwritten by its next keystroke. Drop the ref, + // as BranchCardSessionManager.openNewSessionReferring does. + if (request !== projectSessionOpenRequest) return; draftProjectPrompt = buildReferringPrompt(draftProjectPrompt, ref); showProjectSessionModal = true; void ensureHashtagItems(); diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts index fe76440bc..fb11281b4 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts @@ -6,7 +6,8 @@ import type { Branch, BranchSessionType } from '../../types'; // effect wiring, not the editor/dialog/picker internals. vi.mock('$lib/components/ui/dialog', async () => { const { default: Passthrough } = await import('./testStubs/PassthroughStub.svelte'); - return { Root: Passthrough, Content: Passthrough, Title: Passthrough }; + const { default: Empty } = await import('./testStubs/EmptyStub.svelte'); + return { Root: Passthrough, Content: Passthrough, Title: Passthrough, ResizeHandle: Empty }; }); vi.mock('./HashtagInput.svelte', () => import('./testStubs/PromptInputStub.svelte')); vi.mock('./ImageAttachment.svelte', () => import('./testStubs/EmptyStub.svelte')); @@ -148,6 +149,83 @@ describe('NewSessionModal draft autosave', () => { unmount(modal); }); + it('keeps text typed during a mode-switch load and appends it to the stored draft', async () => { + // Regression: the mode-switch load overwrote `prompt` wholesale when it + // landed, so keystrokes typed between ⌘1/⌘2/⌘3 and the load resolving + // were dropped from both modes — the save effect is gated during the + // switch, and the outgoing save had already been snapshotted. + let resolveNoteLoad!: (value: string | null) => void; + const onDraftChange = vi.fn(); + const modal = mountModal({ + onDraftChange, + loadDraftForMode: vi.fn((mode: BranchSessionType) => { + if (mode === 'note') return new Promise((r) => (resolveNoteLoad = r)); + return Promise.resolve(null); + }), + }); + flushSync(); + await settle(); + + typeInPrompt('commit text'); + await settle(); + + pressModeShortcut('1'); // commit → note; note's draft load stays pending + await settle(); + onDraftChange.mockClear(); + + // The outgoing commit text is still on screen; the user types after it. + typeInPrompt('commit text typed mid-switch'); + await settle(); + + resolveNoteLoad('note stored draft'); + await settle(); + + // The typed delta belongs to note mode: it lands after the stored note + // draft, the commit text does not come along, and the result is saved + // under note's key. + const noteSaves = onDraftChange.mock.calls + .filter(([draft]) => (draft as { mode: string }).mode === 'note') + .map(([draft]) => (draft as { prompt: string }).prompt); + expect(noteSaves.length).toBeGreaterThan(0); + const final = noteSaves[noteSaves.length - 1]; + expect(final).toContain('note stored draft'); + expect(final).toContain('typed mid-switch'); + expect(final).not.toContain('commit text'); + expect(final.indexOf('note stored draft')).toBeLessThan(final.indexOf('typed mid-switch')); + + unmount(modal); + }); + + it('keeps text typed during a mode-switch load when the new mode has no stored draft', async () => { + let resolveNoteLoad!: (value: string | null) => void; + const onDraftChange = vi.fn(); + const modal = mountModal({ + onDraftChange, + loadDraftForMode: vi.fn((mode: BranchSessionType) => { + if (mode === 'note') return new Promise((r) => (resolveNoteLoad = r)); + return Promise.resolve(null); + }), + }); + flushSync(); + await settle(); + + pressModeShortcut('1'); // commit → note; note's draft load stays pending + await settle(); + onDraftChange.mockClear(); + + typeInPrompt('typed mid-switch'); + await settle(); + + resolveNoteLoad(null); + await settle(); + + expect(onDraftChange).toHaveBeenCalledWith( + expect.objectContaining({ prompt: 'typed mid-switch', mode: 'note' }) + ); + + unmount(modal); + }); + it('files the draft under its owner mode when closed during a mode switch', async () => { const pendingNoteLoads: Array<(value: string | null) => void> = []; const onClose = vi.fn(); diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte index 2cd78c71c..b50da28b6 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte @@ -38,7 +38,12 @@ import ImageAttachment from './ImageAttachment.svelte'; import HashtagInput from './HashtagInput.svelte'; import { buildBranchHashtagItems } from './hashtagItems'; - import { foldSnippetsIntoPrompt, snippetLabel, type TextSnippet } from './sessionModalHelpers'; + import { + foldSnippetsIntoPrompt, + insertedText, + snippetLabel, + type TextSnippet, + } from './sessionModalHelpers'; import * as Dialog from '$lib/components/ui/dialog'; import { createDialogWidth, @@ -319,6 +324,12 @@ switchingMode = true; currentMode = newMode; + // From here until the load lands the save gate is closed and the mode on + // screen is already `newMode`, so keystrokes belong to the incoming mode. + // Snapshot now rather than before the outgoing save: anything typed during + // that await was still shown under the outgoing mode and reached its key + // through the save effect. + const promptWhenLoadStarted = prompt; if (loadDraftForMode) { let cachedDraft: string | null = null; @@ -331,7 +342,14 @@ const modePrefill = newMode === 'commit' ? commitPrefill : newMode === 'note' ? notePrefill : ''; - if (cachedDraft) { + // Text typed while the load was in flight was typed over the outgoing + // mode's text, so only the inserted part carries over — after the + // stored draft, or on its own (a prefill would bury it). + const typedDuringLoad = insertedText(promptWhenLoadStarted, prompt).trimStart(); + if (typedDuringLoad) { + prompt = cachedDraft ? `${cachedDraft.trimEnd()}\n${typedDuringLoad}` : typedDuringLoad; + focusPrompt(null); + } else if (cachedDraft) { prompt = cachedDraft; focusPrompt(null); } else if (modePrefill) { diff --git a/apps/staged/src/lib/features/sessions/sessionModalHelpers.test.ts b/apps/staged/src/lib/features/sessions/sessionModalHelpers.test.ts index e38240ec4..917f714a8 100644 --- a/apps/staged/src/lib/features/sessions/sessionModalHelpers.test.ts +++ b/apps/staged/src/lib/features/sessions/sessionModalHelpers.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { foldSnippetsIntoPrompt, formatToolDisplay, + insertedText, makePathsRelative, sessionEndMessage, snippetLabel, @@ -29,6 +30,30 @@ describe('foldSnippetsIntoPrompt', () => { }); }); +describe('insertedText', () => { + it('returns text appended to the end', () => { + expect(insertedText('commit text', 'commit text typed')).toBe(' typed'); + }); + + it('returns text inserted in the middle', () => { + expect(insertedText('abc', 'abXYc')).toBe('XY'); + }); + + it('returns everything when starting from an empty prompt', () => { + expect(insertedText('', 'typed')).toBe('typed'); + }); + + it('returns the replacement when a selection is overwritten', () => { + expect(insertedText('hello world', 'hello there world')).toBe('there '); + }); + + it("returns '' for unchanged text and pure deletions", () => { + expect(insertedText('same', 'same')).toBe(''); + expect(insertedText('commit text', 'commit')).toBe(''); + expect(insertedText('commit', '')).toBe(''); + }); +}); + describe('snippetLabel', () => { it('collapses whitespace to a single line', () => { expect(snippetLabel('first line\n second line')).toBe('first line second line'); diff --git a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts index 72ac3ddd0..09ff1b38b 100644 --- a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts +++ b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts @@ -28,6 +28,28 @@ export function foldSnippetsIntoPrompt(prompt: string, snippets: { text: string ); } +/** + * The text inserted into `before` to produce `after`: whatever lies between + * their longest common prefix and suffix. Pure deletions yield ''. + * + * Used to recover keystrokes typed while a mode-switch draft load was in + * flight: they were typed over the outgoing mode's text, so the prompt as a + * whole can't be kept, but the inserted part belongs to the incoming mode. + */ +export function insertedText(before: string, after: string): string { + const max = Math.min(before.length, after.length); + let prefix = 0; + while (prefix < max && before[prefix] === after[prefix]) prefix++; + let suffix = 0; + while ( + suffix < max - prefix && + before[before.length - 1 - suffix] === after[after.length - 1 - suffix] + ) { + suffix++; + } + return after.slice(prefix, after.length - suffix); +} + export interface ParsedToolCall { name: string; args: Record; diff --git a/apps/staged/src/lib/features/sessions/testStubs/TimelineContextMenuStub.svelte b/apps/staged/src/lib/features/sessions/testStubs/TimelineContextMenuStub.svelte new file mode 100644 index 000000000..29147601f --- /dev/null +++ b/apps/staged/src/lib/features/sessions/testStubs/TimelineContextMenuStub.svelte @@ -0,0 +1,32 @@ + + + +{#if ref && onNewSessionReferring} + +{/if} +{@render children?.()} From 91c692d173a667b79e29d32d7e2edb3ee3d63b4d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Sat, 17 Oct 2026 14:59:16 +1100 Subject: [PATCH 11/15] fix: keep the stored draft when typing during the initial modal load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the three comments from the code review of 6475fe97: - NewSessionModal: loadInitialDraft was the one load path that still discarded a stored draft when the user typed during the load window — `prompt !== promptWhenLoadStarted` skipped cachedDraft entirely, then `switchingMode = false` re-fired the save effect and overwrote the stored draft with just the typed text. BranchCard.openNewSession seeds '' whenever the held draft belongs to another mode, and in web mode the load is a flush plus an HTTP read, so a fast typist could permanently lose that mode's stored draft. Bring it in line with switchMode: when the seed is not the stored draft, place insertedText(seed, prompt) after the stored draft (same newline join), or apply the stored draft alone when nothing was typed; the reopened save effect then files the combined value under the mode's key. When the seed already is the stored draft (the normal case — the caller holds what the store holds) edits made during the load sit on top of it and the prompt is left alone, so nothing is appended twice. With no stored draft the typed text still wins over a prefill, as before. - sessionModalHelpers: document that insertedText is a prefix/suffix diff, not a general-purpose one — a replacement whose typed text shares boundary characters with the replaced text loses them ('note text' → 'new' yields 'ew', 'fix bug' → 'fix bag' yields 'a') — and that this is accepted for the single store round-trip it covers. - Component tests: the `$lib/components/ui/dialog` mock was a closed object copied into NewSessionModal and ProjectSection's suites, so any new Dialog.* subcomponent on main (as ResizeHandle did in #956) failed both wholesale with an opaque "not a component" error. Extract testStubs/dialogMock.ts, which keeps Root/Content/Title as passthroughs and returns the empty stub for any other PascalCase name via a Proxy (`has` included, since vitest checks `prop in module`; `then`, `default` and `__esModule` stay absent), and use it from both suites. Adds jsdom component tests: the initial-load regression (seeded '' with a stored draft, typed during the pending load → the save carries the stored draft followed by the typed text; verified to fail pre-fix), no stored draft → typed text still wins, and seed-equals-stored-draft → edits are kept without duplication. The old "typed text wins over the stored draft" test pinned the data-loss behaviour and is replaced by these. insertedText gets a unit test documenting the boundary-character limitation. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .../projects/ProjectSection.component.test.ts | 8 +- .../NewSessionModal.component.test.ts | 74 ++++++++++++++++--- .../features/sessions/NewSessionModal.svelte | 12 ++- .../sessions/sessionModalHelpers.test.ts | 7 ++ .../features/sessions/sessionModalHelpers.ts | 6 ++ .../features/sessions/testStubs/dialogMock.ts | 30 ++++++++ 6 files changed, 119 insertions(+), 18 deletions(-) create mode 100644 apps/staged/src/lib/features/sessions/testStubs/dialogMock.ts diff --git a/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts b/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts index d47611005..2328535bb 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts +++ b/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts @@ -25,11 +25,9 @@ vi.mock( vi.mock('../notes/NoteModal.svelte', () => import('../sessions/testStubs/EmptyStub.svelte')); vi.mock('./SuggestedRepos.svelte', () => import('../sessions/testStubs/EmptyStub.svelte')); vi.mock('./ProjectStatusPicker.svelte', () => import('../sessions/testStubs/EmptyStub.svelte')); -vi.mock('$lib/components/ui/dialog', async () => { - const { default: Passthrough } = await import('../sessions/testStubs/PassthroughStub.svelte'); - const { default: Empty } = await import('../sessions/testStubs/EmptyStub.svelte'); - return { Root: Passthrough, Content: Passthrough, Title: Passthrough, ResizeHandle: Empty }; -}); +vi.mock('$lib/components/ui/dialog', () => + import('../sessions/testStubs/dialogMock').then((m) => m.dialogModuleMock()) +); vi.mock( '../sessions/HashtagInput.svelte', () => import('../sessions/testStubs/PromptInputStub.svelte') diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts index fb11281b4..583683fe4 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts @@ -4,11 +4,9 @@ import type { Branch, BranchSessionType } from '../../types'; // Heavy leaf components are stubbed out: the tests exercise the modal's draft // effect wiring, not the editor/dialog/picker internals. -vi.mock('$lib/components/ui/dialog', async () => { - const { default: Passthrough } = await import('./testStubs/PassthroughStub.svelte'); - const { default: Empty } = await import('./testStubs/EmptyStub.svelte'); - return { Root: Passthrough, Content: Passthrough, Title: Passthrough, ResizeHandle: Empty }; -}); +vi.mock('$lib/components/ui/dialog', () => + import('./testStubs/dialogMock').then((m) => m.dialogModuleMock()) +); vi.mock('./HashtagInput.svelte', () => import('./testStubs/PromptInputStub.svelte')); vi.mock('./ImageAttachment.svelte', () => import('./testStubs/EmptyStub.svelte')); vi.mock('../agents/AcpConfigPicker.svelte', () => import('./testStubs/EmptyStub.svelte')); @@ -121,10 +119,15 @@ describe('NewSessionModal draft autosave', () => { unmount(modal); }); - it('lets text typed during the initial draft load win, and still saves it', async () => { + it('appends text typed during the initial load to the stored draft and saves both', async () => { + // Regression: the initial load skipped the stored draft entirely once the + // user had typed, then the reopened save effect overwrote the stored + // draft with just the typed text — the one load path that still lost a + // stored draft to a keystroke landing in the load window. let resolveLoad!: (value: string | null) => void; const onDraftChange = vi.fn(); const modal = mountModal({ + initialPrompt: '', onDraftChange, loadDraftForMode: vi.fn(() => new Promise((r) => (resolveLoad = r))), }); @@ -140,11 +143,62 @@ describe('NewSessionModal draft autosave', () => { resolveLoad('stored draft'); await settle(); - // …and once the load lands, the typed text is preserved over the stored - // draft and reaches onDraftChange so it still gets persisted. + // …and once the load lands, the stored draft comes first with the typed + // text after it, and that combined value is what gets persisted. + const prompts = onDraftChange.mock.calls.map(([draft]) => (draft as { prompt: string }).prompt); + expect(prompts.length).toBeGreaterThan(0); + expect(prompts[prompts.length - 1]).toBe('stored draft\ntyped during load'); + expect(prompts).not.toContain('typed during load'); + + unmount(modal); + }); + + it('lets text typed during the initial load win when there is no stored draft', async () => { + let resolveLoad!: (value: string | null) => void; + const onDraftChange = vi.fn(); + const modal = mountModal({ + initialPrompt: 'prefill', + prefilled: true, + onDraftChange, + loadDraftForMode: vi.fn(() => new Promise((r) => (resolveLoad = r))), + }); + flushSync(); + await settle(); + + typeInPrompt('typed over prefill'); + await settle(); + + resolveLoad(null); + await settle(); + expect(onDraftChange).toHaveBeenCalledWith( + expect.objectContaining({ prompt: 'typed over prefill', mode: 'commit' }) + ); + + unmount(modal); + }); + + it('keeps edits made during the initial load when the seed already is the stored draft', async () => { + // The caller seeds the modal with the draft it holds in memory, which is + // normally what the store holds too; edits typed during the load then + // sit on top of the stored draft and must not be re-appended to it. + let resolveLoad!: (value: string | null) => void; + const onDraftChange = vi.fn(); + const modal = mountModal({ + initialPrompt: 'stored draft', + onDraftChange, + loadDraftForMode: vi.fn(() => new Promise((r) => (resolveLoad = r))), + }); + flushSync(); + await settle(); + + typeInPrompt('stored draft plus'); + await settle(); + + resolveLoad('stored draft'); + await settle(); const prompts = onDraftChange.mock.calls.map(([draft]) => (draft as { prompt: string }).prompt); - expect(prompts).toContain('typed during load'); - expect(prompts).not.toContain('stored draft'); + expect(prompts.length).toBeGreaterThan(0); + expect(prompts.every((p) => p === 'stored draft plus')).toBe(true); unmount(modal); }); diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte index b50da28b6..8ea91d3b5 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte @@ -452,9 +452,15 @@ } // A mode switch superseded this load; it owns `prompt` and the save gate now. if (generation !== modeSwitchGeneration) return; - // Apply the stored draft only if the user hasn't typed in the meantime. - if (cachedDraft !== null && cachedDraft !== prompt && prompt === promptWhenLoadStarted) { - prompt = cachedDraft; + // When the seed already was the stored draft, any edits made during the + // load sit on top of it and the prompt is left alone. Otherwise (an empty + // or other-mode seed, or a prefill) the stored draft is authoritative, and + // text typed during the load belongs to this mode too: it goes after the + // stored draft, as in switchMode — the save effect then files the + // combined value under this mode's key once the gate reopens. + if (cachedDraft !== null && cachedDraft !== promptWhenLoadStarted) { + const typedDuringLoad = insertedText(promptWhenLoadStarted, prompt).trimStart(); + prompt = typedDuringLoad ? `${cachedDraft.trimEnd()}\n${typedDuringLoad}` : cachedDraft; focusPrompt(null); } switchingMode = false; diff --git a/apps/staged/src/lib/features/sessions/sessionModalHelpers.test.ts b/apps/staged/src/lib/features/sessions/sessionModalHelpers.test.ts index 917f714a8..a40e81b52 100644 --- a/apps/staged/src/lib/features/sessions/sessionModalHelpers.test.ts +++ b/apps/staged/src/lib/features/sessions/sessionModalHelpers.test.ts @@ -52,6 +52,13 @@ describe('insertedText', () => { expect(insertedText('commit text', 'commit')).toBe(''); expect(insertedText('commit', '')).toBe(''); }); + + it('drops boundary characters a replacement shares with the replaced text (known limitation)', () => { + // A prefix/suffix diff can't tell a shared boundary character apart from + // untouched text; accepted for the one-round-trip window it covers. + expect(insertedText('note text', 'new')).toBe('ew'); + expect(insertedText('fix bug', 'fix bag')).toBe('a'); + }); }); describe('snippetLabel', () => { diff --git a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts index 09ff1b38b..3e3c0171c 100644 --- a/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts +++ b/apps/staged/src/lib/features/sessions/sessionModalHelpers.ts @@ -35,6 +35,12 @@ export function foldSnippetsIntoPrompt(prompt: string, snippets: { text: string * Used to recover keystrokes typed while a mode-switch draft load was in * flight: they were typed over the outgoing mode's text, so the prompt as a * whole can't be kept, but the inserted part belongs to the incoming mode. + * + * This is a prefix/suffix diff, not a general-purpose one: a replacement + * whose typed text shares boundary characters with the text it replaced + * loses those characters (select-all over 'note text' then typing 'new' + * yields 'ew'; 'fix bug' → 'fix bag' yields 'a'). The window is a single + * store round-trip, so dropping a character or two there is accepted. */ export function insertedText(before: string, after: string): string { const max = Math.min(before.length, after.length); diff --git a/apps/staged/src/lib/features/sessions/testStubs/dialogMock.ts b/apps/staged/src/lib/features/sessions/testStubs/dialogMock.ts new file mode 100644 index 000000000..e3dea3f0a --- /dev/null +++ b/apps/staged/src/lib/features/sessions/testStubs/dialogMock.ts @@ -0,0 +1,30 @@ +/** + * Factory for `vi.mock('$lib/components/ui/dialog', dialogModuleMock)` in + * component tests. + * + * Root/Content/Title pass their children through so the dialog body renders; + * every other PascalCase export (ResizeHandle, Overlay, whatever lands on main + * next) resolves to the empty stub. A closed mock object would instead fail + * every suite that renders a dialog with an opaque "not a component" error + * the moment a new `Dialog.*` subcomponent is used upstream. + */ +export async function dialogModuleMock(): Promise> { + const { default: Passthrough } = await import('./PassthroughStub.svelte'); + const { default: Empty } = await import('./EmptyStub.svelte'); + const explicit: Record = { + Root: Passthrough, + Content: Passthrough, + Title: Passthrough, + }; + // Only component-shaped names fall back to the stub: `then`, `default` and + // `__esModule` must stay absent or module interop misreads the mock. + const isComponentName = (prop: PropertyKey): prop is string => + typeof prop === 'string' && /^[A-Z]/.test(prop); + return new Proxy(explicit, { + has: (target, prop) => prop in target || isComponentName(prop), + get: (target, prop) => { + if (prop in target) return target[prop as string]; + return isComponentName(prop) ? Empty : undefined; + }, + }); +} From 934ad35ec77642e5a19a2b503515d6c6016e8417 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Sat, 17 Oct 2026 15:07:44 +1100 Subject: [PATCH 12/15] fix: keep prefill replacements typed during the initial load whole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the three polish comments from the code review of f5a853a9: - NewSessionModal: a prefilled open selects all, so the natural gesture during the initial load window is typing a replacement over the prefill — and loadInitialDraft diffed that against the seed with insertedText, which drops boundary characters the replacement shares with the prefill ('Commit the changes' → 'add tests' yielded 'add test'). Unlike switchMode's rare partial-edit case this is the common prefilled-open gesture, so only diff while the seed is still contained in the live prompt; once it's gone the whole prompt was typed and carries over intact after the stored draft. switchMode is left as is: its seed is the outgoing mode's text with the caret at the end, so a partial delete-and-type there would otherwise carry leftover outgoing text into the new mode. - testStubs/dialogMock: dialog/index.ts also exports Root/Content/Title as Dialog/DialogContent/DialogTitle, which the Proxy resolved to the empty stub. Add the aliases to the explicit passthrough map so a component switching to the alias names fails loudly in tests instead of silently rendering nothing inside the dialog. - BranchCardSessionManager.openNewSessionReferring and ProjectSection.handleProjectNoteNewSessionReferring: document that the immediate save must stay synchronous with opening the modal, since loadInitialDraft only keeps the seeded prompt when the stored draft equals it — an await inserted between the two would let the load read the pre-ref draft and replace the prompt with it, minus the ref. Adds a jsdom component regression test: prefilled open with a stored draft, the prefill replaced during the pending load by text sharing its trailing character, resolve → the saved prompt is the stored draft followed by the full replacement (verified to fail pre-fix with the trailing 's' dropped). Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .../BranchCardSessionManager.svelte.ts | 4 +++ .../features/projects/ProjectSection.svelte | 4 +++ .../NewSessionModal.component.test.ts | 29 +++++++++++++++++++ .../features/sessions/NewSessionModal.svelte | 10 ++++++- .../features/sessions/testStubs/dialogMock.ts | 7 ++++- 5 files changed, 52 insertions(+), 2 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index 443a34dd9..cb32a3549 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -172,6 +172,10 @@ export default class BranchCardSessionManager { this.newSessionMode = 'commit'; this.showNewSession = true; } + // This save must stay synchronous with opening the modal: its + // loadInitialDraft only keeps the seeded prompt when the stored draft + // equals it, so an await between opening and saving would let the load + // read the pre-ref draft and replace the prompt with it, minus the ref. this.handleNewSessionDraftChange( { prompt: this.draftPrompt, mode: this.newSessionMode, imageIds: this.draftImageIds }, { immediate: true } diff --git a/apps/staged/src/lib/features/projects/ProjectSection.svelte b/apps/staged/src/lib/features/projects/ProjectSection.svelte index 034e990e8..5d93cecbc 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.svelte +++ b/apps/staged/src/lib/features/projects/ProjectSection.svelte @@ -556,6 +556,10 @@ draftProjectPrompt = buildReferringPrompt(draftProjectPrompt, ref); showProjectSessionModal = true; void ensureHashtagItems(); + // This save must stay synchronous with opening the modal: its + // loadInitialDraft only keeps the seeded prompt when the stored draft + // equals it, so an await between opening and saving would let the load + // read the pre-ref draft and replace the prompt with it, minus the ref. void projectDraftWriter.saveNow(projectDraftKey(project.id), draftProjectPrompt); } diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts index 583683fe4..ad7df8519 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.component.test.ts @@ -177,6 +177,35 @@ describe('NewSessionModal draft autosave', () => { unmount(modal); }); + it('keeps a replacement typed over the prefill during the initial load whole', async () => { + // A prefilled open selects all, so the natural gesture in the load window + // is typing a replacement over the prefill. Diffing that against the seed + // drops boundary characters the replacement shares with the prefill + // (here the trailing 's'); a prompt that no longer contains the seed is + // wholly typed and must carry over intact. + let resolveLoad!: (value: string | null) => void; + const onDraftChange = vi.fn(); + const modal = mountModal({ + initialPrompt: 'Commit the changes', + prefilled: true, + onDraftChange, + loadDraftForMode: vi.fn(() => new Promise((r) => (resolveLoad = r))), + }); + flushSync(); + await settle(); + + typeInPrompt('add tests'); + await settle(); + + resolveLoad('stored draft'); + await settle(); + const prompts = onDraftChange.mock.calls.map(([draft]) => (draft as { prompt: string }).prompt); + expect(prompts.length).toBeGreaterThan(0); + expect(prompts[prompts.length - 1]).toBe('stored draft\nadd tests'); + + unmount(modal); + }); + it('keeps edits made during the initial load when the seed already is the stored draft', async () => { // The caller seeds the modal with the draft it holds in memory, which is // normally what the store holds too; edits typed during the load then diff --git a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte index 8ea91d3b5..2c9ccde85 100644 --- a/apps/staged/src/lib/features/sessions/NewSessionModal.svelte +++ b/apps/staged/src/lib/features/sessions/NewSessionModal.svelte @@ -459,7 +459,15 @@ // stored draft, as in switchMode — the save effect then files the // combined value under this mode's key once the gate reopens. if (cachedDraft !== null && cachedDraft !== promptWhenLoadStarted) { - const typedDuringLoad = insertedText(promptWhenLoadStarted, prompt).trimStart(); + // A prefilled open selects all, so the usual gesture here is typing a + // replacement over the seed; once the seed is gone the whole prompt was + // typed, and diffing it would drop boundary characters it shares with + // the seed (see insertedText). Only diff while the seed is still there. + const typedDuringLoad = ( + prompt.includes(promptWhenLoadStarted) + ? insertedText(promptWhenLoadStarted, prompt) + : prompt + ).trimStart(); prompt = typedDuringLoad ? `${cachedDraft.trimEnd()}\n${typedDuringLoad}` : cachedDraft; focusPrompt(null); } diff --git a/apps/staged/src/lib/features/sessions/testStubs/dialogMock.ts b/apps/staged/src/lib/features/sessions/testStubs/dialogMock.ts index e3dea3f0a..f61a925f8 100644 --- a/apps/staged/src/lib/features/sessions/testStubs/dialogMock.ts +++ b/apps/staged/src/lib/features/sessions/testStubs/dialogMock.ts @@ -2,7 +2,8 @@ * Factory for `vi.mock('$lib/components/ui/dialog', dialogModuleMock)` in * component tests. * - * Root/Content/Title pass their children through so the dialog body renders; + * Root/Content/Title (and their Dialog* aliases) pass their children through + * so the dialog body renders; * every other PascalCase export (ResizeHandle, Overlay, whatever lands on main * next) resolves to the empty stub. A closed mock object would instead fail * every suite that renders a dialog with an opaque "not a component" error @@ -15,6 +16,10 @@ export async function dialogModuleMock(): Promise> { Root: Passthrough, Content: Passthrough, Title: Passthrough, + // dialog/index.ts also exports these under their aliased names. + Dialog: Passthrough, + DialogContent: Passthrough, + DialogTitle: Passthrough, }; // Only component-shaped names fall back to the stub: `then`, `default` and // `__esModule` must stay absent or module interop misreads the mock. From 4754aa4b9d67f41a96ff5a3ddd2c3561bd52abc1 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Sat, 17 Oct 2026 15:12:20 +1100 Subject: [PATCH 13/15] docs: correct the referring-open save ordering rationale Resolve the two comments from the code review of 9619b4a1: the ordering notes beside the immediate saveNow in BranchCardSessionManager.openNewSessionReferring and ProjectSection.handleProjectNoteNewSessionReferring claimed that an await between opening the modal and the save would let loadInitialDraft read the pre-ref draft and "replace the prompt with it, minus the ref". That isn't what the code does: with the seed X + '\n' + ref and the store holding X, `prompt.includes(X)` is true, insertedText recovers the ref, and the prompt is rebuilt byte-identical to buildReferringPrompt's output; with an empty held draft the stored value is null and the prompt is left alone. Reword both comments (same text at both sites) to the actual reason: keeping the save in the same tick as the open lets loadInitialDraft hit its equality fast path (stored draft === seed), so the ref is preserved exactly rather than the open depending on the insertedText prefix/suffix heuristic. Comment-only; no behaviour change. `pnpm check` and the two affected component suites pass. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .../features/branches/BranchCardSessionManager.svelte.ts | 9 +++++---- .../src/lib/features/projects/ProjectSection.svelte | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts index cb32a3549..a6a1d6d05 100644 --- a/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts +++ b/apps/staged/src/lib/features/branches/BranchCardSessionManager.svelte.ts @@ -172,10 +172,11 @@ export default class BranchCardSessionManager { this.newSessionMode = 'commit'; this.showNewSession = true; } - // This save must stay synchronous with opening the modal: its - // loadInitialDraft only keeps the seeded prompt when the stored draft - // equals it, so an await between opening and saving would let the load - // read the pre-ref draft and replace the prompt with it, minus the ref. + // Keep this save in the same tick as opening the modal so its + // loadInitialDraft hits the equality fast path (stored draft === seed) + // and the ref is preserved exactly. An await here wouldn't lose the ref, + // but the open would then depend on the insertedText prefix/suffix + // heuristic instead. this.handleNewSessionDraftChange( { prompt: this.draftPrompt, mode: this.newSessionMode, imageIds: this.draftImageIds }, { immediate: true } diff --git a/apps/staged/src/lib/features/projects/ProjectSection.svelte b/apps/staged/src/lib/features/projects/ProjectSection.svelte index 5d93cecbc..2328c36b8 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.svelte +++ b/apps/staged/src/lib/features/projects/ProjectSection.svelte @@ -556,10 +556,11 @@ draftProjectPrompt = buildReferringPrompt(draftProjectPrompt, ref); showProjectSessionModal = true; void ensureHashtagItems(); - // This save must stay synchronous with opening the modal: its - // loadInitialDraft only keeps the seeded prompt when the stored draft - // equals it, so an await between opening and saving would let the load - // read the pre-ref draft and replace the prompt with it, minus the ref. + // Keep this save in the same tick as opening the modal so its + // loadInitialDraft hits the equality fast path (stored draft === seed) + // and the ref is preserved exactly. An await here wouldn't lose the ref, + // but the open would then depend on the insertedText prefix/suffix + // heuristic instead. void projectDraftWriter.saveNow(projectDraftKey(project.id), draftProjectPrompt); } From b394b95fbdf76b831d44cce3470e5ed49daac695 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Sat, 31 Oct 2026 15:10:13 +1100 Subject: [PATCH 14/15] test: stub setPaused on the live session hints poller mock ProjectSection now pauses the poller when its layer is hidden, so the component test's poller stub needs the method or the effect throws. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .../lib/features/projects/ProjectSection.component.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts b/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts index 2328535bb..8e6ff991f 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts +++ b/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts @@ -41,7 +41,11 @@ vi.mock('../branches/dragDrop', () => ({ subscribeDragDrop: () => () => {} })); vi.mock('../agents/agent.svelte', () => ({ agentState: { providers: [] } })); vi.mock('../settings/preferences.svelte', () => ({ getPreferredAgent: () => undefined })); vi.mock('../timeline/liveSessionHints', () => ({ - createLiveSessionHints: () => ({ syncRunningSessionIds: () => {}, destroy: () => {} }), + createLiveSessionHints: () => ({ + syncRunningSessionIds: () => {}, + setPaused: () => {}, + destroy: () => {}, + }), })); vi.mock('../sessions/hashtagItems', () => ({ buildProjectHashtagItems: async () => [], From afc00fef262fa1e97c4d5b05181b5d42a60c92b6 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Sun, 1 Nov 2026 11:23:50 +1100 Subject: [PATCH 15/15] test: add the chat timestamp fields to the ProjectNote fixture Rebase onto main, which added lastChatMessageAt and chatReadAt to ProjectNote in #1005 (surface note revisions and chat replies on note rows). The completedNote fixture in ProjectSection.component.test.ts is the only hand-rolled ProjectNote literal that #1005 could not update, since the file exists only on this branch, so the PR's merge-commit typecheck failed while the branch tip stayed green. Set both fields to null; the new noteTimestamps/noteChatState helpers only read note fields, so no other test changes are needed. svelte-check and the ProjectSection component suite pass. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .../src/lib/features/projects/ProjectSection.component.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts b/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts index 8e6ff991f..201e50769 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts +++ b/apps/staged/src/lib/features/projects/ProjectSection.component.test.ts @@ -90,6 +90,8 @@ const completedNote: ProjectNote = { suggestedNextNoteStep: null, sessionStatus: 'completed', completionReason: null, + lastChatMessageAt: null, + chatReadAt: null, }; const REF = `#project-note:${completedNote.id}`;