fix: re-aim a deep edit confirmed mid-stream; count an unstamped carrier's own turn
Delta review findings: - Medium: a confirmed deep edit during a live turn was silently dropped, because the stale check compared the transcript array by identity and streaming replaces it on every delta. The edit now re-plans against the current transcript (revalidateEditPlan). It proceeds when the edited turn keeps its address and the re-planned index is applied. If the turn moved, it stops with an "Edit failed" notice instead of returning silently. Applies to the primary chat and tiles. - Low: a live carrier with no row id of its own never counted its own user turn, so a two-turn cut could pass without 4033. It now counts (fail closed).
This commit is contained in:
@@ -59,6 +59,7 @@ import {
|
||||
planConfirmedReload,
|
||||
planRestore,
|
||||
rebindSurvivorRowIds,
|
||||
revalidateEditPlan,
|
||||
runRewindSubmit,
|
||||
type SurvivorUserRowIds
|
||||
} from '../session/hooks/use-prompt-actions/rewind'
|
||||
@@ -707,15 +708,24 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored
|
||||
|
||||
const editMessage = useCallback(
|
||||
async (edited: AppendMessage, answeredFor?: ChatMessage[]): Promise<void> => {
|
||||
const messages = readMessages()
|
||||
const planning = readMessages()
|
||||
const confirmDeep = () => confirm(deepCutConfirmRequest(t.assistant.thread))
|
||||
const forceDeep = answeredFor !== undefined && answeredFor === messages
|
||||
const planned = await planConfirmedEdit(messages, edited, forceDeep ? async () => true : confirmDeep)
|
||||
const plan = planned && forceDeep ? { ...planned, confirmDeepTruncate: true } : planned
|
||||
const forceDeep = answeredFor !== undefined && answeredFor === planning
|
||||
const planned = await planConfirmedEdit(planning, edited, forceDeep ? async () => true : confirmDeep)
|
||||
const forced = planned && forceDeep ? { ...planned, confirmDeepTruncate: true } : planned
|
||||
|
||||
if (!forced) {
|
||||
return
|
||||
}
|
||||
|
||||
// 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)
|
||||
|
||||
if (!plan) {
|
||||
// The edited turn moved while the confirm was open: nothing was sent.
|
||||
notify({ kind: 'warning', message: copy.editFailed })
|
||||
|
||||
// Edit interrupts a live turn on purpose; only output that landed during the confirm makes
|
||||
// the plan's index and rollback snapshot stale.
|
||||
if (!plan || (plan.confirmDeepTruncate && readMessages() !== messages)) {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -759,7 +769,7 @@ export function useSessionTileActions({ requestGateway, runtimeId, scope, stored
|
||||
notifyError(err, copy.editFailed)
|
||||
} else if (await confirmDeep()) {
|
||||
// Re-run from the top: the session is re-read and re-validated after this wait.
|
||||
await editMessageRef.current(edited, messages)
|
||||
await editMessageRef.current(edited, planning)
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -6200,6 +6200,67 @@ describe('usePromptActions reloadFromMessage failed-submit rollback (#95745)', (
|
||||
})
|
||||
})
|
||||
|
||||
describe('usePromptActions deep edit confirmed while output streams (#133716)', () => {
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
clearNotifications()
|
||||
setMessages([])
|
||||
$busy.set(false)
|
||||
})
|
||||
|
||||
it('re-aims a confirmed deep edit at the grown transcript instead of dropping it', async () => {
|
||||
const seed = [
|
||||
{ id: 'u1', parts: [textPart('first')], role: 'user' as const, rowId: 11, timestamp: 0 },
|
||||
{ id: 'a1', parts: [textPart('reply')], role: 'assistant' as const, timestamp: 1 },
|
||||
{ id: 'u2', parts: [textPart('later')], role: 'user' as const, rowId: 13, timestamp: 2 },
|
||||
{ id: 'a2', parts: [textPart('later reply')], role: 'assistant' as const, timestamp: 3 }
|
||||
]
|
||||
|
||||
setMessages(seed as never)
|
||||
const submits: Record<string, unknown>[] = []
|
||||
|
||||
const requestGateway = vi.fn(async (method: string, params?: Record<string, unknown>) => {
|
||||
if (method === 'prompt.submit') {
|
||||
submits.push(params ?? {})
|
||||
}
|
||||
|
||||
return {} as never
|
||||
})
|
||||
|
||||
let handle: HarnessHandle | undefined
|
||||
|
||||
await actRender(
|
||||
<Harness
|
||||
onReady={h => {
|
||||
handle = h
|
||||
}}
|
||||
refreshSessions={async () => undefined}
|
||||
requestGateway={requestGateway}
|
||||
seedMessages={seed}
|
||||
/>
|
||||
)
|
||||
|
||||
// A stream delta replaces the transcript array while the user reads the dialog.
|
||||
const stopConfirming = $confirmRequest.listen(request => {
|
||||
if (request) {
|
||||
setMessages([...seed, { id: 'a2b', parts: [textPart('more')], role: 'assistant', timestamp: 4 }] as never)
|
||||
settleConfirm(true)
|
||||
}
|
||||
})
|
||||
|
||||
await handle!.editMessage({
|
||||
content: [{ text: 'edited first', type: 'text' }],
|
||||
parentId: null,
|
||||
role: 'user',
|
||||
sourceId: 'u1'
|
||||
} as never)
|
||||
stopConfirming()
|
||||
|
||||
expect(submits).toHaveLength(1)
|
||||
expect(submits[0]).toMatchObject({ confirm_deep_truncate: true, truncate_before_row_id: 11, text: 'edited first' })
|
||||
})
|
||||
})
|
||||
|
||||
describe('usePromptActions live-owner refusal (#106217)', () => {
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
|
||||
@@ -69,6 +69,7 @@ import {
|
||||
planEdit,
|
||||
planRestore,
|
||||
rebindSurvivorRowIds,
|
||||
revalidateEditPlan,
|
||||
runRewindSubmit,
|
||||
type SurvivorUserRowIds
|
||||
} from './rewind'
|
||||
@@ -1209,20 +1210,31 @@ export function usePromptActions({
|
||||
const sessionId = activeSessionIdRef.current
|
||||
|
||||
// Same dual-store read as reloadFromMessage (#68734).
|
||||
const messages = (sessionId ? $sessionStates.get()[sessionId]?.messages : null) ?? $messages.get()
|
||||
const planning = (sessionId ? $sessionStates.get()[sessionId]?.messages : null) ?? $messages.get()
|
||||
|
||||
const confirmDeep = () => confirm(deepCutConfirmRequest(t.assistant.thread))
|
||||
const forceDeep = answeredFor !== undefined && answeredFor === messages
|
||||
const forceDeep = answeredFor !== undefined && answeredFor === planning
|
||||
|
||||
const planned = sessionId
|
||||
? await planConfirmedEdit(messages, edited, forceDeep ? async () => true : confirmDeep)
|
||||
? await planConfirmedEdit(planning, edited, forceDeep ? async () => true : confirmDeep)
|
||||
: null
|
||||
|
||||
const plan = planned && forceDeep ? { ...planned, confirmDeepTruncate: true } : planned
|
||||
const forced = planned && forceDeep ? { ...planned, confirmDeepTruncate: true } : planned
|
||||
|
||||
if (!sessionId || !forced) {
|
||||
return
|
||||
}
|
||||
|
||||
// Edit interrupts a live turn on purpose, so busy is fine; after a confirm wait the plan is
|
||||
// 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)
|
||||
|
||||
if (!plan) {
|
||||
// The edited turn moved while the confirm was open: nothing was sent.
|
||||
notify({ kind: 'warning', message: copy.editFailed })
|
||||
|
||||
// Edit interrupts a live turn on purpose; only output that landed during the confirm makes
|
||||
// the plan's index and rollback snapshot stale.
|
||||
if (!sessionId || !plan || (plan.confirmDeepTruncate && currentMessages(sessionId) !== messages)) {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -1343,7 +1355,7 @@ export function usePromptActions({
|
||||
notifyError(surfaced, unavailable ? copy.editTurnUnavailable : copy.editFailed)
|
||||
} else if (await confirmDeep()) {
|
||||
// Re-run from the top: the session is re-read and re-validated after this wait.
|
||||
await editMessageRef.current(edited, messages)
|
||||
await editMessageRef.current(edited, planning)
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -740,6 +740,30 @@ export async function planConfirmedEdit(
|
||||
return plan && confirmIfDeepCut(messages, plan, plan.sourceIndex, confirmDeep)
|
||||
}
|
||||
|
||||
/**
|
||||
* After an awaited confirm, an edit plan still holds when the same turn is the target in the
|
||||
* current transcript. Streaming replaces the array on every delta, so identity is not the test:
|
||||
* re-plan and compare the turn's address. The re-planned index is the one to apply.
|
||||
*/
|
||||
export function revalidateEditPlan<P extends EditPlan>(
|
||||
plan: P,
|
||||
current: ChatMessage[],
|
||||
edited: AppendMessage
|
||||
): P | null {
|
||||
const fresh = planEdit(current, edited)
|
||||
|
||||
if (
|
||||
!fresh ||
|
||||
fresh.truncateMessageId !== plan.truncateMessageId ||
|
||||
fresh.truncateRowId !== plan.truncateRowId ||
|
||||
fresh.truncateOrdinal !== plan.truncateOrdinal
|
||||
) {
|
||||
return null
|
||||
}
|
||||
|
||||
return { ...plan, ...fresh, confirmDeepTruncate: (plan as P & { confirmDeepTruncate?: boolean }).confirmDeepTruncate }
|
||||
}
|
||||
|
||||
/** Optimistic rewind-to state for restore/edit: drop everything after the
|
||||
* source turn (edit swaps in the edited message; restore keeps the original). */
|
||||
export function applyRewindOptimistic(
|
||||
|
||||
@@ -95,6 +95,7 @@ def test_deep_truncate_counts_every_durable_row_a_merged_carrier_holds(monkeypat
|
||||
assert refused["error"]["code"] == 4033
|
||||
assert refused["error"]["data"]["archived_user_turns"] == 3
|
||||
assert sess["history"] == history and replaced == []
|
||||
|
||||
finally:
|
||||
server._sessions.pop("deep-carrier-sid", None)
|
||||
|
||||
@@ -180,3 +181,20 @@ def test_prompt_submit_empty_truncation_allowed_with_confirm(monkeypatch):
|
||||
]
|
||||
finally:
|
||||
server._sessions.pop("confirm-empty-sid", None)
|
||||
|
||||
|
||||
def test_archived_user_turns_counts_a_carrier_without_its_own_row_id():
|
||||
"""An unstamped carrier's own turn has no id to look up; it still counts (fail closed)."""
|
||||
from tui_gateway import methods_prompt
|
||||
|
||||
history = [{"role": "user", "content": "x"}, {"role": "assistant", "content": "r"},
|
||||
{"_absorbed_row_ids": [102], "role": "user", "content": "b\n\nc"}]
|
||||
sess = _session(history=history)
|
||||
physical = {"rows": [{"_row_id": 102, "role": "user", "content": "c"}]}
|
||||
original = methods_prompt._load_durable_truncation_history
|
||||
methods_prompt._load_durable_truncation_history = lambda *a, **k: physical["rows"]
|
||||
try:
|
||||
assert methods_prompt._archived_user_turns(sess, "sid", history, 2, set()) == 2
|
||||
finally:
|
||||
methods_prompt._load_durable_truncation_history = original
|
||||
|
||||
|
||||
@@ -456,7 +456,10 @@ def _archived_user_turns(session, sid, history, cut_index, survivor_ids) -> int:
|
||||
if physical is None:
|
||||
rows = _load_durable_truncation_history(session, sid, repair_alternation=False) or []
|
||||
physical = {_message_row_id(row): row for row in rows if isinstance(row, dict)}
|
||||
for rid in {_message_row_id(message), *absorbed} - survivor_ids - {None}:
|
||||
own = _message_row_id(message)
|
||||
# A carrier with no id of its own still holds its own (unaddressable) user turn.
|
||||
archived += own is None
|
||||
for rid in {own, *absorbed} - survivor_ids - {None}:
|
||||
row = physical.get(rid)
|
||||
archived += row is None or user_originated_turn_view(row) is not None
|
||||
return archived
|
||||
|
||||
Reference in New Issue
Block a user