From 579d3575c293eb4a629cb46fd0f2eeaa24e64e65 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 7 Oct 2026 18:33:09 +0530 Subject: [PATCH] fix(desktop): keep the 4033 re-run flag off the public callbacks; an unseen later turn stops an edit Delta review 2 Lows: - The internal answeredFor parameter rode on the public onReload/onEdit callbacks, and assistant-ui calls onReload(parentId, config), so it received the config object on every reload. The 4033 re-run now goes through private runReload/runEdit, and the public callbacks take only their own arguments. - A deep edit re-aimed after the confirm accepted a later user turn that landed during the dialog, which archived a turn the user never answered for. revalidateEditPlan now refuses when the count of later user turns changed. Regenerate already asked again in this case. --- .../src/app/chat/session-tile-actions.ts | 21 +++++++++------- .../session/hooks/use-prompt-actions/index.ts | 21 +++++++++------- .../hooks/use-prompt-actions/rewind.test.ts | 25 +++++++++++++++++++ .../hooks/use-prompt-actions/rewind.ts | 8 +++++- 4 files changed, 56 insertions(+), 19 deletions(-) diff --git a/apps/desktop/src/app/chat/session-tile-actions.ts b/apps/desktop/src/app/chat/session-tile-actions.ts index b54c668219..974f19b6fc 100644 --- a/apps/desktop/src/app/chat/session-tile-actions.ts +++ b/apps/desktop/src/app/chat/session-tile-actions.ts @@ -550,7 +550,8 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored [update] ) - const reloadFromMessage = useCallback( + // answeredFor is internal (the 4033 re-run); assistant-ui calls onReload(parentId, config). + const runReload = useCallback( async (parentId: string | null, answeredFor?: ChatMessage[]): Promise => { const state = readState() const runtimeId = runtimeIdRef.current @@ -609,15 +610,16 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored } else if ((await confirmDeep()) && runtimeIdRef.current === runtimeId) { // Re-run from the top: the session is re-read and re-validated after this wait. The answer // belongs to this runtime: a rebind during the dialog drops it. - await reloadFromMessageRef.current(parentId, messages) + await runReloadRef.current(parentId, messages) } } }, [applySurvivorRowIds, copy.regenerateFailed, readState, submitRewind, t, update] ) - const reloadFromMessageRef = useRef(reloadFromMessage) - reloadFromMessageRef.current = reloadFromMessage + const runReloadRef = useRef(runReload) + runReloadRef.current = runReload + const reloadFromMessage = useCallback((parentId: string | null) => runReload(parentId), [runReload]) const restoreToMessage = useCallback( async (messageId: string, target?: { text?: string; userOrdinal?: number | null }) => { @@ -708,7 +710,7 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored [applySurvivorRowIds, readMessages, readState, submitRewind, update] ) - const editMessage = useCallback( + const runEdit = useCallback( async (edited: AppendMessage, answeredFor?: ChatMessage[]): Promise => { const planning = readMessages() const confirmDeep = () => confirm(deepCutConfirmRequest(t.assistant.thread)) @@ -722,7 +724,7 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored // Same re-aim as the primary edit: a stream may have grown the transcript during the confirm. const messages = forced.confirmDeepTruncate ? readMessages() : planning - const plan = messages === planning ? forced : revalidateEditPlan(forced, messages, edited) + const plan = messages === planning ? forced : revalidateEditPlan(forced, planning, messages, edited) if (!plan) { // The edited turn moved while the confirm was open: nothing was sent. @@ -772,15 +774,16 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored } else if ((await confirmDeep()) && runtimeIdRef.current === sessionId) { // Re-run from the top: the session is re-read and re-validated after this wait. The answer // belongs to this runtime: a rebind during the dialog drops it. - await editMessageRef.current(edited, planning) + await runEditRef.current(edited, planning) } } }, [applySurvivorRowIds, copy.editFailed, readMessages, readState, submitRewind, t, update] ) - const editMessageRef = useRef(editMessage) - editMessageRef.current = editMessage + const runEditRef = useRef(runEdit) + runEditRef.current = runEdit + const editMessage = useCallback((edited: AppendMessage) => runEdit(edited), [runEdit]) // Branch-visibility sync (assistant-ui hides non-active branches). const handleThreadMessagesChange = useCallback( diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index 1cadc71ad9..dbf04a16ad 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -1002,7 +1002,8 @@ export function usePromptActions({ [activeSessionIdRef, requestGateway, selectedStoredSessionIdRef] ) - const reloadFromMessage = useCallback( + // answeredFor is internal (the 4033 re-run); assistant-ui calls onReload(parentId, config). + const runReload = useCallback( async (parentId: string | null, answeredFor?: ChatMessage[]): Promise => { // Ref, not the closure-captured prop — a truncating resubmit aimed at a // stale session deletes the wrong transcript. @@ -1062,15 +1063,16 @@ export function usePromptActions({ } else if ((await confirmDeep()) && activeSessionIdRef.current === sessionId) { // Re-run from the top: the session is re-read and re-validated after this wait. The answer // belongs to this session: a switch during the dialog drops it. - await reloadFromMessageRef.current(parentId, messages) + await runReloadRef.current(parentId, messages) } } }, [activeSessionIdRef, applySurvivorRowIds, copy.regenerateFailed, submitRewindPrompt, t, updateSessionState] ) - const reloadFromMessageRef = useRef(reloadFromMessage) - reloadFromMessageRef.current = reloadFromMessage + const runReloadRef = useRef(runReload) + runReloadRef.current = runReload + const reloadFromMessage = useCallback((parentId: string | null) => runReload(parentId), [runReload]) // Cursor-style "restore checkpoint": rewind the conversation to a past user // prompt and run it again from there. Reuses the edit composer's rewind @@ -1204,7 +1206,7 @@ export function usePromptActions({ ] ) - const editMessage = useCallback( + const runEdit = useCallback( async (edited: AppendMessage, answeredFor?: ChatMessage[]): Promise => { // Ref, not the closure-captured prop — an edit rewinds and resubmits, so // a stale target rewrites the wrong session's history. @@ -1230,7 +1232,7 @@ export function usePromptActions({ // re-aimed at the current transcript (a stream may have grown it) and dropped, with a notice, // only if the edited turn itself moved. const messages = forced.confirmDeepTruncate ? currentMessages(sessionId) : planning - const plan = messages === planning ? forced : revalidateEditPlan(forced, messages, edited) + const plan = messages === planning ? forced : revalidateEditPlan(forced, planning, messages, edited) if (!plan) { // The edited turn moved while the confirm was open: nothing was sent. @@ -1357,7 +1359,7 @@ export function usePromptActions({ } else if ((await confirmDeep()) && activeSessionIdRef.current === sessionId) { // Re-run from the top: the session is re-read and re-validated after this wait. The answer // belongs to this session: a switch during the dialog drops it. - await editMessageRef.current(edited, planning) + await runEditRef.current(edited, planning) } } }, @@ -1375,8 +1377,9 @@ export function usePromptActions({ ] ) - const editMessageRef = useRef(editMessage) - editMessageRef.current = editMessage + const runEditRef = useRef(runEdit) + runEditRef.current = runEdit + const editMessage = useCallback((edited: AppendMessage) => runEdit(edited), [runEdit]) const handleThreadMessagesChange = useCallback( (nextMessages: readonly ThreadMessage[]) => { diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/rewind.test.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/rewind.test.ts index 30b5d038f0..5ab4882d7d 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/rewind.test.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/rewind.test.ts @@ -16,6 +16,7 @@ import { planRestore, rebindSurvivorRowIds, resolveDurableRowId, + revalidateEditPlan, runRewindSubmit, survivorRowIdsFrom, truncateSubmitParams @@ -808,3 +809,27 @@ describe('edit and server-refusal confirms (#133716 review)', () => { expect(asked).toBe(1) }) }) + +describe('revalidateEditPlan (#133716)', () => { + const planned = [ + row('u1', 'user', 'old prompt', { rowId: 11 }), + row('a1', 'assistant', 'old reply'), + row('u2', 'user', 'latest prompt', { rowId: 13 }), + row('a2', 'assistant', 'latest reply') + ] + + const edit = { role: 'user', sourceId: 'u1', parentId: null, content: [{ type: 'text', text: 'edited' }] } as never + + it('follows a transcript a stream grew; refuses one that gained a later user turn', () => { + const plan = { ...planEdit(planned, edit)!, confirmDeepTruncate: true } + const streamed = [...planned, row('a2b', 'assistant', 'more')] + + expect(revalidateEditPlan(plan, planned, streamed, edit)).toMatchObject({ + confirmDeepTruncate: true, + truncateRowId: 11 + }) + expect( + revalidateEditPlan(plan, planned, [...streamed, row('u3', 'user', 'unseen', { rowId: 15 })], edit) + ).toBeNull() + }) +}) diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/rewind.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/rewind.ts index 18534ca1de..7eb0518003 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/rewind.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/rewind.ts @@ -747,16 +747,22 @@ export async function planConfirmedEdit( */ export function revalidateEditPlan

( plan: P, + planned: ChatMessage[], current: ChatMessage[], edited: AppendMessage ): P | null { const fresh = planEdit(current, edited) + const laterUsers = (messages: ChatMessage[], index: number) => + messages.filter((m, i) => i > index && isVisibleUserMessage(m)).length + + // A new later user turn landed during the confirm: the user never answered for archiving it. if ( !fresh || fresh.truncateMessageId !== plan.truncateMessageId || fresh.truncateRowId !== plan.truncateRowId || - fresh.truncateOrdinal !== plan.truncateOrdinal + fresh.truncateOrdinal !== plan.truncateOrdinal || + laterUsers(current, fresh.sourceIndex) !== laterUsers(planned, plan.sourceIndex) ) { return null }