fix(wizard): recover safely from conversation conflicts - #122
Merged
Conversation
PR Review — Loreframe StudioRisk: medium Automated review from Findings
Changed files
CONTRIBUTING checklist
Posted by the repo PR review workflow. Re-runs on each push to the PR. |
Owner
Author
|
@cursor review |
Code healthQuality score: 49.1/100Higher is better. The score is a trend dashboard; the independent ratchet below remains the CI gate.
Change vs PR base: +0.2 points.
Markdown, JSON catalogs and tests are out of this table. Only Most complex functions
Trend vs baseline
Warnings
Ratchet passed. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Recovery overwrites newer conversation turns
- Tras un merge por conflicto CAS, solo se omite el siguiente guardado cuando el snapshot canónico ya incluye todos los turnos visibles, permitiendo que los turnos más nuevos se persistan de inmediato.
- ✅ Fixed: Busy isolation mixes workspace events
- Las suscripciones de workflow, el reseteo de pending input y el polling de tareas ahora usan conversationWorkspace y se omiten cuando difiere del workspace activo durante busy.
Or push these changes by commenting:
@cursor push f2eab0fac2
Preview (f2eab0fac2)
diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx
--- a/ui/src/features/agent/AgentAssistantPanel.tsx
+++ b/ui/src/features/agent/AgentAssistantPanel.tsx
@@ -183,7 +183,7 @@
let active = true
ensureRhythmic3dWorkflowRegistered(defaultApplicationAdapters)
const unsubscribe = defaultWizardWorkflowRuntime.subscribe(({ workflow, card }) => {
- if (!active || workflow.workspace !== workspace) return
+ if (!active || workflow.workspace !== conversationWorkspace) return
setActiveWorkflow(workflow)
setPendingInput(workflow.state === 'awaiting_input' ? workflow.pendingInput : null)
setMessages(current => {
@@ -207,10 +207,10 @@
}].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 +220,12 @@
unsubscribe()
closeEvents()
}
- }, [workspace])
+ }, [conversationWorkspace])
useEffect(() => {
setActiveWorkflow(null)
setPendingInput(null)
- }, [workspace])
+ }, [conversationWorkspace])
useEffect(() => {
writeMessages(conversationWorkspace, messages)
@@ -251,8 +251,12 @@
conversationRevisionRef.current = saved.conversation.revision
setConversationSaveError(null)
if (saved.merged) {
- skipNextConversationSaveRef.current = true
const canonicalMessages = normalizeRemoteWizardMessages(saved.conversation.messages, saved.conversation.executions)
+ const canonicalIds = new Set(canonicalMessages.map(message => message.id))
+ const hasExclusiveLocal = messagesRef.current.some(message => !canonicalIds.has(message.id))
+ if (!hasExclusiveLocal) {
+ skipNextConversationSaveRef.current = true
+ }
setMessages(mergeWizardMessages(messagesRef.current, canonicalMessages) as AgentMessage[])
}
}).catch(error => {
@@ -306,6 +310,7 @@
}, [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 @@
})
return changed ? { ...message, cards } : message
}))
- }, [tasks])
+ }, [conversationWorkspace, tasks, workspace])
useEffect(() => {
const closeOnEscape = (event: KeyboardEvent) => {You can send follow-ups to the cloud agent here.
Owner
Author
|
@cursor review |
Owner
Author
|
@cursor review |
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Queued persist merge undoes local updates
- persistQueuedWizardConversation now adopts only the latest confirmed revision and writes the captured conversation as-is, so a local clear or in-place card edit is no longer unioned back into the previous snapshot.
Or push these changes by commenting:
@cursor push 92841a541e
Preview (92841a541e)
diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts
--- a/ui/src/features/agent/wizardConversationPersistence.ts
+++ b/ui/src/features/agent/wizardConversationPersistence.ts
@@ -109,11 +109,15 @@
}
/**
- * 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.
+ * Persist one queued snapshot against the latest confirmed revision known for
+ * its workspace. Queued React effects capture the visible conversation, but
+ * must not capture the revision: an earlier queued save may advance it before
+ * this write starts.
*
+ * The captured conversation is written as-is. Snapshot merge is reserved for a
+ * real CAS 409; unioning with the last snapshot on every queued write would
+ * restore turns a local clear or in-place edit had already replaced.
+ *
* The store is keyed by workspace so changing the visible workspace never
* rebinds or drops a write that was already accepted into the queue.
*/
@@ -125,7 +129,7 @@
): Promise<WizardConversationSaveResult> {
const canonical = snapshots.get(workspace)
const outgoing = canonical
- ? mergeWizardConversationSnapshots(captured, canonical)
+ ? { ...captured, revision: canonical.revision }
: captured
const saved = await saveWizardConversationWithRecovery(workspace, outgoing, transport)
snapshots.set(workspace, saved.conversation)
diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs
--- a/ui/tests/wizardConversationPersistence.test.mjs
+++ b/ui/tests/wizardConversationPersistence.test.mjs
@@ -226,7 +226,7 @@
assert.equal(revision, 2)
})
-test('a stale queued snapshot merges the canonical turn saved by its predecessor', async () => {
+test('a stale queued snapshot adopts the predecessor revision and writes the captured conversation', async () => {
let canonical = payload(0, [])
const snapshots = new Map()
const savedPayloads = []
@@ -241,13 +241,13 @@
}
const firstVisible = payload(0, ['first-user', 'first-assistant'])
- const staleSecondEffect = payload(0, ['second-user', 'second-assistant'])
+ const laterVisible = payload(0, ['first-user', 'first-assistant', '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)
+ persistQueuedWizardConversation('workspace-a', laterVisible, snapshots, transport).then(() => undefined)
))
await chain
@@ -258,6 +258,42 @@
assert.deepEqual(snapshots.get('workspace-a'), canonical)
})
+test('a queued persist does not restore snapshot turns after a local replacement or edit', 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 prior = payload(0, ['old-user', 'old-assistant'], 'old-')
+ const cleared = payload(0, ['welcome-after-clear'])
+ const editedWelcome = {
+ ...cleared,
+ messages: [{
+ ...cleared.messages[0],
+ text: 'updated welcome',
+ cards: [{ id: 'card-1', state: 'completed' }],
+ }],
+ executions: [{ id: 'card-1', state: 'completed' }],
+ }
+
+ await persistQueuedWizardConversation('workspace-a', prior, snapshots, transport)
+ await persistQueuedWizardConversation('workspace-a', cleared, snapshots, transport)
+ await persistQueuedWizardConversation('workspace-a', editedWelcome, snapshots, transport)
+
+ assert.deepEqual(savedPayloads[1].messages.map(message => message.id), ['welcome-after-clear'])
+ assert.equal(savedPayloads[2].messages[0].text, 'updated welcome')
+ assert.deepEqual(savedPayloads[2].messages[0].cards, [{ id: 'card-1', state: 'completed' }])
+ assert.equal(savedPayloads[2].revision, 2)
+})
+
test('a queued write persists to its captured workspace after the visible workspace changes', async () => {
const snapshots = new Map()
const savedWorkspaces = []You can send follow-ups to the cloud agent here.
Owner
Author
|
@cursor review |
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Stale fetch overwrites newer snapshot
- Hydration now writes the fetched payload into conversationSnapshotsRef only when its revision is at least as new as the known snapshot, so a raced fetch cannot replace a confirmed CAS revision.
Or push these changes by commenting:
@cursor push 2b1e38ed69
Preview (2b1e38ed69)
diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx
--- a/ui/src/features/agent/AgentAssistantPanel.tsx
+++ b/ui/src/features/agent/AgentAssistantPanel.tsx
@@ -287,7 +287,12 @@
remoteRevision: payload.revision || 0,
remoteExecutions: payload.executions,
})
- conversationSnapshotsRef.current.set(conversationWorkspace, payload)
+ // A fetch that raced an in-flight save must not replace a newer
+ // confirmed CAS snapshot; later queued writes use this map as
+ // their baseRevision and recovery retries only one 409.
+ if ((payload.revision || 0) >= (knownSnapshot?.revision || 0)) {
+ 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 persistenceYou can send follow-ups to the cloud agent here.
Owner
Author
|
@cursor review |
Owner
Author
|
@cursor review |
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Rebase treats shape diffs as edits
- stableKey now ignores empty optional fields so UI messages and server payloads compare as equal, allowing stale hydration to skip redundant CAS writes and keep confirmed shared-id updates.
Or push these changes by commenting:
@cursor push b07e7070b6
Preview (b07e7070b6)
diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts
--- a/ui/src/features/agent/wizardConversationPersistence.ts
+++ b/ui/src/features/agent/wizardConversationPersistence.ts
@@ -67,11 +67,15 @@
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<string, unknown>
- 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
--- a/ui/tests/wizardConversationPersistence.test.mjs
+++ b/ui/tests/wizardConversationPersistence.test.mjs
@@ -404,3 +404,48 @@
assert.equal(rebased.needsPersist, false)
assert.deepEqual(rebased.conversation.messages, confirmed.messages)
})
+
+test('UI and server message shape diffs 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',
+ ])
+})You can send follow-ups to the cloud agent here.
Owner
Author
|
@cursor review |
Owner
Author
|
@cursor review |
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Clear persist drops recorded ancestor
- Clear persist now merges against the recorded visible ancestor so queued-but-unsaved turns are treated as local deletes before the ancestor is dropped.
Or push these changes by commenting:
@cursor push 4608a381b9
Preview (4608a381b9)
diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx
--- a/ui/src/features/agent/AgentAssistantPanel.tsx
+++ b/ui/src/features/agent/AgentAssistantPanel.tsx
@@ -244,12 +244,12 @@
messages,
executions: cards,
}
+ const queuedClearBase = conversationClearBasesRef.current.get(conversationWorkspace)
const queuedWrite = {
workspace: conversationWorkspace,
captured: capturedConversation,
- base: conversationSnapshotsRef.current.get(conversationWorkspace),
+ base: queuedClearBase ?? conversationSnapshotsRef.current.get(conversationWorkspace),
}
- const queuedClearBase = conversationClearBasesRef.current.get(conversationWorkspace)
conversationSaveChainRef.current = enqueueWizardConversationSave(
conversationSaveChainRef.current,
async () => {You can send follow-ups to the cloud agent here.
Owner
Author
|
@cursor review |
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
There are 5 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Post-merge save drops recovered turns
- Non-clear persists no longer treat the 40-message UI window as deletes, and exclusivity is checked against the full saved snapshot so recovered turns stay on the server.
- ✅ Fixed: Conflict retry resurrects cleared turns
- The 409 retry now three-way-merges against the queued write's recorded ancestor, so a predecessor clear still classifies those ids as deletes.
- ✅ Fixed: Shared-id merge discards local updates
- mergeWizardMessages and remote hydration now keep the newer local copy of a shared id so in-flight card and workflow updates are not overwritten.
Or push these changes by commenting:
@cursor push 904b7d2a13
Preview (904b7d2a13)
diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx
--- a/ui/src/features/agent/AgentAssistantPanel.tsx
+++ b/ui/src/features/agent/AgentAssistantPanel.tsx
@@ -258,6 +258,9 @@
// 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),
+ // The panel only holds 40 messages; older recovered ids must not look
+ // like local deletes unless the user explicitly cleared the chat.
+ honorLocalDeletes: Boolean(queuedClearBase),
}
conversationSaveChainRef.current = enqueueWizardConversationSave(
conversationSaveChainRef.current,
@@ -274,7 +277,10 @@
setConversationSaveError(null)
if (saved.merged) {
const canonicalMessages = normalizeRemoteWizardMessages(saved.conversation.messages, saved.conversation.executions)
- const hasExclusiveLocal = hasExclusiveWizardMessages(messagesRef.current, canonicalMessages)
+ const hasExclusiveLocal = hasExclusiveWizardMessages(
+ messagesRef.current,
+ saved.conversation.messages as AgentMessage[],
+ )
if (!hasExclusiveLocal) {
skipNextConversationSaveRef.current = true
}
diff --git a/ui/src/features/agent/wizardConversationPersistence.ts b/ui/src/features/agent/wizardConversationPersistence.ts
--- a/ui/src/features/agent/wizardConversationPersistence.ts
+++ b/ui/src/features/agent/wizardConversationPersistence.ts
@@ -25,6 +25,8 @@
workspace: string
captured: WizardConversationPayload
base?: WizardConversationPayload
+ /** Windowed UI captures omit older ids; only an explicit clear may delete them. */
+ honorLocalDeletes?: boolean
}
export function resolveWizardConversationHydration(
@@ -172,21 +174,22 @@
local: WizardConversationPayload,
base: WizardConversationPayload | undefined,
canonical: WizardConversationPayload,
+ honorLocalDeletes = true,
): WizardConversationPayload {
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 +237,7 @@
conversation: WizardConversationPayload,
transport: WizardConversationTransport = defaultTransport,
base?: WizardConversationPayload,
+ honorLocalDeletes = true,
): Promise<WizardConversationSaveResult> {
try {
return {
@@ -244,7 +248,7 @@
if (!isWizardConversationConflict(error)) throw error
const remote = await transport.fetch(workspace)
const merged = base
- ? mergeQueuedWizardConversationSnapshots(conversation, base, remote)
+ ? mergeQueuedWizardConversationSnapshots(conversation, base, remote, honorLocalDeletes)
: mergeWizardConversationSnapshots(conversation, remote)
return {
conversation: await transport.save(workspace, merged),
@@ -268,10 +272,17 @@
transport: WizardConversationTransport = defaultTransport,
): Promise<WizardConversationSaveResult> {
const canonical = snapshots.get(write.workspace)
+ const honorLocalDeletes = write.honorLocalDeletes ?? false
const outgoing = canonical
- ? mergeQueuedWizardConversationSnapshots(write.captured, write.base, canonical)
+ ? mergeQueuedWizardConversationSnapshots(write.captured, write.base, canonical, honorLocalDeletes)
: write.captured
- const saved = await saveWizardConversationWithRecovery(write.workspace, outgoing, transport, canonical)
+ const saved = await saveWizardConversationWithRecovery(
+ write.workspace,
+ outgoing,
+ transport,
+ write.base ?? canonical,
+ honorLocalDeletes,
+ )
snapshots.set(write.workspace, saved.conversation)
return saved
}
diff --git a/ui/src/features/agent/wizardConversationSync.ts b/ui/src/features/agent/wizardConversationSync.ts
--- a/ui/src/features/agent/wizardConversationSync.ts
+++ b/ui/src/features/agent/wizardConversationSync.ts
@@ -61,20 +61,21 @@
/**
* 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.
+ * Shared ids keep the local copy so in-flight card and workflow updates are
+ * not replaced by a stale remote snapshot. Remote-only messages stay in
+ * remote order; local-only messages are appended once.
*/
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<string>()
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
@@ -130,7 +131,12 @@
if (localRevision > remoteRevision) {
return { source: 'local', messages: localMessages, revision: localRevision }
}
- return { source: 'remote', messages: remoteMessages, revision: remoteRevision }
+ const localById = new Map(localMessages.map(message => [message.id, message]))
+ return {
+ source: 'remote',
+ messages: remoteMessages.map(message => localById.get(message.id) || message),
+ revision: remoteRevision,
+ }
}
/** Follow the footer workspace only after the in-flight turn finishes. */
diff --git a/ui/tests/wizardConversationPersistence.test.mjs b/ui/tests/wizardConversationPersistence.test.mjs
--- a/ui/tests/wizardConversationPersistence.test.mjs
+++ b/ui/tests/wizardConversationPersistence.test.mjs
@@ -15,8 +15,10 @@
fetchWizardConversation,
} = await import('../src/api/wizard.ts')
const {
+ applyRemoteWizardConversation,
isWizardConversationWriteCurrent,
hasExclusiveWizardMessages,
+ mergeWizardMessages,
shouldFollowWizardWorkspace,
} = await import('../src/features/agent/wizardConversationSync.ts')
@@ -282,6 +284,7 @@
workspace: 'workspace-a',
captured: capturedClear,
base: clearBase,
+ honorLocalDeletes: true,
}, snapshots, transport)
assert.equal(saved.conversation.revision, 3)
@@ -488,6 +491,117 @@
assert.deepEqual(rebased.conversation.messages, confirmed.messages)
})
+test('a windowed follow-up persist keeps recovered turns outside the UI window', async () => {
+ const recoveredIds = Array.from({ length: 40 }, (_, index) => `recovered-${index}`)
+ const visibleIds = Array.from({ length: 40 }, (_, index) => `visible-${index}`)
+ const fullIds = [...recoveredIds, ...visibleIds]
+ const canonical = payload(5, fullIds)
+ const capturedWindow = payload(5, visibleIds)
+ const snapshots = new Map([['workspace-a', clone(canonical)]])
+ const transport = {
+ async fetch() { return clone(canonical) },
+ async save(_workspace, conversation) {
+ assert.deepEqual(conversation.messages.map(message => message.id), fullIds)
+ return { ...clone(conversation), revision: 6 }
+ },
+ }
+
+ const saved = await persistQueuedWizardConversation({
+ workspace: 'workspace-a',
+ captured: capturedWindow,
+ base: canonical,
+ }, snapshots, transport)
+
+ assert.deepEqual(saved.conversation.messages.map(message => message.id), fullIds)
+ assert.equal(hasExclusiveWizardMessages(capturedWindow.messages, saved.conversation.messages), false)
+})
+
+test('a conflict retry after a predecessor clear does not resurrect deleted turns', async () => {
+ const clearBase = payload(1, ['cleared-user', 'cleared-assistant'])
+ const persistStart = payload(2, ['welcome-after-clear'])
+ const capturedClear = payload(1, ['welcome-after-clear'])
+ const snapshots = new Map([['workspace-a', clone(persistStart)]])
+ let canonical = payload(3, ['cleared-user', 'cleared-assistant', 'other-tab-user'])
+ 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.deepEqual(conversation.messages.map(message => message.id), [
+ 'other-tab-user',
+ 'welcome-after-clear',
+ ])
+ canonical = { ...clone(conversation), revision: 4 }
+ return clone(canonical)
+ },
+ }
+
+ const saved = await persistQueuedWizardConversation({
+ workspace: 'workspace-a',
+ captured: capturedClear,
+ base: clearBase,
+ honorLocalDeletes: true,
+ }, snapshots, transport)
+
+ assert.equal(saved.merged, true)
+ assert.equal(saves, 2)
+ assert.deepEqual(saved.conversation.messages.map(message => message.id), [
+ 'other-tab-user',
+ 'welcome-after-clear',
+ ])
+})
+
+test('shared-id merge keeps newer local card and workflow text', () => {
+ const local = [
+ {
+ id: 'shared',
+ role: 'assistant',
+ text: 'workflow updated',
+ createdAt: 1,
+ cards: [{ id: 'card-1', state: 'completed' }],
+ },
+ { id: 'local-only', role: 'user', text: 'new turn', createdAt: 2 },
+ ]
+ const remote = [
+ {
+ id: 'shared',
+ role: 'assistant',
+ text: 'workflow stale',
+ createdAt: 1,
+ cards: [{ id: 'card-1', state: 'running' }],
+ },
+ { id: 'remote-only', role: 'assistant', text: 'from other tab', createdAt: 3 },
+ ]
+
+ const merged = mergeWizardMessages(local, remote)
+ assert.equal(merged.find(message => message.id === 'shared').text, 'workflow updated')
+ assert.equal(merged.find(message => message.id === 'shared').cards[0].state, 'completed')
+ assert.deepEqual(merged.map(message => message.id), ['shared', 'remote-only', 'local-only'])
+
+ const hydrated = applyRemoteWizardConversation({
+ localMessages: [{
+ id: 'shared',
+ role: 'assistant',
+ text: 'workflow updated',
+ createdAt: 1,
+ cards: [{ id: 'card-1', state: 'completed' }],
+ }],
+ localRevision: 4,
+ remoteMessages: [{
+ id: 'shared',
+ role: 'assistant',
+ text: 'workflow stale',
+ createdAt: 1,
+ cards: [{ id: 'card-1', state: 'running' }],
+ }],
+ remoteRevision: 5,
+ })
+ assert.equal(hydrated.source, 'remote')
+ assert.equal(hydrated.messages[0].text, 'workflow updated')
+ assert.equal(hydrated.messages[0].cards[0].state, 'completed')
+})
+
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'])You can send follow-ups to the cloud agent here.
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Clear undone by in-flight save rebase
- Post-save rebase now passes honorLocalDeletes when the write itself was a Clear or a later Clear is still pending, so in-flight persist no longer restores deleted turns.
Or push these changes by commenting:
@cursor push 9b2242297f
Preview (9b2242297f)
diff --git a/ui/src/features/agent/AgentAssistantPanel.tsx b/ui/src/features/agent/AgentAssistantPanel.tsx
--- a/ui/src/features/agent/AgentAssistantPanel.tsx
+++ b/ui/src/features/agent/AgentAssistantPanel.tsx
@@ -272,12 +272,17 @@
if (!mountedRef.current || !isWizardConversationWriteCurrent(conversationWorkspaceRef.current, conversationWorkspace)) return
setConversationSaveError(null)
const visibleMessages = messagesRef.current
+ const pendingClearBase = conversationClearBasesRef.current.get(conversationWorkspace)
const rebased = rebaseStaleWizardConversationHydration({
...queuedWrite.captured,
revision: saved.conversation.revision,
messages: visibleMessages,
executions: visibleMessages.flatMap(message => message.cards || []),
- }, queuedWrite.captured, saved.conversation)
+ }, queuedWrite.captured, saved.conversation, {
+ // A later Clear must not look like a stale-cache omission when this
+ // in-flight save rebases the now-empty thread over its captured turns.
+ honorLocalDeletes: Boolean(queuedWrite.honorLocalDeletes || pendingClearBase),
+ })
if (saved.merged || rebased.needsPersist) {
skipNextConversationSaveRef.current = !rebased.needsPersist
setMessages(normalizeRemoteWizardMessages(You can send follow-ups to the cloud agent here.
Owner
Author
|
@cursor review |
Owner
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
2 issues from previous reviews remain unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c4b627f. Configure here.
21 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
cd ui && npx tsx --tsconfig tsconfig.app.json --import ./tests/setupI18n.ts --test tests/wizardConversationPersistence.test.mjscd ui && npm run checkNo providers, models or generation tokens used. Based on
origin/mainat0d5b077(#121); no backend launcher or hot agent/action files were changed.Note
Medium Risk
Complex client-side merge and CAS logic for durable Wizard state; regressions could drop messages, mishandle clears, or mis-sync across tabs/workspaces, though coverage is extensive in new tests.
Overview
Replaces ad-hoc Wizard conversation saves with serialized, workspace-keyed persistence that handles multi-tab races, in-flight edits, and explicit clears without losing turns or deleting history outside the UI window.
The API layer now throws
WizardConversationRequestErrorwith HTTP status and structured detail so 409 revision conflicts are distinguished from validation/auth failures. Saves run throughsaveWizardConversationWithRecovery(one refetch + merge + retry, no loops on a second 409) and three-way merges when a queued write has a recorded base—including Clear, which alone may honor local deletes.AgentAssistantPaneltracks per-workspace snapshots, chains saves, rebases stale hydration and post-save UI state, delays workspace switches while a turn is busy, and surfacesconversationSaveErrorin the panel. Sync helpers add deterministic message merging and guards for stale async writes.Reviewed by Cursor Bugbot for commit c4b627f. Configure here.