fix: keep the queue order and settle retried steers

Converting a steer sorted the whole queue by creation time, undoing a
reorder made from the rail and changing which message sent next. Each
converted item is placed at its own position instead.

Retries resolved through mutate callbacks, which react-query drops once
the observer unmounts. The pending block unmounts when the run ends, so a
chip could sit on "sending" for the rest of the conversation.

client/src/hooks/Chat/__tests__/useSteerConvert.spec.tsx

@@ -253,6 +253,44 @@ describe('useSteerConvert', () => {
     expect(result.current.queue.map((item) => item.id)).toEqual(['urgent', 'old']);
   });
 
+  /* The rail can be reordered by hand, and that order decides what sends next.
+     A conversion arriving afterwards may only place its own items; sorting the
+     whole list would quietly restore the order the messages were written in. */
+  it('leaves a hand-reordered queue in the order the user left it', () => {
+    const { result } = setup(({ set }) => {
+      set(store.pendingSteersByConvoId(CONVO_ID), [
+        { steerId: 'srv-late', text: 'converted', status: 'pending' as const, createdAt: 9 },
+      ]);
+      // As if the user had dragged the newest message to the front.
+      set(store.queuedMessagesByConvoId(CONVO_ID), [
+        { id: 'm3', text: 'third, promoted', createdAt: 3 },
+        { id: 'm1', text: 'first', createdAt: 1 },
+        { id: 'm2', text: 'second', createdAt: 2 },
+      ]);
+    });
+    act(() => {
+      result.current.convert(CONVO_ID, [{ steerId: 'srv-late', text: 'converted', createdAt: 9 }]);
+    });
+    expect(result.current.queue.map((item) => item.id)).toEqual(['m3', 'm1', 'm2', 'srv-late']);
+  });
+
+  it('still places a converted steer ahead of messages written after it', () => {
+    const { result } = setup(({ set }) => {
+      set(store.pendingSteersByConvoId(CONVO_ID), [
+        { steerId: 'srv-early', text: 'accepted first', status: 'pending' as const, createdAt: 1 },
+      ]);
+      set(store.queuedMessagesByConvoId(CONVO_ID), [
+        { id: 'later', text: 'queued afterwards', createdAt: 5 },
+      ]);
+    });
+    act(() => {
+      result.current.convert(CONVO_ID, [
+        { steerId: 'srv-early', text: 'accepted first', createdAt: 1 },
+      ]);
+    });
+    expect(result.current.queue.map((item) => item.id)).toEqual(['srv-early', 'later']);
+  });
+
   it('is idempotent across double delivery (abort response + final SSE event)', () => {
     const { result } = setup();
     const steers = [{ steerId: 'srv-2', text: 'delivered twice', createdAt: 5 }];

client/src/hooks/Chat/__tests__/useSteerRecovery.spec.tsx

@@ -1,17 +1,19 @@
 import React from 'react';
-import { act, renderHook } from '@testing-library/react';
+import { act, render, renderHook } from '@testing-library/react';
 import { RecoilRoot, useRecoilValue, type MutableSnapshot } from 'recoil';
 import useSteerRecovery from '../useSteerRecovery';
 import store from '~/store';
 
-const mockMutate = jest.fn();
-const mockFetchStreamStatus = jest.fn();
+const mockMutateAsync = jest.fn();
 
 jest.mock('~/data-provider', () => ({
-  useSteerMessageMutation: () => ({ mutate: mockMutate }),
-  fetchStreamStatus: (...args: unknown[]) => mockFetchStreamStatus(...args),
+  useSteerMessageMutation: () => ({ mutateAsync: mockMutateAsync }),
 }));
 
+/** The POST settles through the returned promise, so every case has to let the
+ *  microtask queue run before asserting. */
+const flush = () => act(async () => undefined);
+
 const CONVO_ID = 'convo-steer-recovery';
 
 function setup(initialize?: (snapshot: MutableSnapshot) => void) {
@@ -36,7 +38,7 @@ describe('useSteerRecovery', () => {
 
   describe('retry', () => {
     it('marks the chip sending immediately', () => {
-      mockMutate.mockImplementation(() => undefined);
+      mockMutateAsync.mockReturnValue(new Promise(() => undefined));
       const { result } = setup(({ set }) => {
         set(store.pendingSteersByConvoId(CONVO_ID), [
           { steerId: 'local-1', text: 'redo this', status: 'failed', createdAt: 5 },
@@ -50,9 +52,12 @@ describe('useSteerRecovery', () => {
       ]);
     });
 
-    it('swaps the local id for the server id on success, keeping it pending', () => {
-      mockMutate.mockImplementation((_params, { onSuccess }) => {
-        onSuccess({ steerId: 'srv-9', status: 'queued', position: 1, conversationId: CONVO_ID });
+    it('swaps the local id for the server id on success, keeping it pending', async () => {
+      mockMutateAsync.mockResolvedValue({
+        steerId: 'srv-9',
+        status: 'queued',
+        position: 1,
+        conversationId: CONVO_ID,
       });
       const { result } = setup(({ set }) => {
         set(store.pendingSteersByConvoId(CONVO_ID), [
@@ -62,6 +67,7 @@ describe('useSteerRecovery', () => {
       act(() => {
         result.current.recovery.retry('local-1');
       });
+      await flush();
       // The old local id must be gone entirely — leaving it behind is what let
       // the applied SteerPart and a stale pending copy render together.
       expect(result.current.chips).toEqual([
@@ -69,10 +75,8 @@ describe('useSteerRecovery', () => {
       ]);
     });
 
-    it('routes to the queue on NO_ACTIVE_RUN instead of marking it failed again', () => {
-      mockMutate.mockImplementation((_params, { onError }) => {
-        onError({ response: { data: { code: 'NO_ACTIVE_RUN' } } });
-      });
+    it('routes to the queue on NO_ACTIVE_RUN instead of marking it failed again', async () => {
+      mockMutateAsync.mockRejectedValue({ response: { data: { code: 'NO_ACTIVE_RUN' } } });
       const { result } = setup(({ set }) => {
         set(store.pendingSteersByConvoId(CONVO_ID), [
           { steerId: 'local-2', text: 'too late', status: 'failed', createdAt: 7 },
@@ -81,17 +85,16 @@ describe('useSteerRecovery', () => {
       act(() => {
         result.current.recovery.retry('local-2');
       });
+      await flush();
       expect(result.current.chips).toEqual([]);
       expect(result.current.queue).toEqual([
         expect.objectContaining({ id: 'local-2', text: 'too late' }),
       ]);
     });
 
-    it('also routes to the queue on RUN_PAUSED / STEER_UNSUPPORTED / STEER_QUEUE_FULL', () => {
+    it('also routes to the queue on RUN_PAUSED / STEER_UNSUPPORTED / STEER_QUEUE_FULL', async () => {
       for (const code of ['RUN_PAUSED', 'STEER_UNSUPPORTED', 'STEER_QUEUE_FULL']) {
-        mockMutate.mockImplementation((_params, { onError }) => {
-          onError({ response: { data: { code } } });
-        });
+        mockMutateAsync.mockRejectedValue({ response: { data: { code } } });
         const { result } = setup(({ set }) => {
           set(store.pendingSteersByConvoId(CONVO_ID), [
             { steerId: `local-${code}`, text: code, status: 'failed', createdAt: 1 },
@@ -100,14 +103,13 @@ describe('useSteerRecovery', () => {
         act(() => {
           result.current.recovery.retry(`local-${code}`);
         });
+        await flush();
         expect(result.current.queue).toEqual([expect.objectContaining({ id: `local-${code}` })]);
       }
     });
 
-    it('marks it failed again on an unrecognized error', () => {
-      mockMutate.mockImplementation((_params, { onError }) => {
-        onError(new Error('network'));
-      });
+    it('marks it failed again on an unrecognized error', async () => {
+      mockMutateAsync.mockRejectedValue(new Error('network'));
       const { result } = setup(({ set }) => {
         set(store.pendingSteersByConvoId(CONVO_ID), [
           { steerId: 'local-3', text: 'network flake', status: 'failed', createdAt: 1 },
@@ -116,18 +118,71 @@ describe('useSteerRecovery', () => {
       act(() => {
         result.current.recovery.retry('local-3');
       });
+      await flush();
       expect(result.current.chips).toEqual([
         expect.objectContaining({ steerId: 'local-3', status: 'failed' }),
       ]);
       expect(result.current.queue).toEqual([]);
     });
 
+    /* The block this hook lives in unmounts the moment the run ends, which is
+       exactly when a retry's ack tends to land. It has to survive that: the
+       words go to the queue rather than leaving the chip saying `sending`. */
+    /* The block this hook lives in unmounts the moment the run ends, which is
+       exactly when a retry's ack tends to land. It has to survive that: the
+       words go to the queue rather than leaving the chip saying `sending`. */
+    it('queues a retry whose ack lands after the run ended', async () => {
+      let settle: (value: unknown) => void = () => undefined;
+      mockMutateAsync.mockReturnValue(new Promise((resolve) => (settle = resolve)));
+
+      let recovery: ReturnType<typeof useSteerRecovery> | undefined;
+      let chips: unknown[] = [];
+      let queue: unknown[] = [];
+      const Recovery = () => {
+        recovery = useSteerRecovery(CONVO_ID);
+        return null;
+      };
+      /* Outlives the run, the way the store does: the hook's own tree goes
+         away while the conversation's state stays behind to be read. */
+      const Observer = () => {
+        chips = useRecoilValue(store.pendingSteersByConvoId(CONVO_ID));
+        queue = useRecoilValue(store.queuedMessagesByConvoId(CONVO_ID));
+        return null;
+      };
+      const Tree = ({ live }: { live: boolean }) => (
+        <RecoilRoot
+          initializeState={({ set }) => {
+            set(store.pendingSteersByConvoId(CONVO_ID), [
+              { steerId: 'local-late', text: 'landed too late', status: 'failed', createdAt: 3 },
+            ]);
+          }}
+        >
+          <Observer />
+          {live && <Recovery />}
+        </RecoilRoot>
+      );
+
+      const { rerender } = render(<Tree live={true} />);
+      act(() => {
+        recovery?.retry('local-late');
+      });
+      expect(chips).toEqual([expect.objectContaining({ status: 'sending' })]);
+
+      rerender(<Tree live={false} />);
+      await act(async () => {
+        settle({ steerId: 'srv-late', status: 'queued', position: 1, conversationId: CONVO_ID });
+      });
+
+      expect(chips).toEqual([]);
+      expect(queue).toEqual([expect.objectContaining({ id: 'srv-late', text: 'landed too late' })]);
+    });
+
     it('no-ops when the steer id is no longer pending', () => {
       const { result } = setup();
       act(() => {
         result.current.recovery.retry('missing');
       });
-      expect(mockMutate).not.toHaveBeenCalled();
+      expect(mockMutateAsync).not.toHaveBeenCalled();
     });
   });
 

client/src/hooks/Chat/useSteerConvert.ts

@@ -160,15 +160,27 @@ export default function useSteerConvert() {
           if (fresh.length === 0) {
             return existing;
           }
-          /** Ordinary steer leftovers merge chronologically. A steer that
-           *  originated in this queue restores its exact object/identity and
-           *  captured position instead of being re-minted under the server id. */
-          const ordinary = fresh.filter(({ queuedOrigin }) => queuedOrigin == null);
-          let merged: QueuedMessage[] = [...existing, ...ordinary.map(({ item }) => item)].sort(
-            (a, b) =>
-              Number(b.priority ?? false) - Number(a.priority ?? false) ||
-              a.createdAt - b.createdAt,
-          );
+          // Each new item is placed chronologically — a steer accepted BEFORE
+          // the user queued a later follow-up must drain first — EXCEPT explicit
+          // front-inserts ("Interrupt & send"), whose urgency outranks age.
+          //
+          // Placed rather than sorted: the queue can be reordered by hand from
+          // the rail, and sorting the whole list would quietly restore the order
+          // the messages were written in, changing which one sends next.
+          let merged: QueuedMessage[] = [...existing];
+          const ordinary = fresh
+            .filter(({ queuedOrigin }) => queuedOrigin == null)
+            .map(({ item }) => item)
+            .sort((a, b) => a.createdAt - b.createdAt);
+          for (const item of ordinary) {
+            const at = merged.findIndex(
+              (queued) => queued.priority !== true && queued.createdAt > item.createdAt,
+            );
+            merged.splice(at === -1 ? merged.length : at, 0, item);
+          }
+          /** A steer that originated in this queue restores its exact
+           *  object/identity and captured position instead of being re-minted
+           *  under the server id at a merely chronological spot. */
           for (const { queuedOrigin } of fresh) {
             if (queuedOrigin != null) {
               merged = insertQueuedOrigin(merged, queuedOrigin);

client/src/hooks/Chat/useSteerRecovery.ts

@@ -13,7 +13,7 @@ import store from '~/store';
  * `SteeringControls` object through the message tree.
  */
 export default function useSteerRecovery(conversationId: string) {
-  const { mutate: steerMessage } = useSteerMessageMutation();
+  const { mutateAsync: steerMessage } = useSteerMessageMutation();
   const convertSteersToQueued = useSteerConvert();
 
   /** `PendingSteers` only renders while `isLast && isSubmitting` is true, so
@@ -76,34 +76,36 @@ export default function useSteerRecovery(conversationId: string) {
           return;
         }
         markStatus(steerId, 'sending');
-        steerMessage(
-          { conversationId, text: steer.text, files: steer.files },
-          {
-            onSuccess: (response) => {
-              acknowledgeRetry(
-                steerId,
-                { ...steer, steerId: response.steerId, status: 'pending' },
-                !mountedRef.current,
-              );
-            },
-            onError: (error) => {
-              const code = getSteerErrorCode(error);
-              // The run ended, is paused, or can't accept a steer right now —
-              // none of that means the words are lost, just that a queued
-              // follow-up is the only way left to send them.
-              if (
-                code === 'NO_ACTIVE_RUN' ||
-                code === 'RUN_PAUSED' ||
-                code === 'STEER_UNSUPPORTED' ||
-                code === 'STEER_QUEUE_FULL'
-              ) {
-                queueSteer(steer);
-                return;
-              }
-              markStatus(steerId, 'failed');
-            },
-          },
-        );
+        /* Resolved through the promise rather than through `mutate`'s
+           per-call callbacks, which react-query drops once the observer has no
+           listeners. This hook lives in the block that unmounts the moment the
+           run ends, which is exactly when a retry's ack tends to land: those
+           callbacks never ran, and the chip was left saying `sending` for the
+           rest of the conversation with the words neither sent nor queued. */
+        steerMessage({ conversationId, text: steer.text, files: steer.files })
+          .then((response) => {
+            acknowledgeRetry(
+              steerId,
+              { ...steer, steerId: response.steerId, status: 'pending' },
+              !mountedRef.current,
+            );
+          })
+          .catch((error: unknown) => {
+            const code = getSteerErrorCode(error);
+            // The run ended, is paused, or can't accept a steer right now —
+            // none of that means the words are lost, just that a queued
+            // follow-up is the only way left to send them.
+            if (
+              code === 'NO_ACTIVE_RUN' ||
+              code === 'RUN_PAUSED' ||
+              code === 'STEER_UNSUPPORTED' ||
+              code === 'STEER_QUEUE_FULL'
+            ) {
+              queueSteer(steer);
+              return;
+            }
+            markStatus(steerId, 'failed');
+          });
       },
     [conversationId, steerMessage, markStatus, acknowledgeRetry, queueSteer],
   );