🧟 fix: Prevent Drained Steer From Re-Queuing After Run-End Race (#14276)

This commit is contained in:
Danny Avila 2026-07-15 11:37:39 -04:00 committed by GitHub
parent 7447fddfb2
commit eccc7d58e9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 49 additions and 8 deletions

View file

@ -1,6 +1,6 @@
import React from 'react';
import { act, renderHook } from '@testing-library/react';
import { RecoilRoot, useRecoilValue, type MutableSnapshot } from 'recoil';
import { RecoilRoot, useRecoilValue, useSetRecoilState, type MutableSnapshot } from 'recoil';
import useSteerConvert from '../useSteerConvert';
import store from '~/store';
@ -16,12 +16,17 @@ function setup(initialize?: (snapshot: MutableSnapshot) => void) {
<RecoilRoot initializeState={initialize}>{children}</RecoilRoot>
);
return renderHook(
() => ({
convert: useSteerConvert(),
chips: useRecoilValue(store.pendingSteersByConvoId(CONVO_ID)),
queue: useRecoilValue(store.queuedMessagesByConvoId(CONVO_ID)),
applied: useRecoilValue(store.appliedSteerIdsByConvoId(CONVO_ID)),
}),
() => {
const setQueue = useSetRecoilState(store.queuedMessagesByConvoId(CONVO_ID));
return {
convert: useSteerConvert(),
chips: useRecoilValue(store.pendingSteersByConvoId(CONVO_ID)),
queue: useRecoilValue(store.queuedMessagesByConvoId(CONVO_ID)),
applied: useRecoilValue(store.appliedSteerIdsByConvoId(CONVO_ID)),
// Mirrors `useQueueDrain` dequeuing the head item after auto-send.
drainQueue: () => setQueue((prev) => prev.slice(1)),
};
},
{ wrapper },
);
}
@ -112,6 +117,29 @@ describe('useSteerConvert', () => {
expect(result.current.applied).toEqual(['srv-2']);
});
it('does not re-queue a steer already drained after conversion', () => {
const { result } = setup();
const steers = [{ steerId: 'srv-drained', text: 'submitted once', createdAt: 5 }];
// First delivery converts the leftover steer into a queued chip.
act(() => {
result.current.convert(CONVO_ID, steers);
});
expect(result.current.queue).toEqual([expect.objectContaining({ id: 'srv-drained' })]);
// The run-end drain submits it and removes it from the queue.
act(() => {
result.current.drainQueue();
});
expect(result.current.queue).toEqual([]);
// A late redelivery of the SAME steer (claimParked /chat/status, abort
// response, or reconnect) must NOT resurrect a queued chip for a message
// that was already sent — the applied-id set marks it settled.
act(() => {
result.current.convert(CONVO_ID, steers);
});
expect(result.current.queue).toEqual([]);
expect(result.current.applied).toEqual(['srv-drained']);
});
describe('claimParked (clears the parked server copy of live-delivered steers)', () => {
beforeEach(() => {
mockFetchStreamStatus.mockReset();

View file

@ -46,6 +46,15 @@ export default function useSteerConvert() {
.getValue();
const chipById = new Map(localChips.map((chip) => [chip.steerId, chip]));
const steerIds = new Set(steers.map((steer) => steer.steerId));
// Steers already settled (applied on the server OR converted here on an
// earlier delivery) must not re-enter the queue. Read BEFORE the append
// below so a first-time conversion still queues, but a redelivery whose
// item was already DRAINED out of the queue is a no-op — without this,
// the queue-only dedup below misses a message the run-end drain already
// submitted and re-mints it as a stranded queued chip.
const settledSteerIds = new Set(
snapshot.getLoadable(store.appliedSteerIdsByConvoId(conversationId)).getValue(),
);
set(store.appliedSteerIdsByConvoId(conversationId), (prev) =>
appendAppliedSteerIds(
prev,
@ -57,7 +66,11 @@ export default function useSteerConvert() {
);
set(store.queuedMessagesByConvoId(conversationId), (prev) => {
const fresh = steers
.filter((steer) => !prev.some((queued) => queued.id === steer.steerId))
.filter(
(steer) =>
!settledSteerIds.has(steer.steerId) &&
!prev.some((queued) => queued.id === steer.steerId),
)
.map((steer) => ({
id: steer.steerId,
text: steer.text,