From 8903cb7349ae5245d1160ddb3b317cc33215e7ce Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 17:54:18 +0200 Subject: [PATCH 01/18] fix(wizard): recover safely from conversation conflicts --- ui/src/api/wizard.ts | 56 +++++- ui/src/features/agent/AgentAssistantPanel.tsx | 70 ++++--- .../agent/wizardConversationPersistence.ts | 107 +++++++++++ .../features/agent/wizardConversationSync.ts | 57 +++++- ui/src/i18n/locales/en/wizard.json | 1 + ui/src/i18n/locales/es/wizard.json | 1 + .../wizardConversationPersistence.test.mjs | 176 ++++++++++++++++++ 7 files changed, 419 insertions(+), 49 deletions(-) create mode 100644 ui/src/features/agent/wizardConversationPersistence.ts create mode 100644 ui/tests/wizardConversationPersistence.test.mjs diff --git a/ui/src/api/wizard.ts b/ui/src/api/wizard.ts index 9056b212c..4096e7944 100644 --- a/ui/src/api/wizard.ts +++ b/ui/src/api/wizard.ts @@ -10,6 +10,52 @@ export interface WizardConversationPayload { confirmations?: unknown[] } +export interface WizardConversationErrorDetail { + code?: string + message?: string + expectedRevision?: number + currentRevision?: number + [key: string]: unknown +} + +/** + * Error from the durable Wizard conversation endpoint. + * + * Keeping the HTTP status and structured detail at this boundary prevents + * callers from treating validation/auth/server errors as revision conflicts. + */ +export class WizardConversationRequestError extends Error { + readonly status: number + readonly detail: unknown + readonly code?: string + + constructor(message: string, status: number, detail: unknown = null) { + super(message) + this.name = 'WizardConversationRequestError' + this.status = status + this.detail = detail + const structured = detail && typeof detail === 'object' && 'detail' in detail + ? (detail as { detail?: unknown }).detail + : detail + this.code = structured && typeof structured === 'object' && typeof (structured as WizardConversationErrorDetail).code === 'string' + ? (structured as WizardConversationErrorDetail).code + : undefined + } +} + +async function readWizardError(response: Response, fallback: string): Promise { + const body = await response.json().catch(() => null) + const detail = body && typeof body === 'object' && 'detail' in body + ? (body as { detail?: unknown }).detail + : body + const message = typeof detail === 'string' + ? detail + : detail && typeof detail === 'object' && typeof (detail as WizardConversationErrorDetail).message === 'string' + ? (detail as WizardConversationErrorDetail).message as string + : fallback + return new WizardConversationRequestError(message, response.status, body) +} + export interface WizardWorkflowCollectionPayload { version: 1 revision: number @@ -21,8 +67,7 @@ export async function fetchWizardConversation(workspace: string): Promise ({ detail: 'Could not load Wizard conversation' })) - throw new Error(error.detail || 'Could not load Wizard conversation') + throw await readWizardError(response, 'Could not load Wizard conversation') } return response.json() } @@ -37,12 +82,7 @@ export async function saveWizardConversation( body: JSON.stringify({ workspace, baseRevision: conversation.revision, conversation }), }) if (!response.ok) { - const error: unknown = await response.json().catch(() => null) - if (error && typeof error === 'object') { - const detail = (error as Record).detail - if (typeof detail === 'string') throw new Error(detail) - } - throw new Error('Could not save Wizard conversation') + throw await readWizardError(response, 'Could not save Wizard conversation') } return response.json() } diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 66ccbf8a4..5096ebc64 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -1,7 +1,7 @@ import { useEffect, useMemo, useRef, useState, type FormEvent, type KeyboardEvent as ReactKeyboardEvent } from 'react' import { createPortal } from 'react-dom' import { ArrowUp, Loader2, Maximize2, Minimize2, PanelLeftClose, Sparkles, Trash2 } from 'lucide-react' -import { fetchWizardConversation, generateLlmText, saveWizardConversation, subscribeCanonicalTaskEvents, type CanonicalTask } from '../../api/client' +import { fetchWizardConversation, generateLlmText, subscribeCanonicalTaskEvents, type CanonicalTask } from '../../api/client' import { AgentAvatar, type AgentVisualState } from './AgentAvatar' import { buildAgentTurnPrompt, HOCUSPOCUS_AGENT_SYSTEM_PROMPT, type AgentConversationEntry } from './agentKnowledge' import { @@ -15,11 +15,17 @@ import { type AgentActionResult, } from './agentActions' import { applyPollToCard, cardsFromResults, tabForExecutionTarget, type WizardExecutionCard } from './executionCards' -import { applyRemoteWizardConversation, WIZARD_WELCOME_TEXT } from './wizardConversationSync' +import { + applyRemoteWizardConversation, + isWizardConversationWriteCurrent, + shouldFollowWizardWorkspace, + WIZARD_WELCOME_TEXT, +} from './wizardConversationSync' import { AgentMarkdown } from './AgentMarkdown' import { defaultWizardWorkflowRuntime, type WizardWorkflowPendingInput, type WizardWorkflowRecord } from './wizardWorkflowRuntime' import { ensureRhythmic3dWorkflowRegistered } from './rhythmic3dWorkflow' import { defaultApplicationAdapters } from './applicationAdapters' +import { saveWizardConversationWithRecovery } from './wizardConversationPersistence' import i18n, { useUiTranslation } from '../../i18n' export { AgentAvatar, type AgentVisualState } from './AgentAvatar' @@ -150,6 +156,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const [busyMessage, setBusyMessage] = useState('') const [expanded, setExpanded] = useState(false) const [errorCardId, setErrorCardId] = useState(null) + const [conversationSaveError, setConversationSaveError] = useState(null) const [activeWorkflow, setActiveWorkflow] = useState(null) const [pendingInput, setPendingInput] = useState(null) const endRef = useRef(null) @@ -226,40 +233,30 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals return } const cards = messages.flatMap(message => message.cards || []) - void saveWizardConversation(conversationWorkspace, { + void saveWizardConversationWithRecovery(conversationWorkspace, { version: 1, revision: conversationRevisionRef.current, messages, executions: cards, }).then(saved => { - conversationRevisionRef.current = saved.revision - }).catch(async () => { - // A second tab may have advanced the CAS revision. Re-read and merge by - // message id; the resulting state triggers one save against the current - // backend revision. Local storage remains the fallback if this fails. - try { - const current = await fetchWizardConversation(conversationWorkspace) - if (!mountedRef.current || conversationWorkspaceRef.current !== conversationWorkspace) return - const choice = applyRemoteWizardConversation({ - localMessages: messagesRef.current, - localRevision: conversationRevisionRef.current, - remoteMessages: current.messages, - remoteRevision: current.revision || 0, - remoteExecutions: current.executions, - }) - conversationRevisionRef.current = choice.revision - skipNextConversationSaveRef.current = choice.source === 'remote' - setMessages([...choice.messages] as AgentMessage[]) - } catch { - // Local storage still holds the turn while the backend is unavailable. + if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return + conversationRevisionRef.current = saved.conversation.revision + setConversationSaveError(null) + if (saved.merged) { + skipNextConversationSaveRef.current = true + setMessages(saved.conversation.messages as AgentMessage[]) } + }).catch(error => { + if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return + setConversationSaveError(error instanceof Error ? error.message : String(error)) }) }, [conversationWorkspace, hydratedWorkspace, messages]) useEffect(() => { + if (workspace !== conversationWorkspace) return let cancelled = false - void fetchWizardConversation(workspace).then(payload => { - if (cancelled) return + void fetchWizardConversation(conversationWorkspace).then(payload => { + if (cancelled || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return const choice = applyRemoteWizardConversation({ localMessages: messagesRef.current, localRevision: conversationRevisionRef.current, @@ -273,26 +270,22 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals // merge remote-only messages. Use a fresh array so the persistence // effect retries the canonical save with that revision. setMessages([...choice.messages] as AgentMessage[]) - setHydratedWorkspace(workspace) + setHydratedWorkspace(conversationWorkspace) }).catch(() => { // Fall back to the local cache already loaded for this workspace. - if (!cancelled) setHydratedWorkspace(workspace) + if (!cancelled && isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) { + setHydratedWorkspace(conversationWorkspace) + } }) return () => { cancelled = true } - }, [workspace]) + }, [conversationWorkspace, workspace]) useEffect(() => { - if (workspace === conversationWorkspace) return + if (!shouldFollowWizardWorkspace({ activeWorkspace: workspace, conversationWorkspace, busy })) return conversationRevisionRef.current = 0 skipNextConversationSaveRef.current = false + setConversationSaveError(null) setHydratedWorkspace(null) - if (busy) { - // A Wizard action changed workspace while this turn was executing. - // Keep the visible turn alive and persist it in the destination so its - // real action result is not lost when the footer updates. - setConversationWorkspace(workspace) - return - } setMessages(readMessages(workspace)) setConversationWorkspace(workspace) setState('idle') @@ -552,6 +545,11 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals ))} + {conversationSaveError && ( +

+ {t('conversationSaveError', { message: conversationSaveError })} +

+ )} {busy && (
diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts new file mode 100644 index 000000000..286922bc7 --- /dev/null +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -0,0 +1,107 @@ +import { + fetchWizardConversation, + saveWizardConversation, + WizardConversationRequestError, + type WizardConversationPayload, +} from '../../api/wizard' +import { + mergeWizardMessages, + normalizeRemoteWizardMessages, +} from './wizardConversationSync' + +export interface WizardConversationTransport { + fetch: (workspace: string) => Promise + save: (workspace: string, conversation: WizardConversationPayload) => Promise +} + +export interface WizardConversationSaveResult { + conversation: WizardConversationPayload + merged: boolean +} + +const defaultTransport: WizardConversationTransport = { + fetch: fetchWizardConversation, + save: saveWizardConversation, +} + +function stableKey(value: unknown): string { + if (value === null || typeof value !== 'object') return JSON.stringify(value) ?? String(value) + if (Array.isArray(value)) return `[${value.map(stableKey).join(',')}]` + const record = value as Record + return `{${Object.keys(record).sort().map(key => `${JSON.stringify(key)}:${stableKey(record[key])}`).join(',')}}` +} + +function mergeUniqueValues(remote: unknown, local: unknown): unknown[] { + const merged: unknown[] = [] + const seen = new Set() + for (const value of [ + ...(Array.isArray(remote) ? remote : []), + ...(Array.isArray(local) ? local : []), + ]) { + const key = stableKey(value) + if (seen.has(key)) continue + seen.add(key) + merged.push(value) + } + return merged.slice(-80) +} + +/** + * Build the one payload used after a CAS conflict. + * + * The server snapshot is canonical. Existing ids remain in server order and + * local-only turn ids are appended once, so repeating the merge is harmless. + */ +export function mergeWizardConversationSnapshots( + local: WizardConversationPayload, + remote: WizardConversationPayload, +): WizardConversationPayload { + const remoteMessages = normalizeRemoteWizardMessages(remote.messages, remote.executions) + const localMessages = normalizeRemoteWizardMessages(local.messages, local.executions) + return { + version: 1, + revision: remote.revision, + messages: mergeWizardMessages(localMessages, remoteMessages), + executions: mergeUniqueValues(remote.executions, local.executions), + requestedActions: mergeUniqueValues(remote.requestedActions, local.requestedActions), + executedActions: mergeUniqueValues(remote.executedActions, local.executedActions), + confirmations: mergeUniqueValues(remote.confirmations, local.confirmations), + } +} + +export function isWizardConversationConflict(error: unknown): boolean { + if (error instanceof WizardConversationRequestError) return error.status === 409 + return Boolean( + error + && typeof error === 'object' + && (error as { status?: unknown }).status === 409, + ) +} + +/** + * Save a Wizard conversation, recovering one and only one CAS conflict. + * + * A validation, auth or server error is propagated immediately. If the + * retry conflicts again, that second error is propagated to the caller rather + * than starting an unbounded refetch/save loop. + */ +export async function saveWizardConversationWithRecovery( + workspace: string, + conversation: WizardConversationPayload, + transport: WizardConversationTransport = defaultTransport, +): Promise { + try { + return { + conversation: await transport.save(workspace, conversation), + merged: false, + } + } catch (error) { + if (!isWizardConversationConflict(error)) throw error + const remote = await transport.fetch(workspace) + const merged = mergeWizardConversationSnapshots(conversation, remote) + return { + conversation: await transport.save(workspace, merged), + merged: true, + } + } +} diff --git a/ui/src/features/agent/wizardConversationSync.ts b/ui/src/features/agent/wizardConversationSync.ts index 376f9b592..905d2a833 100644 --- a/ui/src/features/agent/wizardConversationSync.ts +++ b/ui/src/features/agent/wizardConversationSync.ts @@ -7,6 +7,10 @@ export interface WizardSyncMessage { createdAt: number language?: string cards?: unknown[] + executionKey?: string + jobLinks?: unknown[] + lastState?: string + error?: string } export interface WizardConversationChoice { @@ -36,6 +40,10 @@ export function normalizeRemoteWizardMessages( createdAt: typeof message.createdAt === 'number' ? message.createdAt : 0, ...(typeof message.language === 'string' && message.language ? { language: message.language } : {}), cards: Array.isArray(message.cards) && message.cards.length ? message.cards : undefined, + ...(typeof message.executionKey === 'string' && message.executionKey ? { executionKey: message.executionKey } : {}), + jobLinks: Array.isArray(message.jobLinks) && message.jobLinks.length ? message.jobLinks : undefined, + ...(typeof message.lastState === 'string' && message.lastState ? { lastState: message.lastState } : {}), + ...(typeof message.error === 'string' && message.error ? { error: message.error } : {}), }] }) if (!restored.length && Array.isArray(remoteExecutions) && remoteExecutions.length) { @@ -50,6 +58,32 @@ export function normalizeRemoteWizardMessages( return restored.slice(-40) } +/** + * Merge two snapshots without duplicating a message id. + * + * The remote snapshot is canonical and therefore wins for an id already + * persisted there. Local-only messages are appended in their existing order; + * this makes retries idempotent while keeping a user's turn together. + */ +export function mergeWizardMessages( + localMessages: WizardSyncMessage[], + remoteMessages: WizardSyncMessage[], +): WizardSyncMessage[] { + const merged: WizardSyncMessage[] = [] + const seen = new Set() + for (const message of remoteMessages) { + if (!message.id || seen.has(message.id)) continue + seen.add(message.id) + merged.push(message) + } + for (const message of localMessages) { + if (!message.id || seen.has(message.id)) continue + seen.add(message.id) + merged.push(message) + } + return merged.slice(-40) +} + export function isTransientWizardChat(messages: WizardSyncMessage[]): boolean { if (!messages.length) return true return !messages.some(message => ( @@ -78,15 +112,28 @@ export function applyRemoteWizardConversation(input: { const localHasExclusiveTurn = localMessages.some(message => !remoteIds.has(message.id)) && !isTransientWizardChat(localMessages) if (localHasExclusiveTurn) { - const localById = new Map(localMessages.map(message => [message.id, message])) - const merged = remoteMessages.map(message => localById.get(message.id) || message) - for (const message of localMessages) { - if (!remoteIds.has(message.id)) merged.push(message) + return { + source: 'local', + messages: mergeWizardMessages(localMessages, remoteMessages), + revision: Math.max(localRevision, remoteRevision), } - return { source: 'local', messages: merged.slice(-40), revision: Math.max(localRevision, remoteRevision) } } if (localRevision > remoteRevision) { return { source: 'local', messages: localMessages, revision: localRevision } } return { source: 'remote', messages: remoteMessages, revision: remoteRevision } } + +/** Follow the footer workspace only after the in-flight turn finishes. */ +export function shouldFollowWizardWorkspace(input: { + activeWorkspace: string + conversationWorkspace: string + busy: boolean +}): boolean { + return input.activeWorkspace !== input.conversationWorkspace && !input.busy +} + +/** Drop async conversation writes that finished after the owner changed. */ +export function isWizardConversationWriteCurrent(owner: string, current: string): boolean { + return Boolean(owner) && owner === current +} diff --git a/ui/src/i18n/locales/en/wizard.json b/ui/src/i18n/locales/en/wizard.json index 6d5d22a03..8e882ac62 100644 --- a/ui/src/i18n/locales/en/wizard.json +++ b/ui/src/i18n/locales/en/wizard.json @@ -21,6 +21,7 @@ "emptyReply": "My crystal ball did not return a usable answer.", "pendingError": "I cannot apply that answer to the blocked step yet: {{message}}", "llmError": "I could not query the LLM: {{message}}. Check Settings → Services and try again.", + "conversationSaveError": "I could not save this Wizard conversation: {{message}}. Your local copy is still available; retry after reloading.", "pendingChoice": "Choose one of these options to continue the spell: {{choices}}.", "pendingFields": "Answer the fields {{fields}}.", "needDecisionBody": "🪄 **I need a decision to continue the same spell.**\n\n{{reason}}", diff --git a/ui/src/i18n/locales/es/wizard.json b/ui/src/i18n/locales/es/wizard.json index cee3c0a90..c10ab838b 100644 --- a/ui/src/i18n/locales/es/wizard.json +++ b/ui/src/i18n/locales/es/wizard.json @@ -21,6 +21,7 @@ "emptyReply": "Mi bola de cristal no ha devuelto una respuesta utilizable.", "pendingError": "No puedo aplicar todavía esa respuesta al paso bloqueado: {{message}}", "llmError": "No he podido consultar el LLM: {{message}}. Comprueba Ajustes → Servicios y vuelve a intentarlo.", + "conversationSaveError": "No he podido guardar esta conversación del mago: {{message}}. Tu copia local sigue disponible; vuelve a cargar para reintentarlo.", "pendingChoice": "Elige una de estas opciones para continuar el hechizo: {{choices}}.", "pendingFields": "Responde los campos {{fields}}.", "needDecisionBody": "🪄 **Necesito una decisión para continuar el mismo hechizo.**\n\n{{reason}}", diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs new file mode 100644 index 000000000..707a7cde6 --- /dev/null +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -0,0 +1,176 @@ +import assert from 'node:assert/strict' +import test from 'node:test' + +const { + saveWizardConversationWithRecovery, + mergeWizardConversationSnapshots, +} = await import('../src/features/agent/wizardConversationPersistence.ts') +const { + WizardConversationRequestError, + fetchWizardConversation, +} = await import('../src/api/wizard.ts') +const { + isWizardConversationWriteCurrent, + shouldFollowWizardWorkspace, +} = await import('../src/features/agent/wizardConversationSync.ts') + +function clone(value) { + return JSON.parse(JSON.stringify(value)) +} + +function payload(revision, ids, prefix = '') { + return { + version: 1, + revision, + messages: ids.map((id, index) => ({ + id, + role: index % 2 ? 'assistant' : 'user', + text: `${prefix}${id}`, + createdAt: index + 1, + executionKey: `${prefix}${id}-execution`, + jobLinks: [{ taskId: `${prefix}${id}-task`, pipelineId: '' }], + })), + executions: [], + } +} + +function revisionConflict(expected, current) { + return new WizardConversationRequestError( + `expected ${expected}, current ${current}`, + 409, + { + code: 'wizard_conversation_revision_conflict', + expectedRevision: expected, + currentRevision: current, + }, + ) +} + +test('two writers preserve both Wizard turns exactly once after one CAS conflict', async () => { + let canonical = payload(0, []) + const calls = [] + const transport = { + async fetch(workspace) { + calls.push({ method: 'fetch', workspace }) + return clone(canonical) + }, + async save(workspace, conversation) { + calls.push({ method: 'save', workspace, conversation: clone(conversation) }) + if (conversation.revision !== canonical.revision) { + throw revisionConflict(conversation.revision, canonical.revision) + } + canonical = { ...clone(conversation), revision: canonical.revision + 1 } + return clone(canonical) + }, + } + + const first = await saveWizardConversationWithRecovery('workspace-a', payload(0, ['a-user', 'a-assistant'], 'A-'), transport) + const second = await saveWizardConversationWithRecovery('workspace-a', payload(0, ['b-user', 'b-assistant'], 'B-'), transport) + + assert.equal(first.merged, false) + assert.equal(second.merged, true) + assert.equal(canonical.revision, 2) + assert.deepEqual(canonical.messages.map(message => message.id), [ + 'a-user', 'a-assistant', 'b-user', 'b-assistant', + ]) + assert.equal(new Set(canonical.messages.map(message => message.id)).size, 4) + assert.deepEqual(canonical.messages.map(message => message.executionKey), [ + 'A-a-user-execution', 'A-a-assistant-execution', + 'B-b-user-execution', 'B-b-assistant-execution', + ]) + assert.deepEqual(calls.map(call => `${call.method}:${call.workspace}`), [ + 'save:workspace-a', 'save:workspace-a', 'fetch:workspace-a', 'save:workspace-a', + ]) + + const repeatedMerge = mergeWizardConversationSnapshots(second.conversation, canonical) + assert.deepEqual(repeatedMerge.messages.map(message => message.id), canonical.messages.map(message => message.id)) +}) + +test('second conflict is surfaced after one recovery retry and never loops', async () => { + let saves = 0 + let fetches = 0 + const transport = { + async fetch() { + fetches += 1 + return payload(4, ['remote-user', 'remote-assistant'], 'remote-') + }, + async save(_workspace, conversation) { + saves += 1 + throw revisionConflict(conversation.revision, 4) + }, + } + + await assert.rejects( + saveWizardConversationWithRecovery('workspace-a', payload(0, ['local-user', 'local-assistant'], 'local-'), transport), + error => error instanceof WizardConversationRequestError && error.status === 409, + ) + assert.equal(saves, 2) + assert.equal(fetches, 1) +}) + +test('non-recoverable 4xx is surfaced without a refetch or retry', async () => { + let saves = 0 + let fetches = 0 + const transport = { + async fetch() { + fetches += 1 + return payload(0, []) + }, + async save() { + saves += 1 + throw new WizardConversationRequestError( + 'conversation payload is invalid', + 400, + { detail: 'conversation payload is invalid' }, + ) + }, + } + + await assert.rejects( + saveWizardConversationWithRecovery('workspace-a', payload(0, ['local-user']), transport), + error => error instanceof WizardConversationRequestError && error.status === 400, + ) + assert.equal(saves, 1) + assert.equal(fetches, 0) +}) + +test('conversation HTTP errors retain nested API detail and status', async () => { + const originalFetch = globalThis.fetch + globalThis.fetch = async () => new Response(JSON.stringify({ + detail: { + code: 'wizard_conversation_revision_conflict', + message: 'expected 2, current 3', + expectedRevision: 2, + currentRevision: 3, + }, + }), { + status: 409, + headers: { 'content-type': 'application/json' }, + }) + try { + await assert.rejects( + fetchWizardConversation('workspace-a'), + error => error instanceof WizardConversationRequestError + && error.status === 409 + && error.code === 'wizard_conversation_revision_conflict' + && error.message === 'expected 2, current 3', + ) + } finally { + globalThis.fetch = originalFetch + } +}) + +test('workspace changes never rebind an in-flight conversation write', () => { + assert.equal(shouldFollowWizardWorkspace({ + activeWorkspace: 'workspace-b', + conversationWorkspace: 'workspace-a', + busy: true, + }), false) + assert.equal(shouldFollowWizardWorkspace({ + activeWorkspace: 'workspace-b', + conversationWorkspace: 'workspace-a', + busy: false, + }), true) + assert.equal(isWizardConversationWriteCurrent('workspace-a', 'workspace-b'), false) + assert.equal(isWizardConversationWriteCurrent('workspace-a', 'workspace-a'), true) +}) From b7f6c7f2f1f692f5471c246a9b7a558fe3c6da72 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 17:58:39 +0200 Subject: [PATCH 02/18] fix(wizard): guard newer conversation writes --- ui/src/features/agent/AgentAssistantPanel.tsx | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 5096ebc64..9ddabecdd 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -18,6 +18,8 @@ import { applyPollToCard, cardsFromResults, tabForExecutionTarget, type WizardEx import { applyRemoteWizardConversation, isWizardConversationWriteCurrent, + mergeWizardMessages, + normalizeRemoteWizardMessages, shouldFollowWizardWorkspace, WIZARD_WELCOME_TEXT, } from './wizardConversationSync' @@ -162,6 +164,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const endRef = useRef(null) const mountedRef = useRef(true) const conversationRevisionRef = useRef(0) + const conversationWriteTokenRef = useRef(0) const skipNextConversationSaveRef = useRef(false) const conversationWorkspaceRef = useRef(conversationWorkspace) conversationWorkspaceRef.current = conversationWorkspace @@ -233,21 +236,31 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals return } const cards = messages.flatMap(message => message.cards || []) + const writeToken = ++conversationWriteTokenRef.current void saveWizardConversationWithRecovery(conversationWorkspace, { version: 1, revision: conversationRevisionRef.current, messages, executions: cards, }).then(saved => { - if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return + if ( + !mountedRef.current + || writeToken !== conversationWriteTokenRef.current + || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace) + ) return conversationRevisionRef.current = saved.conversation.revision setConversationSaveError(null) if (saved.merged) { skipNextConversationSaveRef.current = true - setMessages(saved.conversation.messages as AgentMessage[]) + const canonicalMessages = normalizeRemoteWizardMessages(saved.conversation.messages, saved.conversation.executions) + setMessages(mergeWizardMessages(messagesRef.current, canonicalMessages) as AgentMessage[]) } }).catch(error => { - if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return + if ( + !mountedRef.current + || writeToken !== conversationWriteTokenRef.current + || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace) + ) return setConversationSaveError(error instanceof Error ? error.message : String(error)) }) }, [conversationWorkspace, hydratedWorkspace, messages]) @@ -282,6 +295,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals useEffect(() => { if (!shouldFollowWizardWorkspace({ activeWorkspace: workspace, conversationWorkspace, busy })) return + conversationWriteTokenRef.current += 1 conversationRevisionRef.current = 0 skipNextConversationSaveRef.current = false setConversationSaveError(null) From ffb17333b628bccc1bacbbafdcbac45a5f7091d3 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 18:08:00 +0200 Subject: [PATCH 03/18] fix(wizard): preserve busy workspace ownership --- ui/src/features/agent/AgentAssistantPanel.tsx | 19 ++++++++++++------- .../features/agent/wizardConversationSync.ts | 9 +++++++++ .../wizardConversationPersistence.test.mjs | 12 ++++++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 9ddabecdd..ce9ffeb68 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -17,6 +17,7 @@ import { import { applyPollToCard, cardsFromResults, tabForExecutionTarget, type WizardExecutionCard } from './executionCards' import { applyRemoteWizardConversation, + hasExclusiveWizardMessages, isWizardConversationWriteCurrent, mergeWizardMessages, normalizeRemoteWizardMessages, @@ -183,7 +184,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals let active = true ensureRhythmic3dWorkflowRegistered(defaultApplicationAdapters) const unsubscribe = defaultWizardWorkflowRuntime.subscribe(({ workflow, card }) => { - if (!active || workflow.workspace !== workspace) return + if (!active || !isWizardConversationWriteCurrent(conversationWorkspace, workflow.workspace)) return setActiveWorkflow(workflow) setPendingInput(workflow.state === 'awaiting_input' ? workflow.pendingInput : null) setMessages(current => { @@ -207,10 +208,10 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals }].slice(-40) }) }) - void defaultWizardWorkflowRuntime.open(workspace).catch(() => { + void defaultWizardWorkflowRuntime.open(conversationWorkspace).catch(() => { // Existing immediate actions remain available if workflow storage is offline. }) - const closeEvents = subscribeCanonicalTaskEvents(workspace, event => { + const closeEvents = subscribeCanonicalTaskEvents(conversationWorkspace, event => { void defaultWizardWorkflowRuntime.handleTaskEvent(event).catch(() => { // The checkpoint stays recoverable; a reconnect replays the same event. }) @@ -220,12 +221,12 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals unsubscribe() closeEvents() } - }, [workspace]) + }, [conversationWorkspace]) useEffect(() => { setActiveWorkflow(null) setPendingInput(null) - }, [workspace]) + }, [conversationWorkspace]) useEffect(() => { writeMessages(conversationWorkspace, messages) @@ -251,8 +252,11 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals conversationRevisionRef.current = saved.conversation.revision setConversationSaveError(null) if (saved.merged) { - skipNextConversationSaveRef.current = true const canonicalMessages = normalizeRemoteWizardMessages(saved.conversation.messages, saved.conversation.executions) + const hasExclusiveLocal = hasExclusiveWizardMessages(messagesRef.current, canonicalMessages) + if (!hasExclusiveLocal) { + skipNextConversationSaveRef.current = true + } setMessages(mergeWizardMessages(messagesRef.current, canonicalMessages) as AgentMessage[]) } }).catch(error => { @@ -306,6 +310,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals }, [busy, conversationWorkspace, workspace]) useEffect(() => { + if (workspace !== conversationWorkspace) return setMessages(current => current.map(message => { if (!message.cards?.length) return message let changed = false @@ -334,7 +339,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals }) return changed ? { ...message, cards } : message })) - }, [tasks]) + }, [conversationWorkspace, tasks, workspace]) useEffect(() => { const closeOnEscape = (event: KeyboardEvent) => { diff --git a/ui/src/features/agent/wizardConversationSync.ts b/ui/src/features/agent/wizardConversationSync.ts index 905d2a833..b52d81bc7 100644 --- a/ui/src/features/agent/wizardConversationSync.ts +++ b/ui/src/features/agent/wizardConversationSync.ts @@ -84,6 +84,15 @@ export function mergeWizardMessages( return merged.slice(-40) } +/** True when the visible client state still contains a turn absent from a saved snapshot. */ +export function hasExclusiveWizardMessages( + visibleMessages: WizardSyncMessage[], + savedMessages: WizardSyncMessage[], +): boolean { + const savedIds = new Set(savedMessages.map(message => message.id)) + return visibleMessages.some(message => Boolean(message.id) && !savedIds.has(message.id)) +} + export function isTransientWizardChat(messages: WizardSyncMessage[]): boolean { if (!messages.length) return true return !messages.some(message => ( diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 707a7cde6..d6d30ca80 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -11,6 +11,7 @@ const { } = await import('../src/api/wizard.ts') const { isWizardConversationWriteCurrent, + hasExclusiveWizardMessages, shouldFollowWizardWorkspace, } = await import('../src/features/agent/wizardConversationSync.ts') @@ -174,3 +175,14 @@ test('workspace changes never rebind an in-flight conversation write', () => { assert.equal(isWizardConversationWriteCurrent('workspace-a', 'workspace-b'), false) assert.equal(isWizardConversationWriteCurrent('workspace-a', 'workspace-a'), true) }) + +test('a conflict response cannot suppress persistence of a newer visible turn', () => { + const saved = payload(3, ['old-user', 'old-assistant']).messages + const withNewTurn = [ + ...saved, + { id: 'new-user', role: 'user', text: 'new request', createdAt: 3 }, + ] + + assert.equal(hasExclusiveWizardMessages(withNewTurn, saved), true) + assert.equal(hasExclusiveWizardMessages(saved, saved), false) +}) From f114e9c0e497764f3da57409efda0d59f8c8515b Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 18:19:03 +0200 Subject: [PATCH 04/18] fix(wizard): serialize conversation persistence --- ui/src/features/agent/AgentAssistantPanel.tsx | 61 +++++++++---------- .../agent/wizardConversationPersistence.ts | 11 ++++ .../wizardConversationPersistence.test.mjs | 38 ++++++++++++ 3 files changed, 78 insertions(+), 32 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index ce9ffeb68..bc3d1b55c 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -28,7 +28,7 @@ import { AgentMarkdown } from './AgentMarkdown' import { defaultWizardWorkflowRuntime, type WizardWorkflowPendingInput, type WizardWorkflowRecord } from './wizardWorkflowRuntime' import { ensureRhythmic3dWorkflowRegistered } from './rhythmic3dWorkflow' import { defaultApplicationAdapters } from './applicationAdapters' -import { saveWizardConversationWithRecovery } from './wizardConversationPersistence' +import { enqueueWizardConversationSave, saveWizardConversationWithRecovery } from './wizardConversationPersistence' import i18n, { useUiTranslation } from '../../i18n' export { AgentAvatar, type AgentVisualState } from './AgentAvatar' @@ -165,7 +165,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const endRef = useRef(null) const mountedRef = useRef(true) const conversationRevisionRef = useRef(0) - const conversationWriteTokenRef = useRef(0) + const conversationSaveChainRef = useRef>(Promise.resolve()) const skipNextConversationSaveRef = useRef(false) const conversationWorkspaceRef = useRef(conversationWorkspace) conversationWorkspaceRef.current = conversationWorkspace @@ -237,36 +237,34 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals return } const cards = messages.flatMap(message => message.cards || []) - const writeToken = ++conversationWriteTokenRef.current - void saveWizardConversationWithRecovery(conversationWorkspace, { - version: 1, - revision: conversationRevisionRef.current, - messages, - executions: cards, - }).then(saved => { - if ( - !mountedRef.current - || writeToken !== conversationWriteTokenRef.current - || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace) - ) return - conversationRevisionRef.current = saved.conversation.revision - setConversationSaveError(null) - if (saved.merged) { - const canonicalMessages = normalizeRemoteWizardMessages(saved.conversation.messages, saved.conversation.executions) - const hasExclusiveLocal = hasExclusiveWizardMessages(messagesRef.current, canonicalMessages) - if (!hasExclusiveLocal) { - skipNextConversationSaveRef.current = true + conversationSaveChainRef.current = enqueueWizardConversationSave( + conversationSaveChainRef.current, + async () => { + if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return + try { + const saved = await saveWizardConversationWithRecovery(conversationWorkspace, { + version: 1, + revision: conversationRevisionRef.current, + messages, + executions: cards, + }) + if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return + conversationRevisionRef.current = saved.conversation.revision + setConversationSaveError(null) + if (saved.merged) { + const canonicalMessages = normalizeRemoteWizardMessages(saved.conversation.messages, saved.conversation.executions) + const hasExclusiveLocal = hasExclusiveWizardMessages(messagesRef.current, canonicalMessages) + if (!hasExclusiveLocal) { + skipNextConversationSaveRef.current = true + } + setMessages(mergeWizardMessages(messagesRef.current, canonicalMessages) as AgentMessage[]) + } + } catch (error) { + if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return + setConversationSaveError(error instanceof Error ? error.message : String(error)) } - setMessages(mergeWizardMessages(messagesRef.current, canonicalMessages) as AgentMessage[]) - } - }).catch(error => { - if ( - !mountedRef.current - || writeToken !== conversationWriteTokenRef.current - || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace) - ) return - setConversationSaveError(error instanceof Error ? error.message : String(error)) - }) + }, + ) }, [conversationWorkspace, hydratedWorkspace, messages]) useEffect(() => { @@ -299,7 +297,6 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals useEffect(() => { if (!shouldFollowWizardWorkspace({ activeWorkspace: workspace, conversationWorkspace, busy })) return - conversationWriteTokenRef.current += 1 conversationRevisionRef.current = 0 skipNextConversationSaveRef.current = false setConversationSaveError(null) diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index 286922bc7..493cf16c5 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -105,3 +105,14 @@ export async function saveWizardConversationWithRecovery( } } } + +/** + * Serialize browser conversation writes so every save reads the revision + * confirmed by its predecessor. A rejected write does not poison the queue. + */ +export function enqueueWizardConversationSave( + previous: Promise, + write: () => Promise, +): Promise { + return previous.catch(() => undefined).then(write) +} diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index d6d30ca80..83507f9dd 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -4,6 +4,7 @@ import test from 'node:test' const { saveWizardConversationWithRecovery, mergeWizardConversationSnapshots, + enqueueWizardConversationSave, } = await import('../src/features/agent/wizardConversationPersistence.ts') const { WizardConversationRequestError, @@ -186,3 +187,40 @@ test('a conflict response cannot suppress persistence of a newer visible turn', assert.equal(hasExclusiveWizardMessages(withNewTurn, saved), true) assert.equal(hasExclusiveWizardMessages(saved, saved), false) }) + +test('conversation writes are serialized and a later write sees the confirmed revision', async () => { + let revision = 0 + let active = 0 + let maximumActive = 0 + const observed = [] + let releaseFirst + let markFirstStarted + const firstGate = new Promise(resolve => { releaseFirst = resolve }) + const firstStarted = new Promise(resolve => { markFirstStarted = resolve }) + + let chain = Promise.resolve() + chain = enqueueWizardConversationSave(chain, async () => { + active += 1 + maximumActive = Math.max(maximumActive, active) + observed.push(revision) + markFirstStarted() + await firstGate + revision = 1 + active -= 1 + }) + chain = enqueueWizardConversationSave(chain, async () => { + active += 1 + maximumActive = Math.max(maximumActive, active) + observed.push(revision) + revision = 2 + active -= 1 + }) + + await firstStarted + assert.deepEqual(observed, [0]) + releaseFirst() + await chain + assert.deepEqual(observed, [0, 1]) + assert.equal(maximumActive, 1) + assert.equal(revision, 2) +}) From cb33c729313be9341c4e0389533f772c0d479149 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 18:39:14 +0200 Subject: [PATCH 05/18] fix(wizard): preserve queued workspace saves --- ui/src/features/agent/AgentAssistantPanel.tsx | 31 +++++----- .../agent/wizardConversationPersistence.ts | 26 ++++++++ .../wizardConversationPersistence.test.mjs | 61 +++++++++++++++++++ 3 files changed, 104 insertions(+), 14 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index bc3d1b55c..54554b2be 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -1,7 +1,7 @@ import { useEffect, useMemo, useRef, useState, type FormEvent, type KeyboardEvent as ReactKeyboardEvent } from 'react' import { createPortal } from 'react-dom' import { ArrowUp, Loader2, Maximize2, Minimize2, PanelLeftClose, Sparkles, Trash2 } from 'lucide-react' -import { fetchWizardConversation, generateLlmText, subscribeCanonicalTaskEvents, type CanonicalTask } from '../../api/client' +import { fetchWizardConversation, generateLlmText, subscribeCanonicalTaskEvents, type CanonicalTask, type WizardConversationPayload } from '../../api/client' import { AgentAvatar, type AgentVisualState } from './AgentAvatar' import { buildAgentTurnPrompt, HOCUSPOCUS_AGENT_SYSTEM_PROMPT, type AgentConversationEntry } from './agentKnowledge' import { @@ -28,7 +28,7 @@ import { AgentMarkdown } from './AgentMarkdown' import { defaultWizardWorkflowRuntime, type WizardWorkflowPendingInput, type WizardWorkflowRecord } from './wizardWorkflowRuntime' import { ensureRhythmic3dWorkflowRegistered } from './rhythmic3dWorkflow' import { defaultApplicationAdapters } from './applicationAdapters' -import { enqueueWizardConversationSave, saveWizardConversationWithRecovery } from './wizardConversationPersistence' +import { enqueueWizardConversationSave, persistQueuedWizardConversation } from './wizardConversationPersistence' import i18n, { useUiTranslation } from '../../i18n' export { AgentAvatar, type AgentVisualState } from './AgentAvatar' @@ -164,7 +164,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const [pendingInput, setPendingInput] = useState(null) const endRef = useRef(null) const mountedRef = useRef(true) - const conversationRevisionRef = useRef(0) + const conversationSnapshotsRef = useRef>(new Map()) const conversationSaveChainRef = useRef>(Promise.resolve()) const skipNextConversationSaveRef = useRef(false) const conversationWorkspaceRef = useRef(conversationWorkspace) @@ -237,19 +237,22 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals return } const cards = messages.flatMap(message => message.cards || []) + const capturedConversation: WizardConversationPayload = { + version: 1, + revision: conversationSnapshotsRef.current.get(conversationWorkspace)?.revision || 0, + messages, + executions: cards, + } conversationSaveChainRef.current = enqueueWizardConversationSave( conversationSaveChainRef.current, async () => { - if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return try { - const saved = await saveWizardConversationWithRecovery(conversationWorkspace, { - version: 1, - revision: conversationRevisionRef.current, - messages, - executions: cards, - }) + const saved = await persistQueuedWizardConversation( + conversationWorkspace, + capturedConversation, + conversationSnapshotsRef.current, + ) if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return - conversationRevisionRef.current = saved.conversation.revision setConversationSaveError(null) if (saved.merged) { const canonicalMessages = normalizeRemoteWizardMessages(saved.conversation.messages, saved.conversation.executions) @@ -272,14 +275,15 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals let cancelled = false void fetchWizardConversation(conversationWorkspace).then(payload => { if (cancelled || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return + const knownSnapshot = conversationSnapshotsRef.current.get(conversationWorkspace) const choice = applyRemoteWizardConversation({ localMessages: messagesRef.current, - localRevision: conversationRevisionRef.current, + localRevision: knownSnapshot?.revision || 0, remoteMessages: payload.messages, remoteRevision: payload.revision || 0, remoteExecutions: payload.executions, }) - conversationRevisionRef.current = choice.revision + conversationSnapshotsRef.current.set(conversationWorkspace, payload) skipNextConversationSaveRef.current = choice.source === 'remote' // A local choice may still adopt the backend's newer CAS revision and // merge remote-only messages. Use a fresh array so the persistence @@ -297,7 +301,6 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals useEffect(() => { if (!shouldFollowWizardWorkspace({ activeWorkspace: workspace, conversationWorkspace, busy })) return - conversationRevisionRef.current = 0 skipNextConversationSaveRef.current = false setConversationSaveError(null) setHydratedWorkspace(null) diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index 493cf16c5..a0bebe891 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -19,6 +19,8 @@ export interface WizardConversationSaveResult { merged: boolean } +export type WizardConversationSnapshotStore = Map + const defaultTransport: WizardConversationTransport = { fetch: fetchWizardConversation, save: saveWizardConversation, @@ -106,6 +108,30 @@ export async function saveWizardConversationWithRecovery( } } +/** + * Persist one queued snapshot against the latest canonical state known for its + * workspace. Queued React effects intentionally capture their visible turn, + * but must not capture the revision/canonical payload: an earlier queued save + * may advance both before this write starts. + * + * The store is keyed by workspace so changing the visible workspace never + * rebinds or drops a write that was already accepted into the queue. + */ +export async function persistQueuedWizardConversation( + workspace: string, + captured: WizardConversationPayload, + snapshots: WizardConversationSnapshotStore, + transport: WizardConversationTransport = defaultTransport, +): Promise { + const canonical = snapshots.get(workspace) + const outgoing = canonical + ? mergeWizardConversationSnapshots(captured, canonical) + : captured + const saved = await saveWizardConversationWithRecovery(workspace, outgoing, transport) + snapshots.set(workspace, saved.conversation) + return saved +} + /** * Serialize browser conversation writes so every save reads the revision * confirmed by its predecessor. A rejected write does not poison the queue. diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 83507f9dd..8e9362ef0 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -5,6 +5,7 @@ const { saveWizardConversationWithRecovery, mergeWizardConversationSnapshots, enqueueWizardConversationSave, + persistQueuedWizardConversation, } = await import('../src/features/agent/wizardConversationPersistence.ts') const { WizardConversationRequestError, @@ -224,3 +225,63 @@ test('conversation writes are serialized and a later write sees the confirmed re assert.equal(maximumActive, 1) assert.equal(revision, 2) }) + +test('a stale queued snapshot merges the canonical turn saved by its predecessor', async () => { + let canonical = payload(0, []) + const snapshots = new Map() + const savedPayloads = [] + const transport = { + async fetch() { return clone(canonical) }, + async save(_workspace, conversation) { + assert.equal(conversation.revision, canonical.revision) + savedPayloads.push(clone(conversation)) + canonical = { ...clone(conversation), revision: canonical.revision + 1 } + return clone(canonical) + }, + } + + const firstVisible = payload(0, ['first-user', 'first-assistant']) + const staleSecondEffect = payload(0, ['second-user', 'second-assistant']) + let chain = Promise.resolve() + chain = enqueueWizardConversationSave(chain, () => ( + persistQueuedWizardConversation('workspace-a', firstVisible, snapshots, transport).then(() => undefined) + )) + chain = enqueueWizardConversationSave(chain, () => ( + persistQueuedWizardConversation('workspace-a', staleSecondEffect, snapshots, transport).then(() => undefined) + )) + await chain + + assert.deepEqual(savedPayloads[1].messages.map(message => message.id), [ + 'first-user', 'first-assistant', 'second-user', 'second-assistant', + ]) + assert.equal(savedPayloads[1].revision, 1) + assert.deepEqual(snapshots.get('workspace-a'), canonical) +}) + +test('a queued write persists to its captured workspace after the visible workspace changes', async () => { + const snapshots = new Map() + const savedWorkspaces = [] + let visibleWorkspace = 'workspace-a' + let releaseWrite + const gate = new Promise(resolve => { releaseWrite = resolve }) + const transport = { + async fetch() { return payload(0, []) }, + async save(workspace, conversation) { + await gate + savedWorkspaces.push(workspace) + return { ...clone(conversation), revision: 1 } + }, + } + + let chain = Promise.resolve() + chain = enqueueWizardConversationSave(chain, () => ( + persistQueuedWizardConversation('workspace-a', payload(0, ['a-user']), snapshots, transport).then(() => undefined) + )) + visibleWorkspace = 'workspace-b' + releaseWrite() + await chain + + assert.equal(visibleWorkspace, 'workspace-b') + assert.deepEqual(savedWorkspaces, ['workspace-a']) + assert.equal(snapshots.get('workspace-a').revision, 1) +}) From 525bd06fe55bf0e96aca413f48c7a1200865f88c Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 18:55:11 +0200 Subject: [PATCH 06/18] fix(wizard): merge queued edits in three ways --- ui/src/features/agent/AgentAssistantPanel.tsx | 8 +- .../agent/wizardConversationPersistence.ts | 81 +++++++++++++++++-- .../wizardConversationPersistence.test.mjs | 44 +++++++++- 3 files changed, 121 insertions(+), 12 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 54554b2be..e408ba573 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -243,13 +243,17 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals messages, executions: cards, } + const queuedWrite = { + workspace: conversationWorkspace, + captured: capturedConversation, + base: conversationSnapshotsRef.current.get(conversationWorkspace), + } conversationSaveChainRef.current = enqueueWizardConversationSave( conversationSaveChainRef.current, async () => { try { const saved = await persistQueuedWizardConversation( - conversationWorkspace, - capturedConversation, + queuedWrite, conversationSnapshotsRef.current, ) if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index a0bebe891..667e98c32 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -21,6 +21,12 @@ export interface WizardConversationSaveResult { export type WizardConversationSnapshotStore = Map +export interface QueuedWizardConversationWrite { + workspace: string + captured: WizardConversationPayload + base?: WizardConversationPayload +} + const defaultTransport: WizardConversationTransport = { fetch: fetchWizardConversation, save: saveWizardConversation, @@ -48,6 +54,68 @@ function mergeUniqueValues(remote: unknown, local: unknown): unknown[] { return merged.slice(-80) } +function valueIdentity(value: unknown): string { + if (value && typeof value === 'object') { + const record = value as Record + if (typeof record.id === 'string' && record.id) return `id:${record.id}` + if (typeof record.executionKey === 'string' && record.executionKey) return `execution:${record.executionKey}` + } + return `value:${stableKey(value)}` +} + +/** Apply local edits/deletes since base without discarding concurrent values. */ +function mergeQueuedValues(local: unknown, base: unknown, canonical: unknown): unknown[] { + const localValues = Array.isArray(local) ? local : [] + const baseValues = Array.isArray(base) ? base : [] + const canonicalValues = Array.isArray(canonical) ? canonical : [] + const localById = new Map(localValues.map(value => [valueIdentity(value), value])) + const baseById = new Map(baseValues.map(value => [valueIdentity(value), value])) + const merged: unknown[] = [] + const seen = new Set() + + canonicalValues.forEach(value => { + const id = valueIdentity(value) + const baseValue = baseById.get(id) + const localValue = localById.get(id) + if (baseById.has(id) && !localById.has(id)) return + if (localById.has(id) && (!baseById.has(id) || stableKey(localValue) !== stableKey(baseValue))) { + merged.push(localValue) + } else { + merged.push(value) + } + seen.add(id) + }) + localValues.forEach(value => { + const id = valueIdentity(value) + if (seen.has(id)) return + merged.push(value) + seen.add(id) + }) + return merged.slice(-80) +} + +export function mergeQueuedWizardConversationSnapshots( + local: WizardConversationPayload, + base: WizardConversationPayload | undefined, + canonical: WizardConversationPayload, +): WizardConversationPayload { + return { + version: 1, + revision: canonical.revision, + messages: mergeQueuedValues(local.messages, base?.messages, canonical.messages), + executions: mergeQueuedValues(local.executions, base?.executions, canonical.executions), + requestedActions: local.requestedActions === undefined + ? canonical.requestedActions + : mergeQueuedValues(local.requestedActions, base?.requestedActions, canonical.requestedActions), + executedActions: local.executedActions === undefined + ? canonical.executedActions + : mergeQueuedValues(local.executedActions, base?.executedActions, canonical.executedActions), + confirmations: local.confirmations === undefined + ? canonical.confirmations + : mergeQueuedValues(local.confirmations, base?.confirmations, canonical.confirmations), + } +} + /** * Build the one payload used after a CAS conflict. * @@ -118,17 +186,16 @@ export async function saveWizardConversationWithRecovery( * rebinds or drops a write that was already accepted into the queue. */ export async function persistQueuedWizardConversation( - workspace: string, - captured: WizardConversationPayload, + write: QueuedWizardConversationWrite, snapshots: WizardConversationSnapshotStore, transport: WizardConversationTransport = defaultTransport, ): Promise { - const canonical = snapshots.get(workspace) + const canonical = snapshots.get(write.workspace) const outgoing = canonical - ? mergeWizardConversationSnapshots(captured, canonical) - : captured - const saved = await saveWizardConversationWithRecovery(workspace, outgoing, transport) - snapshots.set(workspace, saved.conversation) + ? mergeQueuedWizardConversationSnapshots(write.captured, write.base, canonical) + : write.captured + const saved = await saveWizardConversationWithRecovery(write.workspace, outgoing, transport) + snapshots.set(write.workspace, saved.conversation) return saved } diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 8e9362ef0..181110f2a 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -4,6 +4,7 @@ import test from 'node:test' const { saveWizardConversationWithRecovery, mergeWizardConversationSnapshots, + mergeQueuedWizardConversationSnapshots, enqueueWizardConversationSave, persistQueuedWizardConversation, } = await import('../src/features/agent/wizardConversationPersistence.ts') @@ -244,10 +245,10 @@ test('a stale queued snapshot merges the canonical turn saved by its predecessor const staleSecondEffect = payload(0, ['second-user', 'second-assistant']) let chain = Promise.resolve() chain = enqueueWizardConversationSave(chain, () => ( - persistQueuedWizardConversation('workspace-a', firstVisible, snapshots, transport).then(() => undefined) + persistQueuedWizardConversation({ workspace: 'workspace-a', captured: firstVisible }, snapshots, transport).then(() => undefined) )) chain = enqueueWizardConversationSave(chain, () => ( - persistQueuedWizardConversation('workspace-a', staleSecondEffect, snapshots, transport).then(() => undefined) + persistQueuedWizardConversation({ workspace: 'workspace-a', captured: staleSecondEffect }, snapshots, transport).then(() => undefined) )) await chain @@ -275,7 +276,7 @@ test('a queued write persists to its captured workspace after the visible worksp let chain = Promise.resolve() chain = enqueueWizardConversationSave(chain, () => ( - persistQueuedWizardConversation('workspace-a', payload(0, ['a-user']), snapshots, transport).then(() => undefined) + persistQueuedWizardConversation({ workspace: 'workspace-a', captured: payload(0, ['a-user']) }, snapshots, transport).then(() => undefined) )) visibleWorkspace = 'workspace-b' releaseWrite() @@ -285,3 +286,40 @@ test('a queued write persists to its captured workspace after the visible worksp assert.deepEqual(savedWorkspaces, ['workspace-a']) assert.equal(snapshots.get('workspace-a').revision, 1) }) + +test('three-way queued merge applies local edits while retaining concurrent turns', () => { + const base = payload(3, ['shared-user']) + const local = clone(base) + local.messages[0].text = 'locally edited' + const canonical = payload(4, ['shared-user', 'remote-assistant']) + canonical.messages[0].text = 'old canonical text' + + const merged = mergeQueuedWizardConversationSnapshots(local, base, canonical) + + assert.equal(merged.revision, 4) + assert.equal(merged.messages.find(message => message.id === 'shared-user').text, 'locally edited') + assert.deepEqual(merged.messages.map(message => message.id), ['shared-user', 'remote-assistant']) +}) + +test('three-way queued merge keeps concurrent additions but honors a local clear', () => { + const base = payload(2, ['old-user', 'old-assistant']) + const local = payload(2, []) + const canonical = payload(3, ['old-user', 'old-assistant', 'concurrent-user']) + + const merged = mergeQueuedWizardConversationSnapshots(local, base, canonical) + + assert.deepEqual(merged.messages.map(message => message.id), ['concurrent-user']) +}) + +test('three-way queued merge persists an updated execution card by stable id', () => { + const base = payload(1, ['user']) + base.executions = [{ id: 'card-1', state: 'running' }] + const local = clone(base) + local.executions = [{ id: 'card-1', state: 'completed' }] + const canonical = clone(base) + canonical.revision = 2 + + const merged = mergeQueuedWizardConversationSnapshots(local, base, canonical) + + assert.deepEqual(merged.executions, [{ id: 'card-1', state: 'completed' }]) +}) From 949606f8e5dd25b1cc97989f9cdcd3042d6489b9 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 19:02:24 +0200 Subject: [PATCH 07/18] fix(wizard): preserve edits through retry conflicts --- .../agent/wizardConversationPersistence.ts | 7 +++-- .../wizardConversationPersistence.test.mjs | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index 667e98c32..f8874c998 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -159,6 +159,7 @@ export async function saveWizardConversationWithRecovery( workspace: string, conversation: WizardConversationPayload, transport: WizardConversationTransport = defaultTransport, + base?: WizardConversationPayload, ): Promise { try { return { @@ -168,7 +169,9 @@ export async function saveWizardConversationWithRecovery( } catch (error) { if (!isWizardConversationConflict(error)) throw error const remote = await transport.fetch(workspace) - const merged = mergeWizardConversationSnapshots(conversation, remote) + const merged = base + ? mergeQueuedWizardConversationSnapshots(conversation, base, remote) + : mergeWizardConversationSnapshots(conversation, remote) return { conversation: await transport.save(workspace, merged), merged: true, @@ -194,7 +197,7 @@ export async function persistQueuedWizardConversation( const outgoing = canonical ? mergeQueuedWizardConversationSnapshots(write.captured, write.base, canonical) : write.captured - const saved = await saveWizardConversationWithRecovery(write.workspace, outgoing, transport) + const saved = await saveWizardConversationWithRecovery(write.workspace, outgoing, transport, canonical) snapshots.set(write.workspace, saved.conversation) return saved } diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 181110f2a..72510bdeb 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -323,3 +323,33 @@ test('three-way queued merge persists an updated execution card by stable id', ( assert.deepEqual(merged.executions, [{ id: 'card-1', state: 'completed' }]) }) + +test('a CAS conflict during a queued edit keeps the edit and the concurrent turn', async () => { + const base = payload(4, ['shared-user']) + const snapshots = new Map([['workspace-a', clone(base)]]) + const local = clone(base) + local.messages[0].text = 'edited after hydration' + let canonical = payload(5, ['shared-user', 'remote-assistant']) + canonical.messages[0].text = base.messages[0].text + let saves = 0 + const transport = { + async fetch() { return clone(canonical) }, + async save(_workspace, conversation) { + saves += 1 + if (saves === 1) throw revisionConflict(conversation.revision, canonical.revision) + assert.equal(conversation.revision, 5) + canonical = { ...clone(conversation), revision: 6 } + return clone(canonical) + }, + } + + const saved = await persistQueuedWizardConversation({ + workspace: 'workspace-a', + captured: local, + base, + }, snapshots, transport) + + assert.equal(saved.merged, true) + assert.equal(saved.conversation.messages.find(message => message.id === 'shared-user').text, 'edited after hydration') + assert.deepEqual(saved.conversation.messages.map(message => message.id), ['shared-user', 'remote-assistant']) +}) From ae9be14fe20de8b4e5ebc5195c7ea9f4786a10d1 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 19:13:44 +0200 Subject: [PATCH 08/18] fix(wizard): reject stale hydration snapshots --- ui/src/features/agent/AgentAssistantPanel.tsx | 11 ++++++----- .../features/agent/wizardConversationPersistence.ts | 7 +++++++ ui/tests/wizardConversationPersistence.test.mjs | 10 ++++++++++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index e408ba573..5bd680d9f 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -28,7 +28,7 @@ import { AgentMarkdown } from './AgentMarkdown' import { defaultWizardWorkflowRuntime, type WizardWorkflowPendingInput, type WizardWorkflowRecord } from './wizardWorkflowRuntime' import { ensureRhythmic3dWorkflowRegistered } from './rhythmic3dWorkflow' import { defaultApplicationAdapters } from './applicationAdapters' -import { enqueueWizardConversationSave, persistQueuedWizardConversation } from './wizardConversationPersistence' +import { enqueueWizardConversationSave, newestWizardConversationSnapshot, persistQueuedWizardConversation } from './wizardConversationPersistence' import i18n, { useUiTranslation } from '../../i18n' export { AgentAvatar, type AgentVisualState } from './AgentAvatar' @@ -280,14 +280,15 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals void fetchWizardConversation(conversationWorkspace).then(payload => { if (cancelled || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return const knownSnapshot = conversationSnapshotsRef.current.get(conversationWorkspace) + const canonicalSnapshot = newestWizardConversationSnapshot(knownSnapshot, payload) const choice = applyRemoteWizardConversation({ localMessages: messagesRef.current, localRevision: knownSnapshot?.revision || 0, - remoteMessages: payload.messages, - remoteRevision: payload.revision || 0, - remoteExecutions: payload.executions, + remoteMessages: canonicalSnapshot.messages, + remoteRevision: canonicalSnapshot.revision || 0, + remoteExecutions: canonicalSnapshot.executions, }) - conversationSnapshotsRef.current.set(conversationWorkspace, payload) + conversationSnapshotsRef.current.set(conversationWorkspace, canonicalSnapshot) skipNextConversationSaveRef.current = choice.source === 'remote' // A local choice may still adopt the backend's newer CAS revision and // merge remote-only messages. Use a fresh array so the persistence diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index f8874c998..0516daef5 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -27,6 +27,13 @@ export interface QueuedWizardConversationWrite { base?: WizardConversationPayload } +export function newestWizardConversationSnapshot( + known: WizardConversationPayload | undefined, + incoming: WizardConversationPayload, +): WizardConversationPayload { + return known && known.revision >= incoming.revision ? known : incoming +} + const defaultTransport: WizardConversationTransport = { fetch: fetchWizardConversation, save: saveWizardConversation, diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 72510bdeb..5673cec92 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -7,6 +7,7 @@ const { mergeQueuedWizardConversationSnapshots, enqueueWizardConversationSave, persistQueuedWizardConversation, + newestWizardConversationSnapshot, } = await import('../src/features/agent/wizardConversationPersistence.ts') const { WizardConversationRequestError, @@ -353,3 +354,12 @@ test('a CAS conflict during a queued edit keeps the edit and the concurrent turn assert.equal(saved.conversation.messages.find(message => message.id === 'shared-user').text, 'edited after hydration') assert.deepEqual(saved.conversation.messages.map(message => message.id), ['shared-user', 'remote-assistant']) }) + +test('a late hydration fetch cannot replace a newer confirmed snapshot', () => { + const confirmed = payload(8, ['confirmed-user', 'confirmed-assistant']) + const staleFetch = payload(7, ['stale-user']) + + assert.equal(newestWizardConversationSnapshot(confirmed, staleFetch), confirmed) + assert.equal(newestWizardConversationSnapshot(undefined, staleFetch), staleFetch) + assert.equal(newestWizardConversationSnapshot(staleFetch, confirmed), confirmed) +}) From c3e442fb01408bab99ed40742b097da40f2328d0 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 19:23:36 +0200 Subject: [PATCH 09/18] fix(wizard): ignore stale hydration payloads --- ui/src/features/agent/AgentAssistantPanel.tsx | 14 ++++++++--- .../agent/wizardConversationPersistence.ts | 9 +++++--- .../wizardConversationPersistence.test.mjs | 23 +++++++++++++++---- 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 5bd680d9f..7304b6d4c 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -28,7 +28,7 @@ import { AgentMarkdown } from './AgentMarkdown' import { defaultWizardWorkflowRuntime, type WizardWorkflowPendingInput, type WizardWorkflowRecord } from './wizardWorkflowRuntime' import { ensureRhythmic3dWorkflowRegistered } from './rhythmic3dWorkflow' import { defaultApplicationAdapters } from './applicationAdapters' -import { enqueueWizardConversationSave, newestWizardConversationSnapshot, persistQueuedWizardConversation } from './wizardConversationPersistence' +import { enqueueWizardConversationSave, persistQueuedWizardConversation, resolveWizardConversationHydration } from './wizardConversationPersistence' import i18n, { useUiTranslation } from '../../i18n' export { AgentAvatar, type AgentVisualState } from './AgentAvatar' @@ -280,7 +280,16 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals void fetchWizardConversation(conversationWorkspace).then(payload => { if (cancelled || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return const knownSnapshot = conversationSnapshotsRef.current.get(conversationWorkspace) - const canonicalSnapshot = newestWizardConversationSnapshot(knownSnapshot, payload) + const hydration = resolveWizardConversationHydration(knownSnapshot, payload) + conversationSnapshotsRef.current.set(conversationWorkspace, hydration.snapshot) + if (!hydration.applyToVisibleState) { + // The response raced a confirmed save (or contains no newer revision). + // Keep visible local edits intact and let the normal persistence effect + // write them against the latest known canonical revision. + setHydratedWorkspace(conversationWorkspace) + return + } + const canonicalSnapshot = hydration.snapshot const choice = applyRemoteWizardConversation({ localMessages: messagesRef.current, localRevision: knownSnapshot?.revision || 0, @@ -288,7 +297,6 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals remoteRevision: canonicalSnapshot.revision || 0, remoteExecutions: canonicalSnapshot.executions, }) - conversationSnapshotsRef.current.set(conversationWorkspace, canonicalSnapshot) skipNextConversationSaveRef.current = choice.source === 'remote' // A local choice may still adopt the backend's newer CAS revision and // merge remote-only messages. Use a fresh array so the persistence diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index 0516daef5..4e0d3e36c 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -27,11 +27,14 @@ export interface QueuedWizardConversationWrite { base?: WizardConversationPayload } -export function newestWizardConversationSnapshot( +export function resolveWizardConversationHydration( known: WizardConversationPayload | undefined, incoming: WizardConversationPayload, -): WizardConversationPayload { - return known && known.revision >= incoming.revision ? known : incoming +): { snapshot: WizardConversationPayload; applyToVisibleState: boolean } { + if (known && known.revision >= incoming.revision) { + return { snapshot: known, applyToVisibleState: false } + } + return { snapshot: incoming, applyToVisibleState: true } } const defaultTransport: WizardConversationTransport = { diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 5673cec92..0502e773b 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -7,7 +7,7 @@ const { mergeQueuedWizardConversationSnapshots, enqueueWizardConversationSave, persistQueuedWizardConversation, - newestWizardConversationSnapshot, + resolveWizardConversationHydration, } = await import('../src/features/agent/wizardConversationPersistence.ts') const { WizardConversationRequestError, @@ -355,11 +355,24 @@ test('a CAS conflict during a queued edit keeps the edit and the concurrent turn assert.deepEqual(saved.conversation.messages.map(message => message.id), ['shared-user', 'remote-assistant']) }) -test('a late hydration fetch cannot replace a newer confirmed snapshot', () => { +test('a late hydration fetch cannot replace a newer confirmed snapshot or visible edits', () => { const confirmed = payload(8, ['confirmed-user', 'confirmed-assistant']) const staleFetch = payload(7, ['stale-user']) - assert.equal(newestWizardConversationSnapshot(confirmed, staleFetch), confirmed) - assert.equal(newestWizardConversationSnapshot(undefined, staleFetch), staleFetch) - assert.equal(newestWizardConversationSnapshot(staleFetch, confirmed), confirmed) + assert.deepEqual(resolveWizardConversationHydration(confirmed, staleFetch), { + snapshot: confirmed, + applyToVisibleState: false, + }) + assert.deepEqual(resolveWizardConversationHydration(confirmed, clone(confirmed)), { + snapshot: confirmed, + applyToVisibleState: false, + }) + assert.deepEqual(resolveWizardConversationHydration(undefined, staleFetch), { + snapshot: staleFetch, + applyToVisibleState: true, + }) + assert.deepEqual(resolveWizardConversationHydration(staleFetch, confirmed), { + snapshot: confirmed, + applyToVisibleState: true, + }) }) From 6f8262395f1bc5c0e95e36aa2968d69172c2268e Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 19:31:40 +0200 Subject: [PATCH 10/18] fix(wizard): rebase raced hydration state --- ui/src/features/agent/AgentAssistantPanel.tsx | 16 ++++++++--- .../agent/wizardConversationPersistence.ts | 25 +++++++++++++++++ .../wizardConversationPersistence.test.mjs | 28 +++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 7304b6d4c..0d1f7dfe9 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -28,7 +28,7 @@ import { AgentMarkdown } from './AgentMarkdown' import { defaultWizardWorkflowRuntime, type WizardWorkflowPendingInput, type WizardWorkflowRecord } from './wizardWorkflowRuntime' import { ensureRhythmic3dWorkflowRegistered } from './rhythmic3dWorkflow' import { defaultApplicationAdapters } from './applicationAdapters' -import { enqueueWizardConversationSave, persistQueuedWizardConversation, resolveWizardConversationHydration } from './wizardConversationPersistence' +import { enqueueWizardConversationSave, persistQueuedWizardConversation, rebaseStaleWizardConversationHydration, resolveWizardConversationHydration } from './wizardConversationPersistence' import i18n, { useUiTranslation } from '../../i18n' export { AgentAvatar, type AgentVisualState } from './AgentAvatar' @@ -283,9 +283,17 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const hydration = resolveWizardConversationHydration(knownSnapshot, payload) conversationSnapshotsRef.current.set(conversationWorkspace, hydration.snapshot) if (!hydration.applyToVisibleState) { - // The response raced a confirmed save (or contains no newer revision). - // Keep visible local edits intact and let the normal persistence effect - // write them against the latest known canonical revision. + const visibleMessages = messagesRef.current + const rebased = rebaseStaleWizardConversationHydration({ + ...payload, + messages: visibleMessages, + executions: visibleMessages.flatMap(message => message.cards || []), + }, payload, hydration.snapshot) + skipNextConversationSaveRef.current = !rebased.needsPersist + setMessages(normalizeRemoteWizardMessages( + rebased.conversation.messages, + rebased.conversation.executions, + ) as AgentMessage[]) setHydratedWorkspace(conversationWorkspace) return } diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index 4e0d3e36c..2bd0658b9 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -37,6 +37,31 @@ export function resolveWizardConversationHydration( return { snapshot: incoming, applyToVisibleState: true } } +/** + * Rebase browser-visible edits over a hydration response that lost a race to + * a confirmed save. The stale response is the three-way base, so confirmed + * turns added after it are retained while actual local edits and clears still + * win for values that existed in that base. + */ +export function rebaseStaleWizardConversationHydration( + visible: WizardConversationPayload, + stale: WizardConversationPayload, + confirmed: WizardConversationPayload, +): { conversation: WizardConversationPayload; needsPersist: boolean } { + const conversation = mergeQueuedWizardConversationSnapshots(visible, stale, confirmed) + const semanticContent = (value: WizardConversationPayload) => ({ + messages: value.messages, + executions: value.executions, + requestedActions: value.requestedActions ?? [], + executedActions: value.executedActions ?? [], + confirmations: value.confirmations ?? [], + }) + return { + conversation, + needsPersist: stableKey(semanticContent(conversation)) !== stableKey(semanticContent(confirmed)), + } +} + const defaultTransport: WizardConversationTransport = { fetch: fetchWizardConversation, save: saveWizardConversation, diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 0502e773b..0dc76732f 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -7,6 +7,7 @@ const { mergeQueuedWizardConversationSnapshots, enqueueWizardConversationSave, persistQueuedWizardConversation, + rebaseStaleWizardConversationHydration, resolveWizardConversationHydration, } = await import('../src/features/agent/wizardConversationPersistence.ts') const { @@ -376,3 +377,30 @@ test('a late hydration fetch cannot replace a newer confirmed snapshot or visibl applyToVisibleState: true, }) }) + +test('stale hydration rebases visible edits without dropping confirmed turns', () => { + const stale = payload(7, ['shared-user']) + const confirmed = payload(8, ['shared-user', 'confirmed-assistant']) + const visible = clone(stale) + visible.messages[0].text = 'edited while hydration was in flight' + + const rebased = rebaseStaleWizardConversationHydration(visible, stale, confirmed) + + assert.equal(rebased.needsPersist, true) + assert.equal(rebased.conversation.revision, 8) + assert.equal(rebased.conversation.messages[0].text, 'edited while hydration was in flight') + assert.deepEqual(rebased.conversation.messages.map(message => message.id), [ + 'shared-user', + 'confirmed-assistant', + ]) +}) + +test('stale hydration restores confirmed-only turns without scheduling a redundant save', () => { + const stale = payload(7, ['shared-user']) + const confirmed = payload(8, ['shared-user', 'confirmed-assistant']) + + const rebased = rebaseStaleWizardConversationHydration(clone(stale), stale, confirmed) + + assert.equal(rebased.needsPersist, false) + assert.deepEqual(rebased.conversation.messages, confirmed.messages) +}) From 9fea9080c063557a45cf1bbfdb58b0d767205625 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 19:48:25 +0200 Subject: [PATCH 11/18] fix(wizard): distinguish stale cache from explicit clear --- ui/src/features/agent/AgentAssistantPanel.tsx | 10 ++++- .../agent/wizardConversationPersistence.ts | 41 ++++++++++++++++--- .../wizardConversationPersistence.test.mjs | 27 ++++++++++++ 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 0d1f7dfe9..a5cfcad1d 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -167,6 +167,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const conversationSnapshotsRef = useRef>(new Map()) const conversationSaveChainRef = useRef>(Promise.resolve()) const skipNextConversationSaveRef = useRef(false) + const explicitConversationClearRef = useRef(false) const conversationWorkspaceRef = useRef(conversationWorkspace) conversationWorkspaceRef.current = conversationWorkspace const messagesRef = useRef(messages) @@ -282,13 +283,16 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const knownSnapshot = conversationSnapshotsRef.current.get(conversationWorkspace) const hydration = resolveWizardConversationHydration(knownSnapshot, payload) conversationSnapshotsRef.current.set(conversationWorkspace, hydration.snapshot) - if (!hydration.applyToVisibleState) { + if (!hydration.applyToVisibleState || explicitConversationClearRef.current) { const visibleMessages = messagesRef.current const rebased = rebaseStaleWizardConversationHydration({ ...payload, messages: visibleMessages, executions: visibleMessages.flatMap(message => message.cards || []), - }, payload, hydration.snapshot) + }, payload, hydration.snapshot, { + honorLocalDeletes: explicitConversationClearRef.current, + }) + explicitConversationClearRef.current = false skipNextConversationSaveRef.current = !rebased.needsPersist setMessages(normalizeRemoteWizardMessages( rebased.conversation.messages, @@ -323,6 +327,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals useEffect(() => { if (!shouldFollowWizardWorkspace({ activeWorkspace: workspace, conversationWorkspace, busy })) return skipNextConversationSaveRef.current = false + explicitConversationClearRef.current = false setConversationSaveError(null) setHydratedWorkspace(null) setMessages(readMessages(workspace)) @@ -378,6 +383,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const clearConversation = () => { const next = [welcomeMessage()] + explicitConversationClearRef.current = true setMessages(next) setState('idle') } diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index 2bd0658b9..634b5d673 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -40,15 +40,41 @@ export function resolveWizardConversationHydration( /** * Rebase browser-visible edits over a hydration response that lost a race to * a confirmed save. The stale response is the three-way base, so confirmed - * turns added after it are retained while actual local edits and clears still - * win for values that existed in that base. + * turns added after it are retained while local edits still win. Missing + * browser-cache values are not treated as deletes unless the caller records + * an explicit user clear. */ export function rebaseStaleWizardConversationHydration( visible: WizardConversationPayload, stale: WizardConversationPayload, confirmed: WizardConversationPayload, + options: { honorLocalDeletes?: boolean } = {}, ): { conversation: WizardConversationPayload; needsPersist: boolean } { - const conversation = mergeQueuedWizardConversationSnapshots(visible, stale, confirmed) + const honorLocalDeletes = options.honorLocalDeletes ?? false + const conversation: WizardConversationPayload = { + version: 1, + revision: confirmed.revision, + messages: mergeQueuedValues(visible.messages, stale.messages, confirmed.messages, honorLocalDeletes), + executions: mergeQueuedValues(visible.executions, stale.executions, confirmed.executions, honorLocalDeletes), + requestedActions: mergeQueuedValues( + visible.requestedActions, + stale.requestedActions, + confirmed.requestedActions, + honorLocalDeletes, + ), + executedActions: mergeQueuedValues( + visible.executedActions, + stale.executedActions, + confirmed.executedActions, + honorLocalDeletes, + ), + confirmations: mergeQueuedValues( + visible.confirmations, + stale.confirmations, + confirmed.confirmations, + honorLocalDeletes, + ), + } const semanticContent = (value: WizardConversationPayload) => ({ messages: value.messages, executions: value.executions, @@ -99,7 +125,12 @@ function valueIdentity(value: unknown): string { } /** Apply local edits/deletes since base without discarding concurrent values. */ -function mergeQueuedValues(local: unknown, base: unknown, canonical: unknown): unknown[] { +function mergeQueuedValues( + local: unknown, + base: unknown, + canonical: unknown, + honorLocalDeletes = true, +): unknown[] { const localValues = Array.isArray(local) ? local : [] const baseValues = Array.isArray(base) ? base : [] const canonicalValues = Array.isArray(canonical) ? canonical : [] @@ -112,7 +143,7 @@ function mergeQueuedValues(local: unknown, base: unknown, canonical: unknown): u const id = valueIdentity(value) const baseValue = baseById.get(id) const localValue = localById.get(id) - if (baseById.has(id) && !localById.has(id)) return + if (honorLocalDeletes && baseById.has(id) && !localById.has(id)) return if (localById.has(id) && (!baseById.has(id) || stableKey(localValue) !== stableKey(baseValue))) { merged.push(localValue) } else { diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 0dc76732f..074e6ad1e 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -404,3 +404,30 @@ test('stale hydration restores confirmed-only turns without scheduling a redunda assert.equal(rebased.needsPersist, false) assert.deepEqual(rebased.conversation.messages, confirmed.messages) }) + +test('stale browser cache omissions do not delete newer canonical turns during hydration', () => { + const stale = payload(7, ['shared-user', 'stale-assistant']) + const confirmed = payload(8, ['shared-user', 'stale-assistant', 'confirmed-user']) + const olderVisibleCache = payload(5, ['shared-user']) + + const rebased = rebaseStaleWizardConversationHydration(olderVisibleCache, stale, confirmed) + + assert.equal(rebased.needsPersist, false) + assert.deepEqual(rebased.conversation.messages, confirmed.messages) +}) + +test('explicit clear deletes base turns while retaining concurrent canonical additions', () => { + const stale = payload(7, ['old-user', 'old-assistant']) + const confirmed = payload(8, ['old-user', 'old-assistant', 'concurrent-user']) + const cleared = payload(7, ['welcome-after-clear']) + + const rebased = rebaseStaleWizardConversationHydration(cleared, stale, confirmed, { + honorLocalDeletes: true, + }) + + assert.equal(rebased.needsPersist, true) + assert.deepEqual(rebased.conversation.messages.map(message => message.id), [ + 'concurrent-user', + 'welcome-after-clear', + ]) +}) From 71b71824b2d87edb0efe60e67e7bf03544e5d4ea Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 20:00:52 +0200 Subject: [PATCH 12/18] fix(wizard): normalize optional conversation fields --- .../agent/wizardConversationPersistence.ts | 10 ++++- .../wizardConversationPersistence.test.mjs | 45 +++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index 634b5d673..bef6a46d8 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -93,11 +93,19 @@ const defaultTransport: WizardConversationTransport = { save: saveWizardConversation, } +function isEmptyStableValue(value: unknown): boolean { + return value == null || value === '' || (Array.isArray(value) && value.length === 0) +} + function stableKey(value: unknown): string { if (value === null || typeof value !== 'object') return JSON.stringify(value) ?? String(value) if (Array.isArray(value)) return `[${value.map(stableKey).join(',')}]` const record = value as Record - return `{${Object.keys(record).sort().map(key => `${JSON.stringify(key)}:${stableKey(record[key])}`).join(',')}}` + return `{${Object.keys(record) + .sort() + .filter(key => !isEmptyStableValue(record[key])) + .map(key => `${JSON.stringify(key)}:${stableKey(record[key])}`) + .join(',')}}` } function mergeUniqueValues(remote: unknown, local: unknown): unknown[] { diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 074e6ad1e..ac10a779b 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -405,6 +405,51 @@ test('stale hydration restores confirmed-only turns without scheduling a redunda assert.deepEqual(rebased.conversation.messages, confirmed.messages) }) +test('UI and server message shape differences are not treated as local edits', () => { + const serverMessage = (id, role, text, createdAt, extra = {}) => ({ + id, + role, + text, + createdAt, + cards: [], + executionKey: '', + jobLinks: [], + lastState: '', + error: '', + ...extra, + }) + const stale = { + version: 1, + revision: 7, + messages: [serverMessage('shared-user', 'user', 'hello', 1)], + executions: [], + } + const confirmed = { + version: 1, + revision: 8, + messages: [ + serverMessage('shared-user', 'user', 'hello', 1, { executionKey: 'server-key' }), + serverMessage('confirmed-assistant', 'assistant', 'reply', 2), + ], + executions: [], + } + const visible = { + version: 1, + revision: 7, + messages: [{ id: 'shared-user', role: 'user', text: 'hello', createdAt: 1 }], + executions: [], + } + + const rebased = rebaseStaleWizardConversationHydration(visible, stale, confirmed) + + assert.equal(rebased.needsPersist, false) + assert.equal(rebased.conversation.messages[0].executionKey, 'server-key') + assert.deepEqual(rebased.conversation.messages.map(message => message.id), [ + 'shared-user', + 'confirmed-assistant', + ]) +}) + test('stale browser cache omissions do not delete newer canonical turns during hydration', () => { const stale = payload(7, ['shared-user', 'stale-assistant']) const confirmed = payload(8, ['shared-user', 'stale-assistant', 'confirmed-user']) From 0cb97c32f30de361a97cc61a2a787e24c16f91a5 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 20:12:57 +0200 Subject: [PATCH 13/18] fix(wizard): preserve clear ancestry by workspace --- ui/src/features/agent/AgentAssistantPanel.tsx | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index a5cfcad1d..ab69c1e96 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -167,7 +167,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const conversationSnapshotsRef = useRef>(new Map()) const conversationSaveChainRef = useRef>(Promise.resolve()) const skipNextConversationSaveRef = useRef(false) - const explicitConversationClearRef = useRef(false) + const conversationClearBasesRef = useRef>(new Map()) const conversationWorkspaceRef = useRef(conversationWorkspace) conversationWorkspaceRef.current = conversationWorkspace const messagesRef = useRef(messages) @@ -249,6 +249,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals captured: capturedConversation, base: conversationSnapshotsRef.current.get(conversationWorkspace), } + const queuedClearBase = conversationClearBasesRef.current.get(conversationWorkspace) conversationSaveChainRef.current = enqueueWizardConversationSave( conversationSaveChainRef.current, async () => { @@ -257,6 +258,9 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals queuedWrite, conversationSnapshotsRef.current, ) + if (queuedClearBase && conversationClearBasesRef.current.get(conversationWorkspace) === queuedClearBase) { + conversationClearBasesRef.current.delete(conversationWorkspace) + } if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return setConversationSaveError(null) if (saved.merged) { @@ -283,16 +287,16 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const knownSnapshot = conversationSnapshotsRef.current.get(conversationWorkspace) const hydration = resolveWizardConversationHydration(knownSnapshot, payload) conversationSnapshotsRef.current.set(conversationWorkspace, hydration.snapshot) - if (!hydration.applyToVisibleState || explicitConversationClearRef.current) { + const clearBase = conversationClearBasesRef.current.get(conversationWorkspace) + if (!hydration.applyToVisibleState || clearBase) { const visibleMessages = messagesRef.current const rebased = rebaseStaleWizardConversationHydration({ ...payload, messages: visibleMessages, executions: visibleMessages.flatMap(message => message.cards || []), - }, payload, hydration.snapshot, { - honorLocalDeletes: explicitConversationClearRef.current, + }, clearBase ?? payload, hydration.snapshot, { + honorLocalDeletes: Boolean(clearBase), }) - explicitConversationClearRef.current = false skipNextConversationSaveRef.current = !rebased.needsPersist setMessages(normalizeRemoteWizardMessages( rebased.conversation.messages, @@ -327,7 +331,6 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals useEffect(() => { if (!shouldFollowWizardWorkspace({ activeWorkspace: workspace, conversationWorkspace, busy })) return skipNextConversationSaveRef.current = false - explicitConversationClearRef.current = false setConversationSaveError(null) setHydratedWorkspace(null) setMessages(readMessages(workspace)) @@ -383,7 +386,14 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const clearConversation = () => { const next = [welcomeMessage()] - explicitConversationClearRef.current = true + const snapshot = conversationSnapshotsRef.current.get(conversationWorkspace) + conversationClearBasesRef.current.set(conversationWorkspace, { + ...snapshot, + version: 1, + revision: snapshot?.revision || 0, + messages: messagesRef.current, + executions: messagesRef.current.flatMap(message => message.cards || []), + }) setMessages(next) setState('idle') } From 841a8f3e376e4c8c53739acf4a8059cd8d6734a8 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 20:22:38 +0200 Subject: [PATCH 14/18] fix(wizard): persist clears from their recorded base --- ui/src/features/agent/AgentAssistantPanel.tsx | 9 +++++-- .../wizardConversationPersistence.test.mjs | 27 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index ab69c1e96..16d32bbed 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -244,12 +244,14 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals messages, executions: cards, } + const queuedClearBase = conversationClearBasesRef.current.get(conversationWorkspace) const queuedWrite = { workspace: conversationWorkspace, captured: capturedConversation, - base: conversationSnapshotsRef.current.get(conversationWorkspace), + // A clear is a three-way delete relative to the exact conversation the + // user saw, even if an earlier queued save advances the canonical state. + base: queuedClearBase ?? conversationSnapshotsRef.current.get(conversationWorkspace), } - const queuedClearBase = conversationClearBasesRef.current.get(conversationWorkspace) conversationSaveChainRef.current = enqueueWizardConversationSave( conversationSaveChainRef.current, async () => { @@ -394,6 +396,9 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals messages: messagesRef.current, executions: messagesRef.current.flatMap(message => message.cards || []), }) + // An explicit user mutation must never consume a skip reserved for an + // earlier canonical hydration render. + skipNextConversationSaveRef.current = false setMessages(next) setState('idle') } diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index ac10a779b..8ef4718c7 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -261,6 +261,33 @@ test('a stale queued snapshot merges the canonical turn saved by its predecessor assert.deepEqual(snapshots.get('workspace-a'), canonical) }) +test('a queued clear uses its recorded ancestor after a predecessor advances the snapshot', async () => { + const clearBase = payload(1, ['cleared-user', 'cleared-assistant']) + const canonical = payload(2, ['cleared-user', 'cleared-assistant', 'concurrent-user']) + const capturedClear = payload(1, ['welcome-after-clear']) + const snapshots = new Map([['workspace-a', clone(canonical)]]) + const transport = { + async fetch() { return clone(canonical) }, + async save(_workspace, conversation) { + assert.equal(conversation.revision, 2) + assert.deepEqual(conversation.messages.map(message => message.id), [ + 'concurrent-user', + 'welcome-after-clear', + ]) + return { ...clone(conversation), revision: 3 } + }, + } + + const saved = await persistQueuedWizardConversation({ + workspace: 'workspace-a', + captured: capturedClear, + base: clearBase, + }, snapshots, transport) + + assert.equal(saved.conversation.revision, 3) + assert.deepEqual(snapshots.get('workspace-a'), saved.conversation) +}) + test('a queued write persists to its captured workspace after the visible workspace changes', async () => { const snapshots = new Map() const savedWorkspaces = [] From 692e99b0439f68efc57942503004c8f8ceff721f Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 20:27:30 +0200 Subject: [PATCH 15/18] refactor(wizard): isolate task state mapping --- ui/src/features/agent/AgentAssistantPanel.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 16d32bbed..63704121f 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -147,6 +147,13 @@ function writeMessages(workspace: string, messages: AgentMessage[]): void { } } +function taskExecutionState(status: CanonicalTask['status']): 'completed' | 'failed' | 'queued' | 'running' { + if (status === 'completed') return 'completed' + if (status === 'failed' || status === 'cancelled') return 'failed' + if (status === 'queued' || status === 'waiting_resource') return 'queued' + return 'running' +} + export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = false }: AgentAssistantPanelProps) { const { t } = useUiTranslation('wizard') const { t: tCommon } = useUiTranslation('common') @@ -351,10 +358,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals || (card.pipelineId && item.pipeline_id === card.pipelineId) )) if (!task) return card - const state = task.status === 'completed' ? 'completed' - : task.status === 'failed' || task.status === 'cancelled' ? 'failed' - : task.status === 'queued' || task.status === 'waiting_resource' ? 'queued' - : 'running' + const state = taskExecutionState(task.status) const outputNames = task.result_refs?.length ? task.result_refs : card.outputNames if (state === card.state && (task.message || card.message) === card.message && outputNames === card.outputNames) { return card From a03e0af31f31d0c02690c62e64c1cac7e4bfb510 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 20:49:24 +0200 Subject: [PATCH 16/18] fix(wizard): preserve explicit conversation write intent --- ui/src/features/agent/AgentAssistantPanel.tsx | 30 ++++--- .../agent/wizardConversationPersistence.ts | 25 ++++-- .../features/agent/wizardConversationSync.ts | 10 ++- .../wizardConversationPersistence.test.mjs | 78 +++++++++++++++++++ 4 files changed, 119 insertions(+), 24 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 63704121f..4f4972bb7 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -17,9 +17,7 @@ import { import { applyPollToCard, cardsFromResults, tabForExecutionTarget, type WizardExecutionCard } from './executionCards' import { applyRemoteWizardConversation, - hasExclusiveWizardMessages, isWizardConversationWriteCurrent, - mergeWizardMessages, normalizeRemoteWizardMessages, shouldFollowWizardWorkspace, WIZARD_WELCOME_TEXT, @@ -258,6 +256,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals // A clear is a three-way delete relative to the exact conversation the // user saw, even if an earlier queued save advances the canonical state. base: queuedClearBase ?? conversationSnapshotsRef.current.get(conversationWorkspace), + honorLocalDeletes: Boolean(queuedClearBase), } conversationSaveChainRef.current = enqueueWizardConversationSave( conversationSaveChainRef.current, @@ -272,13 +271,19 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals } if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return setConversationSaveError(null) - if (saved.merged) { - const canonicalMessages = normalizeRemoteWizardMessages(saved.conversation.messages, saved.conversation.executions) - const hasExclusiveLocal = hasExclusiveWizardMessages(messagesRef.current, canonicalMessages) - if (!hasExclusiveLocal) { - skipNextConversationSaveRef.current = true - } - setMessages(mergeWizardMessages(messagesRef.current, canonicalMessages) as AgentMessage[]) + const visibleMessages = messagesRef.current + const rebased = rebaseStaleWizardConversationHydration({ + ...queuedWrite.captured, + revision: saved.conversation.revision, + messages: visibleMessages, + executions: visibleMessages.flatMap(message => message.cards || []), + }, queuedWrite.captured, saved.conversation) + if (saved.merged || rebased.needsPersist) { + skipNextConversationSaveRef.current = !rebased.needsPersist + setMessages(normalizeRemoteWizardMessages( + rebased.conversation.messages, + rebased.conversation.executions, + ) as AgentMessage[]) } } catch (error) { if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return @@ -297,13 +302,13 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const hydration = resolveWizardConversationHydration(knownSnapshot, payload) conversationSnapshotsRef.current.set(conversationWorkspace, hydration.snapshot) const clearBase = conversationClearBasesRef.current.get(conversationWorkspace) - if (!hydration.applyToVisibleState || clearBase) { + if (!hydration.applyToVisibleState || clearBase || knownSnapshot) { const visibleMessages = messagesRef.current const rebased = rebaseStaleWizardConversationHydration({ ...payload, messages: visibleMessages, executions: visibleMessages.flatMap(message => message.cards || []), - }, clearBase ?? payload, hydration.snapshot, { + }, clearBase ?? knownSnapshot ?? payload, hydration.snapshot, { honorLocalDeletes: Boolean(clearBase), }) skipNextConversationSaveRef.current = !rebased.needsPersist @@ -317,7 +322,8 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals const canonicalSnapshot = hydration.snapshot const choice = applyRemoteWizardConversation({ localMessages: messagesRef.current, - localRevision: knownSnapshot?.revision || 0, + // A known snapshot is handled by the three-way branch above. + localRevision: 0, remoteMessages: canonicalSnapshot.messages, remoteRevision: canonicalSnapshot.revision || 0, remoteExecutions: canonicalSnapshot.executions, diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index bef6a46d8..a4cde8cfd 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -25,6 +25,8 @@ export interface QueuedWizardConversationWrite { workspace: string captured: WizardConversationPayload base?: WizardConversationPayload + /** Only explicit user mutations such as Clear may delete values absent locally. */ + honorLocalDeletes?: boolean } export function resolveWizardConversationHydration( @@ -172,21 +174,23 @@ export function mergeQueuedWizardConversationSnapshots( local: WizardConversationPayload, base: WizardConversationPayload | undefined, canonical: WizardConversationPayload, + options: { honorLocalDeletes?: boolean } = {}, ): WizardConversationPayload { + const honorLocalDeletes = options.honorLocalDeletes ?? true return { version: 1, revision: canonical.revision, - messages: mergeQueuedValues(local.messages, base?.messages, canonical.messages), - executions: mergeQueuedValues(local.executions, base?.executions, canonical.executions), + messages: mergeQueuedValues(local.messages, base?.messages, canonical.messages, honorLocalDeletes), + executions: mergeQueuedValues(local.executions, base?.executions, canonical.executions, honorLocalDeletes), requestedActions: local.requestedActions === undefined ? canonical.requestedActions - : mergeQueuedValues(local.requestedActions, base?.requestedActions, canonical.requestedActions), + : mergeQueuedValues(local.requestedActions, base?.requestedActions, canonical.requestedActions, honorLocalDeletes), executedActions: local.executedActions === undefined ? canonical.executedActions - : mergeQueuedValues(local.executedActions, base?.executedActions, canonical.executedActions), + : mergeQueuedValues(local.executedActions, base?.executedActions, canonical.executedActions, honorLocalDeletes), confirmations: local.confirmations === undefined ? canonical.confirmations - : mergeQueuedValues(local.confirmations, base?.confirmations, canonical.confirmations), + : mergeQueuedValues(local.confirmations, base?.confirmations, canonical.confirmations, honorLocalDeletes), } } @@ -234,6 +238,7 @@ export async function saveWizardConversationWithRecovery( conversation: WizardConversationPayload, transport: WizardConversationTransport = defaultTransport, base?: WizardConversationPayload, + options: { honorLocalDeletes?: boolean } = {}, ): Promise { try { return { @@ -244,7 +249,7 @@ export async function saveWizardConversationWithRecovery( if (!isWizardConversationConflict(error)) throw error const remote = await transport.fetch(workspace) const merged = base - ? mergeQueuedWizardConversationSnapshots(conversation, base, remote) + ? mergeQueuedWizardConversationSnapshots(conversation, base, remote, options) : mergeWizardConversationSnapshots(conversation, remote) return { conversation: await transport.save(workspace, merged), @@ -269,9 +274,13 @@ export async function persistQueuedWizardConversation( ): Promise { const canonical = snapshots.get(write.workspace) const outgoing = canonical - ? mergeQueuedWizardConversationSnapshots(write.captured, write.base, canonical) + ? mergeQueuedWizardConversationSnapshots(write.captured, write.base, canonical, { + honorLocalDeletes: write.honorLocalDeletes ?? false, + }) : write.captured - const saved = await saveWizardConversationWithRecovery(write.workspace, outgoing, transport, canonical) + const saved = await saveWizardConversationWithRecovery(write.workspace, outgoing, transport, write.base, { + honorLocalDeletes: write.honorLocalDeletes ?? false, + }) snapshots.set(write.workspace, saved.conversation) return saved } diff --git a/ui/src/features/agent/wizardConversationSync.ts b/ui/src/features/agent/wizardConversationSync.ts index b52d81bc7..0ffe9d807 100644 --- a/ui/src/features/agent/wizardConversationSync.ts +++ b/ui/src/features/agent/wizardConversationSync.ts @@ -61,20 +61,22 @@ export function normalizeRemoteWizardMessages( /** * Merge two snapshots without duplicating a message id. * - * The remote snapshot is canonical and therefore wins for an id already - * persisted there. Local-only messages are appended in their existing order; - * this makes retries idempotent while keeping a user's turn together. + * Remote order is canonical, but a local value wins for a shared id. Callers + * use this fallback only when no common ancestor is available, so preserving + * an in-browser card/workflow update is safer than silently reverting it. + * Local-only messages are appended in their existing order. */ export function mergeWizardMessages( localMessages: WizardSyncMessage[], remoteMessages: WizardSyncMessage[], ): WizardSyncMessage[] { + const localById = new Map(localMessages.map(message => [message.id, message])) const merged: WizardSyncMessage[] = [] const seen = new Set() for (const message of remoteMessages) { if (!message.id || seen.has(message.id)) continue seen.add(message.id) - merged.push(message) + merged.push(localById.get(message.id) ?? message) } for (const message of localMessages) { if (!message.id || seen.has(message.id)) continue diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 8ef4718c7..744459eb3 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -17,6 +17,7 @@ const { const { isWizardConversationWriteCurrent, hasExclusiveWizardMessages, + mergeWizardMessages, shouldFollowWizardWorkspace, } = await import('../src/features/agent/wizardConversationSync.ts') @@ -282,6 +283,7 @@ test('a queued clear uses its recorded ancestor after a predecessor advances the workspace: 'workspace-a', captured: capturedClear, base: clearBase, + honorLocalDeletes: true, }, snapshots, transport) assert.equal(saved.conversation.revision, 3) @@ -383,6 +385,82 @@ test('a CAS conflict during a queued edit keeps the edit and the concurrent turn assert.deepEqual(saved.conversation.messages.map(message => message.id), ['shared-user', 'remote-assistant']) }) +test('a normal queued save cannot delete canonical turns outside the 40-message UI window', async () => { + const ids = Array.from({ length: 50 }, (_value, index) => `message-${index + 1}`) + const canonical = payload(7, ids) + const visibleWindow = payload(7, [...ids.slice(-40), 'new-local-message']) + const snapshots = new Map([['workspace-a', clone(canonical)]]) + const transport = { + async fetch() { return clone(canonical) }, + async save(_workspace, conversation) { + assert.equal(conversation.revision, 7) + assert.deepEqual(conversation.messages.map(message => message.id), [...ids, 'new-local-message']) + return { ...clone(conversation), revision: 8 } + }, + } + + await persistQueuedWizardConversation({ + workspace: 'workspace-a', + captured: visibleWindow, + base: canonical, + }, snapshots, transport) +}) + +test('a conflict retry keeps using the recorded clear ancestor', async () => { + const clearBase = payload(1, ['cleared-user', 'cleared-assistant']) + const capturedClear = payload(1, ['welcome-after-clear']) + const snapshotAfterEarlierWrite = payload(2, ['concurrent-before-conflict']) + const remoteAfterConflict = payload(3, [ + 'cleared-user', + 'cleared-assistant', + 'concurrent-before-conflict', + 'concurrent-during-conflict', + ]) + const snapshots = new Map([['workspace-a', clone(snapshotAfterEarlierWrite)]]) + let saves = 0 + const transport = { + async fetch() { return clone(remoteAfterConflict) }, + async save(_workspace, conversation) { + saves += 1 + if (saves === 1) throw revisionConflict(conversation.revision, remoteAfterConflict.revision) + assert.equal(conversation.revision, 3) + assert.deepEqual(conversation.messages.map(message => message.id), [ + 'concurrent-before-conflict', + 'concurrent-during-conflict', + 'welcome-after-clear', + ]) + return { ...clone(conversation), revision: 4 } + }, + } + + const saved = await persistQueuedWizardConversation({ + workspace: 'workspace-a', + captured: capturedClear, + base: clearBase, + honorLocalDeletes: true, + }, snapshots, transport) + + assert.equal(saved.merged, true) + assert.equal(saved.conversation.revision, 4) +}) + +test('a shared message id retains the newer local workflow card', () => { + const remote = [{ + id: 'assistant-workflow', role: 'assistant', text: 'Generating', createdAt: 1, + cards: [{ id: 'card-1', state: 'running' }], + }] + const local = [{ + ...remote[0], + text: 'Completed', + cards: [{ id: 'card-1', state: 'completed' }], + }] + + const merged = mergeWizardMessages(local, remote) + + assert.equal(merged[0].text, 'Completed') + assert.equal(merged[0].cards[0].state, 'completed') +}) + test('a late hydration fetch cannot replace a newer confirmed snapshot or visible edits', () => { const confirmed = payload(8, ['confirmed-user', 'confirmed-assistant']) const staleFetch = payload(7, ['stale-user']) From d72da0ca99f93f0c024032a9fab75cbb0214d154 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 21:01:20 +0200 Subject: [PATCH 17/18] fix(wizard): preserve clears across in-flight saves --- ui/src/features/agent/AgentAssistantPanel.tsx | 7 +++--- .../agent/wizardConversationPersistence.ts | 19 ++++++++++++++ .../wizardConversationPersistence.test.mjs | 25 +++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 4f4972bb7..92940a9ca 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -26,7 +26,7 @@ import { AgentMarkdown } from './AgentMarkdown' import { defaultWizardWorkflowRuntime, type WizardWorkflowPendingInput, type WizardWorkflowRecord } from './wizardWorkflowRuntime' import { ensureRhythmic3dWorkflowRegistered } from './rhythmic3dWorkflow' import { defaultApplicationAdapters } from './applicationAdapters' -import { enqueueWizardConversationSave, persistQueuedWizardConversation, rebaseStaleWizardConversationHydration, resolveWizardConversationHydration } from './wizardConversationPersistence' +import { enqueueWizardConversationSave, persistQueuedWizardConversation, rebaseStaleWizardConversationHydration, rebaseWizardConversationAfterSave, resolveWizardConversationHydration } from './wizardConversationPersistence' import i18n, { useUiTranslation } from '../../i18n' export { AgentAvatar, type AgentVisualState } from './AgentAvatar' @@ -272,12 +272,13 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return setConversationSaveError(null) const visibleMessages = messagesRef.current - const rebased = rebaseStaleWizardConversationHydration({ + const pendingClearBase = conversationClearBasesRef.current.get(conversationWorkspace) + const rebased = rebaseWizardConversationAfterSave({ ...queuedWrite.captured, revision: saved.conversation.revision, messages: visibleMessages, executions: visibleMessages.flatMap(message => message.cards || []), - }, queuedWrite.captured, saved.conversation) + }, queuedWrite.captured, saved.conversation, pendingClearBase) if (saved.merged || rebased.needsPersist) { skipNextConversationSaveRef.current = !rebased.needsPersist setMessages(normalizeRemoteWizardMessages( diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts index a4cde8cfd..c3ed96c4c 100644 --- a/ui/src/features/agent/wizardConversationPersistence.ts +++ b/ui/src/features/agent/wizardConversationPersistence.ts @@ -90,6 +90,25 @@ export function rebaseStaleWizardConversationHydration( } } +/** + * Reconcile a save result with UI changes made while that save was in flight. + * A pending Clear supplies its own visible ancestor and is the only case where + * absent values represent intentional deletion. + */ +export function rebaseWizardConversationAfterSave( + visible: WizardConversationPayload, + captured: WizardConversationPayload, + confirmed: WizardConversationPayload, + pendingClearBase?: WizardConversationPayload, +): { conversation: WizardConversationPayload; needsPersist: boolean } { + return rebaseStaleWizardConversationHydration( + visible, + pendingClearBase ?? captured, + confirmed, + { honorLocalDeletes: Boolean(pendingClearBase) }, + ) +} + const defaultTransport: WizardConversationTransport = { fetch: fetchWizardConversation, save: saveWizardConversation, diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs index 744459eb3..3c46196b9 100644 --- a/ui/tests/wizardConversationPersistence.test.mjs +++ b/ui/tests/wizardConversationPersistence.test.mjs @@ -8,6 +8,7 @@ const { enqueueWizardConversationSave, persistQueuedWizardConversation, rebaseStaleWizardConversationHydration, + rebaseWizardConversationAfterSave, resolveWizardConversationHydration, } = await import('../src/features/agent/wizardConversationPersistence.ts') const { @@ -461,6 +462,30 @@ test('a shared message id retains the newer local workflow card', () => { assert.equal(merged[0].cards[0].state, 'completed') }) +test('a Clear made while an earlier save is in flight is not undone by that save', () => { + const earlierCaptured = payload(5, ['old-user', 'old-assistant']) + const confirmedEarlierSave = payload(6, [ + 'old-user', + 'old-assistant', + 'concurrent-before-clear', + ]) + const pendingClearBase = payload(5, ['old-user', 'old-assistant']) + const visibleAfterClear = payload(5, ['welcome-after-clear']) + + const rebased = rebaseWizardConversationAfterSave( + visibleAfterClear, + earlierCaptured, + confirmedEarlierSave, + pendingClearBase, + ) + + assert.equal(rebased.needsPersist, true) + assert.deepEqual(rebased.conversation.messages.map(message => message.id), [ + 'concurrent-before-clear', + 'welcome-after-clear', + ]) +}) + test('a late hydration fetch cannot replace a newer confirmed snapshot or visible edits', () => { const confirmed = payload(8, ['confirmed-user', 'confirmed-assistant']) const staleFetch = payload(7, ['stale-user']) From c4b627fc91553bba55daa062f0653c8d65410343 Mon Sep 17 00:00:00 2001 From: pinokio Date: Thu, 3 Sep 2026 21:08:57 +0200 Subject: [PATCH 18/18] fix(wizard): retain clear intent through settlement --- ui/src/features/agent/AgentAssistantPanel.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx index 92940a9ca..519d84480 100644 --- a/ui/src/features/agent/AgentAssistantPanel.tsx +++ b/ui/src/features/agent/AgentAssistantPanel.tsx @@ -273,6 +273,7 @@ export function AgentAssistantPanel({ workspace, tasks, onClose, embedded = fals setConversationSaveError(null) const visibleMessages = messagesRef.current const pendingClearBase = conversationClearBasesRef.current.get(conversationWorkspace) + ?? (queuedWrite.honorLocalDeletes ? queuedWrite.base : undefined) const rebased = rebaseWizardConversationAfterSave({ ...queuedWrite.captured, revision: saved.conversation.revision,