From 82cfe41033218f9719f8f2c75d6526a217cb62cf Mon Sep 17 00:00:00 2001
From: Marco Beretta <81851188+berry-13@users.noreply.github.com>
Date: Sun, 26 Jul 2026 16:32:30 +0200
Subject: [PATCH] fix: sweep only failed chips when the user aborts

The abort listener also fires on navigating away while the run continues
server side. Sweeping pending chips there stranded a steer the server had
already acked and sent it a second time. selectLocalSteersForQueue takes
a statuses argument so abort sweeps failed only.
---
 .../components/Chat/Input/Composer/Queue.tsx  | 14 ++++--
 .../Input/Composer/__tests__/Queue.spec.tsx   |  7 ++-
 .../SSE/__tests__/useResumableSSE.spec.ts     | 25 +++++++++-
 client/src/hooks/SSE/useResumableSSE.ts       | 48 +++++++++++++------
 client/src/locales/en/translation.json        |  2 -
 5 files changed, 73 insertions(+), 23 deletions(-)

diff --git a/client/src/components/Chat/Input/Composer/Queue.tsx b/client/src/components/Chat/Input/Composer/Queue.tsx
index 95b1ca79c2..0b4e2158a4 100644
--- a/client/src/components/Chat/Input/Composer/Queue.tsx
+++ b/client/src/components/Chat/Input/Composer/Queue.tsx
@@ -82,12 +82,16 @@ function Queue({ steering, conversationId, onEditToComposer, onRestoreToComposer
               <span
                 className="shrink-0 text-xs text-text-secondary"
                 title={localize('com_ui_queued_attachment_count', { 0: String(fileCount) })}
-                aria-label={localize('com_ui_queued_attachment_count', { 0: String(fileCount) })}
               >
-                {localize(
-                  fileCount === 1 ? 'com_ui_attachment_count_one' : 'com_ui_attachment_count',
-                  { count: fileCount },
-                )}
+                <span className="sr-only">
+                  {localize('com_ui_queued_attachment_count', { 0: String(fileCount) })}
+                </span>
+                <span aria-hidden="true">
+                  {localize(
+                    fileCount === 1 ? 'com_ui_attachment_count_one' : 'com_ui_attachment_count',
+                    { count: fileCount },
+                  )}
+                </span>
               </span>
             )}
             <button
diff --git a/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx b/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx
index e2374b4c7b..5dd091fe46 100644
--- a/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx
+++ b/client/src/components/Chat/Input/Composer/__tests__/Queue.spec.tsx
@@ -93,6 +93,11 @@ describe('Queue', () => {
     renderQueue([queued({ files: [{ file_id: 'f1' }, { file_id: 'f2' }] as never })]);
     const attachmentLabel = screen.getByText('com_ui_attachment_count:2');
     expect(attachmentLabel).toBeInTheDocument();
-    expect(attachmentLabel).toHaveAttribute('title', 'com_ui_queued_attachment_count:2');
+    expect(attachmentLabel.parentElement).toHaveAttribute(
+      'title',
+      'com_ui_queued_attachment_count:2',
+    );
+    expect(attachmentLabel.parentElement).not.toHaveAttribute('aria-label');
+    expect(screen.getByText('com_ui_queued_attachment_count:2')).toHaveClass('sr-only');
   });
 });
diff --git a/client/src/hooks/SSE/__tests__/useResumableSSE.spec.ts b/client/src/hooks/SSE/__tests__/useResumableSSE.spec.ts
index 97a397cab1..53a6de973b 100644
--- a/client/src/hooks/SSE/__tests__/useResumableSSE.spec.ts
+++ b/client/src/hooks/SSE/__tests__/useResumableSSE.spec.ts
@@ -3870,6 +3870,14 @@ describe('useResumableSSE', () => {
  * the selection logic directly (this is the exact piece of logic finding 3
  * was about — which statuses survive a run end) rather than faking a
  * RecoilRoot-backed integration around the rest of this large hook's mocks.
+ *
+ * The `statuses` parameter pins a second contract: the `final` handler and
+ * the two error/404 terminals call `convertLocalSteersToQueued` with no
+ * override (default `pending || failed`, the run is genuinely over), while
+ * the intentional-close `abort` listener passes `{ statuses: ['failed'] }`
+ * because that listener also fires on navigation away while the run
+ * CONTINUES server-side — a server-ACK'd `pending` chip must be left alone
+ * there, not swept into the queue as a duplicate of the server's own injection.
  */
 describe('selectLocalSteersForQueue', () => {
   const chip = (over: Partial<PendingSteer> = {}): PendingSteer => ({
@@ -3880,7 +3888,7 @@ describe('selectLocalSteersForQueue', () => {
     ...over,
   });
 
-  it('includes pending and failed chips, excluding sending', () => {
+  it('final/error-path selection (default statuses) includes pending and failed chips, excluding sending', () => {
     const chips = [
       chip({ steerId: 'p1', status: 'pending' }),
       chip({ steerId: 'f1', status: 'failed' }),
@@ -3889,6 +3897,21 @@ describe('selectLocalSteersForQueue', () => {
     expect(selectLocalSteersForQueue(chips).map((steer) => steer.steerId)).toEqual(['p1', 'f1']);
   });
 
+  it('abort-path selection (statuses: ["failed"]) sweeps only failed chips, leaving pending alone', () => {
+    // The abort listener fires on navigation away while the run CONTINUES
+    // server-side: a `pending` chip already has a server id and will be
+    // injected by the server regardless, so sweeping it here too would make
+    // `useQueueDrain` resend the same words as a duplicate turn at run end.
+    const chips = [
+      chip({ steerId: 'p1', status: 'pending' }),
+      chip({ steerId: 'f1', status: 'failed' }),
+      chip({ steerId: 'sending-1', status: 'sending' }),
+    ];
+    expect(selectLocalSteersForQueue(chips, ['failed']).map((steer) => steer.steerId)).toEqual([
+      'f1',
+    ]);
+  });
+
   it('converts a failed local chip present at a run-end path into a queueable item', () => {
     // The leak finding 3 was about: a `failed` chip carries a local-* id the
     // server never reports, so `data.pendingSteers`/the abort response can
diff --git a/client/src/hooks/SSE/useResumableSSE.ts b/client/src/hooks/SSE/useResumableSSE.ts
index 712d11dd56..2e4acb0ad8 100644
--- a/client/src/hooks/SSE/useResumableSSE.ts
+++ b/client/src/hooks/SSE/useResumableSSE.ts
@@ -607,29 +607,39 @@ const mergeResumeMessages = (
   return [...nextMessages, userMessage, responseMessage];
 };
 
+/** Default sweep for a run that has genuinely ended (final/error/404): a
+ *  `pending` chip behind a server id that no `on_steer_applied` will ever
+ *  confirm, or a `failed` chip that never reached the server at all. Either
+ *  one left behind survives past this run end and renders under whatever
+ *  comes next — `PendingSteers` reads the same atom keyed only by
+ *  conversation, not by run. */
+const RUN_ENDED_STATUSES: readonly PendingSteer['status'][] = ['pending', 'failed'];
+
 /**
- * Local chips with no injection-boundary event left to resolve them: `pending`
- * behind a server id that no `on_steer_applied` will ever confirm (the run
- * ended), or `failed` and never having reached the server at all. Either one
- * left behind survives past this run end and renders under whatever comes
- * next — `PendingSteers` reads the same atom keyed only by conversation, not
- * by run.
+ * Local chips with no injection-boundary event left to resolve them.
+ * `statuses` defaults to `RUN_ENDED_STATUSES` for terminals where the run is
+ * actually over; pass `['failed']` for a path (like intentional abort) where
+ * the run may still be live server-side and a server-ACK'd `pending` chip
+ * must be left alone — the server injects it regardless, and sweeping it here
+ * would resend the same words as a duplicate queued turn.
  *
- * `sending` chips are deliberately excluded: they have their own in-flight
- * POST whose `onSuccess`/`onError` will settle them, and sweeping them here
- * too would race that callback — a late ACK's re-add in
+ * `sending` chips are never included: they have their own in-flight POST
+ * whose `onSuccess`/`onError` will settle them, and sweeping them here too
+ * would race that callback — a late ACK's re-add in
  * `resolveAcknowledgedSteer` could then double-queue the same words.
  */
 export function selectLocalSteersForQueue(
   chips: PendingSteer[],
+  statuses: readonly PendingSteer['status'][] = RUN_ENDED_STATUSES,
   /** Ids the caller has already routed elsewhere this run end; matched against
    *  both ids a chip can be known by, since either may be the one excluded. */
   excluded: ReadonlySet<string> = new Set(),
 ): TPendingSteer[] {
+  const allowed = new Set(statuses);
   return chips
     .filter(
       (steer) =>
-        (steer.status === 'pending' || steer.status === 'failed') &&
+        allowed.has(steer.status) &&
         !excluded.has(steer.steerId) &&
         (steer.clientSteerId == null || !excluded.has(steer.clientSteerId)),
     )
@@ -977,15 +987,18 @@ export default function useResumableSSE(
   /** Sweeps this conversation's local chips (see `selectLocalSteersForQueue`)
    *  into queued follow-ups. Error events carry no `pendingSteers` payload of
    *  their own (the server drops its copy on failure) — this is the only
-   *  source of truth for those runs; the `final` and intentional-`abort`
-   *  paths call it alongside their own server-reported list as a backstop for
-   *  `failed` chips, which never rode that list at all. */
+   *  source of truth for those runs; the `final` and error/404 terminals call
+   *  it (default `statuses`, i.e. `pending || failed`) alongside their own
+   *  server-reported list as a backstop for `failed` chips, which never rode
+   *  that list at all. The intentional-abort path passes `statuses: ['failed']`
+   *  since the run may still be live server-side there (see its call site). */
   const convertLocalSteersToQueued = useRecoilCallback(
     ({ snapshot }) =>
       (
         conversationId: string,
         options?: {
           claimParked?: boolean;
+          statuses?: readonly PendingSteer['status'][];
           excludeSteerIds?: Iterable<string>;
           generationProtocolVersion?: GenerationProtocolVersion;
         },
@@ -993,6 +1006,7 @@ export default function useResumableSSE(
         const chips = snapshot.getLoadable(store.pendingSteersByConvoId(conversationId)).getValue();
         const settled = selectLocalSteersForQueue(
           chips,
+          options?.statuses,
           new Set(options?.excludeSteerIds ?? []),
         );
         if (settled.length > 0) {
@@ -3106,9 +3120,15 @@ export default function useResumableSSE(
         resetLive({ ...currentSubmission, userMessage });
         // No final/error event fires on this path, so it's the only place left
         // to sweep a local `failed` chip — otherwise it survives this close and
-        // renders (with a live Retry) under whatever run starts next.
+        // renders (with a live Retry) under whatever run starts next. `pending`
+        // chips are deliberately left alone here: this listener also fires on
+        // navigation away while the run CONTINUES server-side, so a
+        // server-ACK'd `pending` steer is not stranded — the server injects it
+        // regardless, and sweeping it into the queue too would resend the same
+        // words as a duplicate turn once `useQueueDrain` fires at run end.
         convertLocalSteersToQueued(
           currentSubmission.conversation?.conversationId ?? currentStreamId,
+          { statuses: ['failed'] },
         );
       });
 
diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json
index 7076e007bc..be21ba9d56 100644
--- a/client/src/locales/en/translation.json
+++ b/client/src/locales/en/translation.json
@@ -1957,8 +1957,6 @@
   "com_ui_stateful_sessions": "Stateful code sessions",
   "com_ui_status_prefix": "Status:",
   "com_ui_steer": "Steer",
-  "com_ui_steer_already_applied": "That steering message already reached the agent, so it was left in the response",
-  "com_ui_steer_cancel": "Cancel steering message",
   "com_ui_steer_cancel_failed": "Could not cancel the steering message — it may still reach the agent",
   "com_ui_steer_failed": "Steering failed",
   "com_ui_steer_failed_inline": "Couldn't add to this reply",