From 9d566cfb2e042f23b95e6d32b7717267bef70d08 Mon Sep 17 00:00:00 2001
From: Danny Avila <danny@librechat.ai>
Date: Tue, 4 Aug 2026 07:49:36 -0400
Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=9F=20fix:=20Address=20Codex=20Round?=
 =?UTF-8?q?=2012=20On=20The=20Queued=20Outbox?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Four P2 findings, all valid.

A recovered row now leaves the queue for its cancellation round trip. It
stayed put through that await, so a run completing mid-flight could drain
and send the very message being removed — the hazard clear-all already
avoided by reserving up front. Refused cancellation returns the row to its
original slot untouched; a settled one returns it already downgraded, which
retires `downgradeQueuedRecovery` and keeps the public contract unchanged.

Clear all takes the empty-edit standdown. I had argued this one as a
deliberate exception, since its payload lands in the composer where a
reappearing word is visible rather than silently sent. Codex pressed, and
uniformity across every reader of the queue is easier to reason about than
an exception worth remembering.

The expanded rows scroll. The composer box is `overflow-hidden`, so a deep
queue clipped rows with no way to reach them; the cap is on the ROWS alone,
leaving the disclosure and the actions outside it. The scroll container is
named by its own count rather than repeating the outer stack's label, which
would have nested two identically named lists.

An already-cancelled tombstone survives a second queue action. Retiring
twice turned it back to null and exposed the parked epoch it existed to
neutralize.
---
 .../components/Chat/Input/QueuedOutbox.tsx    | 17 +++-
 .../__tests__/PendingSteerChips.test.tsx      | 40 ++++++++
 .../hooks/Chat/__tests__/useSteering.spec.tsx | 97 +++++++++++++++++++
 client/src/hooks/Chat/useSteering.ts          | 81 ++++++++--------
 4 files changed, 192 insertions(+), 43 deletions(-)

diff --git a/client/src/components/Chat/Input/QueuedOutbox.tsx b/client/src/components/Chat/Input/QueuedOutbox.tsx
index 5af6b61880..d6067db949 100644
--- a/client/src/components/Chat/Input/QueuedOutbox.tsx
+++ b/client/src/components/Chat/Input/QueuedOutbox.tsx
@@ -467,6 +467,10 @@ function QueuedOutboxBase({
     }
     return emptyEditId != null ? localize('com_ui_queue_edit_empty') : undefined;
   })();
+  /** Clear all folds the queue the same way Merge does, so an unresolved empty
+   *  edit would hand words the user deleted back to the composer. Uniform with
+   *  every other reader rather than an exception worth remembering. */
+  const clearBlockedReason = emptyEditId != null ? localize('com_ui_queue_edit_empty') : undefined;
   /** The shortcut's promise is the NEWEST waiting message, which is not the
    *  last array slot once a promotion has reordered the queue — so pick by
    *  stamp. Recovery-bound rows are skipped because steering refuses them
@@ -572,8 +576,15 @@ function QueuedOutboxBase({
       {expanded && (
         <div
           role="list"
-          aria-label={localize('com_ui_queued_messages')}
-          className="flex flex-col gap-1.5"
+          /* Named by its own count rather than repeating the outer stack's
+           * label, which would nest two identically named lists. */
+          aria-label={localize('com_ui_queue_count', { 0: String(queued.length) })}
+          data-testid="queue-rows"
+          /* The composer box is `overflow-hidden`, so a deep queue would clip
+           * rows with no way to reach them. Cap the ROWS only: the disclosure
+           * above and the actions below must stay put. Same ceiling the
+           * in-flight overlay uses. */
+          className="flex max-h-[35vh] flex-col gap-1.5 overflow-y-auto"
         >
           {queued.map((message, position) => (
             <QueuedRow
@@ -610,6 +621,8 @@ function QueuedOutboxBase({
             type="button"
             className={PRIMARY_BTN_CLASS}
             data-testid="queue-clear-all"
+            disabled={clearBlockedReason != null}
+            title={clearBlockedReason}
             onClick={clearAll}
           >
             <Trash2 className="h-4 w-4" aria-hidden="true" />
diff --git a/client/src/components/Chat/Input/__tests__/PendingSteerChips.test.tsx b/client/src/components/Chat/Input/__tests__/PendingSteerChips.test.tsx
index b77080c0a1..4847660282 100644
--- a/client/src/components/Chat/Input/__tests__/PendingSteerChips.test.tsx
+++ b/client/src/components/Chat/Input/__tests__/PendingSteerChips.test.tsx
@@ -813,6 +813,46 @@ describe('PendingSteerChips — queued outbox group', () => {
     expect(screen.getByTestId('queue-merge')).not.toBeDisabled();
   });
 
+  /** 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', () => {
+    renderChips(twoQueued, { steering: outboxSteering() });
+    fireEvent.click(screen.getByTestId('queue-group-toggle'));
+
+    const rows = screen.getAllByTestId('queued-message-row');
+    fireEvent.click(rows[1].querySelector('span[title]') as HTMLElement);
+    fireEvent.change(screen.getByTestId('queued-message-edit'), { target: { value: '' } });
+
+    const clear = screen.getByTestId('queue-clear-all');
+    expect(clear).toBeDisabled();
+    fireEvent.click(clear);
+    expect(mockClearQueued).not.toHaveBeenCalled();
+
+    fireEvent.change(screen.getByTestId('queued-message-edit'), { target: { value: 'kept' } });
+    expect(screen.getByTestId('queue-clear-all')).not.toBeDisabled();
+  });
+
+  /** The composer box clips, so a deep queue needs its own scroll — and the
+   *  disclosure and the actions must stay outside it to remain reachable. */
+  it('scrolls the expanded rows without clipping the header or the actions', () => {
+    renderChips(
+      Array.from({ length: 8 }, (_, i) => ({ id: `q${i}`, text: `queued ${i}`, createdAt: i })),
+      { steering: outboxSteering() },
+    );
+    fireEvent.click(screen.getByTestId('queue-group-toggle'));
+
+    const list = screen.getByTestId('queue-rows');
+    // Named by its own count, so it does not duplicate the outer stack's label.
+    expect(list).toHaveAttribute('role', 'list');
+    expect(list).toHaveAccessibleName('com_ui_queue_count');
+    expect(list.className).toContain('overflow-y-auto');
+    expect(list.className).toContain('max-h-[35vh]');
+    // Outside the scroll container, so they cannot be clipped away.
+    expect(list).not.toContainElement(screen.getByTestId('queue-group-toggle'));
+    expect(list).not.toContainElement(screen.getByTestId('queue-merge'));
+    expect(list).not.toContainElement(screen.getByTestId('queue-clear-all'));
+  });
+
   /** Collapsing unmounts the rows, so the disclosure's own text is the queue's
    *  only description — an aria-label would overwrite it. */
   it('announces the count and next-up preview as the disclosure name', () => {
diff --git a/client/src/hooks/Chat/__tests__/useSteering.spec.tsx b/client/src/hooks/Chat/__tests__/useSteering.spec.tsx
index 599cef2925..d90f017fcf 100644
--- a/client/src/hooks/Chat/__tests__/useSteering.spec.tsx
+++ b/client/src/hooks/Chat/__tests__/useSteering.spec.tsx
@@ -2551,6 +2551,103 @@ describe('useSteering — clear all', () => {
     expect(result.current.queue).toEqual([]);
   });
 
+  /** The row must be OUT of the queue for the cancellation round trip: a run
+   *  completing mid-flight would otherwise drain and send the very message the
+   *  user is removing. */
+  it('holds a recovered row out of the queue while its source cancels', async () => {
+    let settleCancel: ((value: { removed: boolean }) => void) | undefined;
+    mockCancelSteer.mockImplementationOnce(
+      () =>
+        new Promise<{ removed: boolean }>((resolve) => {
+          settleCancel = resolve;
+        }),
+    );
+
+    const recovered = {
+      id: 'q1',
+      text: 'being removed',
+      createdAt: 1,
+      recoverySteerId: 'server-source',
+      recoveryClientSteerId: 'client-source',
+    };
+    const { result } = setupClearAll(({ set }) => {
+      set(store.queuedMessagesByConvoId(CONVO_ID), [recovered]);
+    });
+
+    let pending: Promise<boolean> | undefined;
+    act(() => {
+      pending = result.current.steering.discardQueued(recovered);
+    });
+
+    // Nothing for a drain to find while the receipt is in flight.
+    expect(result.current.queue).toEqual([]);
+
+    let settled = false;
+    await act(async () => {
+      settleCancel?.({ removed: true });
+      settled = (await pending) ?? false;
+    });
+
+    expect(settled).toBe(true);
+    // Back in its slot, downgraded: the parked copy is gone, so it is an
+    // ordinary local row the caller can edit or remove.
+    expect(result.current.queue).toEqual([
+      expect.objectContaining({ id: 'q1', text: 'being removed' }),
+    ]);
+    expect(result.current.queue[0].recoverySteerId).toBeUndefined();
+    expect(result.current.queue[0].recoveryClientSteerId).toBeUndefined();
+  });
+
+  it('returns a recovered row untouched when its source refuses to cancel', async () => {
+    mockCancelSteer.mockResolvedValueOnce({ removed: false, generationProtocolVersion: 2 });
+    const recovered = {
+      id: 'q1',
+      text: 'stays put',
+      createdAt: 1,
+      recoverySteerId: 'server-source',
+      recoveryClientSteerId: 'client-source',
+    };
+    const { result } = setupClearAll(({ set }) => {
+      set(store.queuedMessagesByConvoId(CONVO_ID), [recovered]);
+    });
+
+    let settled = true;
+    await act(async () => {
+      settled = await result.current.steering.discardQueued(recovered);
+    });
+
+    expect(settled).toBe(false);
+    expect(result.current.queue).toEqual([
+      expect.objectContaining({ id: 'q1', recoverySteerId: 'server-source' }),
+    ]);
+  });
+
+  /** Retiring twice must not resurrect a parked epoch: the second call has to
+   *  leave an existing tombstone standing. */
+  it('keeps a cancelled tombstone across a second queue action', () => {
+    const { result } = setupClearAll(({ set }) => {
+      set(store.queuedMessagesByConvoId(CONVO_ID), [
+        { id: 'q1', text: 'first', createdAt: 1 },
+        { id: 'q2', text: 'second', createdAt: 2 },
+      ]);
+      set(store.queueDrainHoldByConvoId(CONVO_ID), {
+        runEnd: { conversationId: CONVO_ID, outcome: 'completed', endedAt: 1 },
+        dueAt: 1,
+        status: 'cancelled',
+      });
+    });
+
+    act(() => {
+      result.current.steering.removeQueued('q1');
+    });
+    expect(result.current.drainHold).toMatchObject({ status: 'cancelled' });
+
+    act(() => {
+      result.current.steering.removeQueued('q2');
+    });
+    expect(result.current.drainHold).toMatchObject({ status: 'cancelled' });
+  });
+
   /** A standing window would otherwise fire on whatever the fallbacks put back
    *  — sending exactly what the user just cleared. */
   it('cancels a pending automatic send as part of clearing', async () => {
diff --git a/client/src/hooks/Chat/useSteering.ts b/client/src/hooks/Chat/useSteering.ts
index 7baa97a580..21205e4d42 100644
--- a/client/src/hooks/Chat/useSteering.ts
+++ b/client/src/hooks/Chat/useSteering.ts
@@ -637,6 +637,12 @@ export default function useSteering({
         if (held == null) {
           return;
         }
+        /** Already neutralized and waiting for its parked epoch to be consumed.
+         *  Clearing it here would expose that epoch again, so a second queue
+         *  action must leave the tombstone standing. */
+        if (held.status === 'cancelled') {
+          return;
+        }
         set(
           store.queueDrainHoldByConvoId(queueKey),
           held.status === 'released' ? { ...held, status: 'cancelled' } : null,
@@ -725,36 +731,6 @@ export default function useSteering({
    *  rows simply stay put for a manual send. */
   const cancelQueueDrain = retireDrainHold;
 
-  /** Once a parked source is discarded it must never be retried as a recovery
-   * attempt. Downgrade the row in place so a guarded Edit that finds a newer
-   * draft can leave the same words, context, identity, and queue position as
-   * an ordinary local follow-up. */
-  const downgradeQueuedRecovery = useRecoilCallback(
-    ({ snapshot, set }) =>
-      (id: string): boolean => {
-        const queue = snapshot.getLoadable(store.queuedMessagesByConvoId(queueKey)).getValue();
-        let found = false;
-        const next = queue.map((item) => {
-          if (item.id !== id) {
-            return item;
-          }
-          found = true;
-          const {
-            clientRequestId: _clientRequestId,
-            recoverySteerId: _recoverySteerId,
-            recoveryClientSteerId: _recoveryClientSteerId,
-            ...ordinary
-          } = item;
-          return ordinary;
-        });
-        if (found) {
-          set(store.queuedMessagesByConvoId(queueKey), next);
-        }
-        return found;
-      },
-    [queueKey],
-  );
-
   /** Settle a queued row's terminal recovery source before an Edit/Remove.
    * Ordinary rows have no server copy. A v2 leftover first uses its durable
    * receipt to atomically discard the parked copy, then becomes an ordinary
@@ -792,17 +768,6 @@ export default function useSteering({
     [cancelSteer, conversationId, hasRealConvoId, localize, showToast],
   );
 
-  const discardQueued = useCallback(
-    async (item: QueuedMessage): Promise<boolean> => {
-      if (item.recoverySteerId == null) {
-        return true;
-      }
-      const cancelled = await cancelParkedSource(item);
-      return cancelled ? downgradeQueuedRecovery(item.id) : false;
-    },
-    [cancelParkedSource, downgradeQueuedRecovery],
-  );
-
   /** Empties the queue and hands back what it held. Removing up front is the
    *  point: the parked-source cancellations below are round trips, and rows
    *  left in place could be drained by a run ending mid-clear — sending the
@@ -958,6 +923,40 @@ export default function useSteering({
     [queueKey, releaseQueuedOrigin],
   );
 
+  /**
+   * Settles a row's parked source for an Edit or Remove, holding the row OUT of
+   * the queue for the round trip. Leaving it in place let a run completing
+   * mid-cancellation drain and send the very message being removed — the same
+   * hazard clear-all avoids by taking its rows up front. Cancellation refused:
+   * the row returns to its original slot untouched. Cancellation settled: it
+   * returns already downgraded, so what the caller does next (hand the words to
+   * the composer, remove it) sees an ordinary local row with no parked copy.
+   */
+  const discardQueued = useCallback(
+    async (item: QueuedMessage): Promise<boolean> => {
+      if (item.recoverySteerId == null) {
+        return true;
+      }
+      const origin = takeQueued(item.id);
+      if (origin == null) {
+        return false;
+      }
+      if (!(await cancelParkedSource(item))) {
+        restoreQueued(origin);
+        return false;
+      }
+      const {
+        clientRequestId: _clientRequestId,
+        recoverySteerId: _recoverySteerId,
+        recoveryClientSteerId: _recoveryClientSteerId,
+        ...ordinary
+      } = origin.item;
+      restoreQueued({ ...origin, item: ordinary });
+      return true;
+    },
+    [cancelParkedSource, restoreQueued, takeQueued],
+  );
+
   /**
    * Re-posts a spent run-end signal so the drain wakes and reconsiders the
    * queue. No-op while a signal for THIS conversation is still armed: that