diff --git a/client/src/components/Chat/Input/QueuedOutbox.tsx b/client/src/components/Chat/Input/QueuedOutbox.tsx index d6067db949..228a54814b 100644 --- a/client/src/components/Chat/Input/QueuedOutbox.tsx +++ b/client/src/components/Chat/Input/QueuedOutbox.tsx @@ -350,13 +350,20 @@ function QueuedRowBase({ const remove = () => { /* Same safety net as the in-flight cancel: once removal is safely * settled, return the words to the composer when it is free (the - * gated restore refuses rather than clobber a draft). */ - onRestoreToComposer( - message.text, - message.files, - { quotes: message.quotes, manualSkills: message.manualSkills }, - conversationId, - ); + * gated restore refuses rather than clobber a draft). + * + * Skipped when the editor was left empty: the queue still holds the + * pre-edit words, so handing them back would resurrect text the + * user had visibly deleted. Emptying a row and then removing it + * reads as "delete this", and there is nothing to return. */ + if (!emptyEdit) { + onRestoreToComposer( + message.text, + message.files, + { quotes: message.quotes, manualSkills: message.manualSkills }, + conversationId, + ); + } steering.removeQueued(message.id); return true; }; @@ -511,20 +518,12 @@ function QueuedOutboxBase({ ); /** The gated restore refuses rather than clobber a draft the user has * since staged. Hand the words back to the queue instead of dropping - * them — one row now, since they were folded on the way out. */ + * them — one row now, since they were folded on the way out. The ITEM + * goes back whole: rebuilding one from parts dropped its predecessor + * fence once and its interrupt tier once, and the next field would be + * just as quiet. */ if (!restored) { - steering.enqueue(cleared.text, { - files: cleared.files, - quotes: cleared.quotes, - manualSkills: cleared.manualSkills, - skipUsageMark: true, - id: cleared.id, - createdAt: cleared.createdAt, - /** Kept explicitly: an idle chat has no active epoch for `enqueue` to - * substitute, and an unfenced row can replace a newer generation - * instead of taking the predecessor-mismatch recovery path. */ - expectedPredecessorCreatedAt: cleared.expectedPredecessorCreatedAt, - }); + steering.requeueCleared([cleared]); showToast({ message: localize('com_ui_steer_edit_queued'), status: 'info' }); } })(); diff --git a/client/src/components/Chat/Input/__tests__/PendingSteerChips.test.tsx b/client/src/components/Chat/Input/__tests__/PendingSteerChips.test.tsx index 4847660282..da8f6c15c1 100644 --- a/client/src/components/Chat/Input/__tests__/PendingSteerChips.test.tsx +++ b/client/src/components/Chat/Input/__tests__/PendingSteerChips.test.tsx @@ -568,6 +568,7 @@ const mockUpdateQueuedText = jest.fn(() => true); const mockMergeQueued = jest.fn(() => true); const mockCancelQueueDrain = jest.fn(); const mockEnqueue = jest.fn(); +const mockRequeueCleared = jest.fn(); let mockClearQueued = jest.fn(async (): Promise => null); const outboxSteering = (overrides: Partial = {}) => ({ @@ -577,6 +578,7 @@ const outboxSteering = (overrides: Partial = {}) => ({ clearQueued: mockClearQueued, cancelQueueDrain: mockCancelQueueDrain, enqueue: mockEnqueue, + requeueCleared: mockRequeueCleared, ...overrides, }); @@ -813,6 +815,34 @@ describe('PendingSteerChips — queued outbox group', () => { expect(screen.getByTestId('queue-merge')).not.toBeDisabled(); }); + /** The queue still holds the pre-edit words, so handing them back would + * resurrect text the user visibly deleted. Emptying a row and removing it + * reads as "delete this". */ + it('does not return stale words to the composer when removing an emptied row', () => { + renderChips([twoQueued[0]], { steering: outboxSteering() }); + + fireEvent.click(screen.getByText('first thought')); + fireEvent.change(screen.getByTestId('queued-message-edit'), { target: { value: '' } }); + fireEvent.click(screen.getByLabelText('com_ui_remove_queued')); + + expect(mockRestoreToComposer).not.toHaveBeenCalled(); + expect(mockRemoveQueued).toHaveBeenCalledWith('q1'); + }); + + it('still returns the words when removing a row that was not being emptied', () => { + renderChips([twoQueued[0]], { steering: outboxSteering() }); + + fireEvent.click(screen.getByLabelText('com_ui_remove_queued')); + + expect(mockRestoreToComposer).toHaveBeenCalledWith( + 'first thought', + undefined, + { quotes: undefined, manualSkills: undefined }, + CONVO_ID, + ); + expect(mockRemoveQueued).toHaveBeenCalledWith('q1'); + }); + /** Clear all folds the queue exactly as Merge does, so it takes the same * standdown rather than being a documented exception. */ it('refuses to clear all while an inline edit is empty', () => { @@ -904,6 +934,8 @@ describe('PendingSteerChips — queued outbox group', () => { text: 'not lost', createdAt: 1, expectedPredecessorCreatedAt: 4242, + priority: true, + bumpedAt: 99, }; mockClearQueued = jest.fn(async () => folded); mockRestoreToComposer.mockReturnValueOnce(false); @@ -912,18 +944,12 @@ describe('PendingSteerChips — queued outbox group', () => { fireEvent.click(screen.getByTestId('queue-clear-all')); await waitFor(() => { - expect(mockEnqueue).toHaveBeenCalledWith( - 'not lost', - expect.objectContaining({ - id: 'q1', - createdAt: 1, - skipUsageMark: true, - /** An idle chat has no active epoch to substitute, so an unfenced - * requeue could replace a newer generation from another tab. */ - expectedPredecessorCreatedAt: 4242, - }), - ); + /** The ITEM goes back whole, so no field can be quietly dropped — the + * fence and the interrupt tier were each lost once when this path + * rebuilt a row from parts. */ + expect(mockRequeueCleared).toHaveBeenCalledWith([folded]); }); + expect(mockEnqueue).not.toHaveBeenCalled(); expect(mockShowToast).toHaveBeenCalledWith( expect.objectContaining({ message: 'com_ui_steer_edit_queued' }), ); diff --git a/client/src/hooks/Chat/useSteering.ts b/client/src/hooks/Chat/useSteering.ts index 21205e4d42..12d01c8790 100644 --- a/client/src/hooks/Chat/useSteering.ts +++ b/client/src/hooks/Chat/useSteering.ts @@ -784,7 +784,13 @@ export default function useSteering({ [queueKey], ); - /** Puts rows back without clobbering anything queued in the meantime. */ + /** + * Puts rows back without clobbering anything queued in the meantime. Also the + * public requeue path for a cleared payload the composer refused: passing the + * ITEM back preserves every field it carries, where rebuilding one from parts + * has twice now lost a field that mattered (the predecessor fence, then the + * interrupt tier). + */ const restoreQueuedBatch = useRecoilCallback( ({ set }) => (items: QueuedMessage[]) => { @@ -1647,6 +1653,7 @@ export default function useSteering({ convertSteerToQueue, queueReclaimedSteer, enqueue, + requeueCleared: restoreQueuedBatch, removeQueued, discardQueued, bumpQueued, @@ -1678,6 +1685,7 @@ export default function useSteering({ convertSteerToQueue, queueReclaimedSteer, enqueue, + restoreQueuedBatch, removeQueued, discardQueued, bumpQueued,