mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
fix: flip the escalation lock synchronously before the arm request
Codex round 6, one P2. Round 4 deleted the escalating flag along with the reclaim continuation it guarded, but that left the one-interrupt gate blind during the arm request's own round trip: the chip-derived check cannot see an arm until its response relabels the chip, so on a slow connection two bubbles could both arm before either response landed. Double-arm is harmless server-side now (the run seals once and drains the whole queue in order), but every escalation control advertises "one interrupt at a time" by disabling, and the controls must tell the truth. The per-conversation escalating flag returns as a pure UX gate: set synchronously at click, before the mutation, cleared on settlement, and folded into interruptPending on both surfaces. Unlike its round 1-3 ancestor there is no continuation behind it to guard and no recheck to pair with it. Counterfactually verified: without the synchronous set, the two-bubble race test arms twice. 207 tests green across the Chat Input suites.
This commit is contained in:
parent
e6ff2530c3
commit
964897a3b3
5 changed files with 119 additions and 37 deletions
|
|
@ -1,5 +1,5 @@
|
|||
import { memo, useId, useRef, useMemo, useState, useEffect, useCallback } from 'react';
|
||||
import { useSetAtom } from 'jotai';
|
||||
import { useSetAtom, useAtomValue } from 'jotai';
|
||||
import { useToastContext } from '@librechat/client';
|
||||
import { useRecoilValue, useRecoilCallback } from 'recoil';
|
||||
import { X, Zap, ZapOff, Clock, Pencil, ChevronUp, ChevronDown } from 'lucide-react';
|
||||
|
|
@ -9,11 +9,11 @@ import type { PendingSteer } from '~/store/families';
|
|||
import type { MenuEntry } from './SteerMenu';
|
||||
import { RowMenu, useDefaultToggleEntry, useInterruptToggleEntry } from './SteerMenu';
|
||||
import FilePreviewDialog from '~/components/Chat/Messages/Content/FilePreviewDialog';
|
||||
import { steerOverlayHeightFamily, escalatingSteerFamily } from '~/store/steer';
|
||||
import MarkdownLite from '~/components/Chat/Messages/Content/MarkdownLite';
|
||||
import FileContainer from '~/components/Chat/Input/Files/FileContainer';
|
||||
import { useSteerCancel, useSteerReclaim, useLocalize } from '~/hooks';
|
||||
import ImagePreview from '~/components/Chat/Input/Files/ImagePreview';
|
||||
import { steerOverlayHeightFamily } from '~/store/steer';
|
||||
import { useArmSteerMutation } from '~/data-provider';
|
||||
import { carriedSteerContext, cn } from '~/utils';
|
||||
import store from '~/store';
|
||||
|
|
@ -142,6 +142,7 @@ const InFlightSteer = memo(function InFlightSteer({
|
|||
[conversationId],
|
||||
);
|
||||
const { mutateAsync: armSteer } = useArmSteerMutation();
|
||||
const setEscalating = useSetAtom(escalatingSteerFamily(conversationId));
|
||||
|
||||
/**
|
||||
* Takes the steer back off the server queue so its words can be re-homed.
|
||||
|
|
@ -238,32 +239,38 @@ const InFlightSteer = memo(function InFlightSteer({
|
|||
* `armed: false`, and the chip is only relabelled on a confirmed
|
||||
* durable arm. */
|
||||
onClick: () => {
|
||||
void armSteer({ conversationId, steerId: steer.steerId }).then(
|
||||
(response) => {
|
||||
if (response.armed === true) {
|
||||
markSteerPreempt(steer.steerId);
|
||||
return;
|
||||
}
|
||||
/* `armed: false` is deliberately ambiguous — injected,
|
||||
* cancelled, re-homed, or run over — so the message only
|
||||
* says the escalation lost, and the chip defers to the
|
||||
* events for whatever actually happened. */
|
||||
showToast({
|
||||
message: localize(
|
||||
response.code === 'PREEMPT_UNSUPPORTED'
|
||||
? 'com_ui_steer_preempt_unsupported'
|
||||
: 'com_ui_steer_arm_lost_race',
|
||||
),
|
||||
status: 'info',
|
||||
});
|
||||
},
|
||||
() => {
|
||||
showToast({
|
||||
message: localize('com_ui_steer_arm_failed'),
|
||||
status: 'error',
|
||||
});
|
||||
},
|
||||
);
|
||||
/* Synchronously, before the request: the chip-derived gate
|
||||
* cannot see this arm until the response lands, and the other
|
||||
* escalation controls advertise "one interrupt at a time". */
|
||||
setEscalating(true);
|
||||
void armSteer({ conversationId, steerId: steer.steerId })
|
||||
.then(
|
||||
(response) => {
|
||||
if (response.armed === true) {
|
||||
markSteerPreempt(steer.steerId);
|
||||
return;
|
||||
}
|
||||
/* `armed: false` is deliberately ambiguous — injected,
|
||||
* cancelled, re-homed, or run over — so the message only
|
||||
* says the escalation lost, and the chip defers to the
|
||||
* events for whatever actually happened. */
|
||||
showToast({
|
||||
message: localize(
|
||||
response.code === 'PREEMPT_UNSUPPORTED'
|
||||
? 'com_ui_steer_preempt_unsupported'
|
||||
: 'com_ui_steer_arm_lost_race',
|
||||
),
|
||||
status: 'info',
|
||||
});
|
||||
},
|
||||
() => {
|
||||
showToast({
|
||||
message: localize('com_ui_steer_arm_failed'),
|
||||
status: 'error',
|
||||
});
|
||||
},
|
||||
)
|
||||
.finally(() => setEscalating(false));
|
||||
},
|
||||
} satisfies MenuEntry,
|
||||
]),
|
||||
|
|
@ -448,10 +455,13 @@ const InFlightSteers = memo(function InFlightSteers({
|
|||
const steers = useRecoilValue(store.pendingSteersByConvoId(conversationId));
|
||||
const inFlight = useMemo(() => steers.filter((steer) => steer.status !== 'failed'), [steers]);
|
||||
/** Mirrors `PendingSteerChips`: while one interrupt is unresolved, every
|
||||
* other escalation control disables rather than arming a second seal. */
|
||||
* other escalation control disables rather than arming a second seal. The
|
||||
* escalating flag covers an arm request's round trip, before its chip
|
||||
* relabels for the chip-derived check to see. */
|
||||
const escalating = useAtomValue(escalatingSteerFamily(conversationId));
|
||||
const interruptPending = useMemo(
|
||||
() => inFlight.some((steer) => steer.preempt === true),
|
||||
[inFlight],
|
||||
() => escalating || inFlight.some((steer) => steer.preempt === true),
|
||||
[escalating, inFlight],
|
||||
);
|
||||
const setOverlayHeight = useSetAtom(steerOverlayHeightFamily(conversationId));
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { memo, useMemo } from 'react';
|
||||
import { useAtomValue } from 'jotai';
|
||||
import { useRecoilValue } from 'recoil';
|
||||
import * as Ariakit from '@ariakit/react';
|
||||
import { X, Zap, Send, Clock, Pencil, Trash2, ZapOff, Paperclip, RotateCcw } from 'lucide-react';
|
||||
|
|
@ -15,6 +16,7 @@ import {
|
|||
useInterruptChordHint,
|
||||
useInterruptToggleEntry,
|
||||
} from './SteerMenu';
|
||||
import { escalatingSteerFamily } from '~/store/steer';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { cn } from '~/utils';
|
||||
import store from '~/store';
|
||||
|
|
@ -289,10 +291,13 @@ function PendingSteerChips({
|
|||
const queued = useRecoilValue(store.queuedMessagesByConvoId(steering.queueKey));
|
||||
const failedSteers = useMemo(() => steers.filter((steer) => steer.status === 'failed'), [steers]);
|
||||
/** Only one interrupt can be in flight: a second preempt while one is
|
||||
* unresolved would arm a second seal, so escalation buttons disable. */
|
||||
* unresolved would arm a second seal, so escalation buttons disable. The
|
||||
* escalating flag covers a bubble arm's round trip, before its chip
|
||||
* relabels for the chip-derived check to see. */
|
||||
const escalating = useAtomValue(escalatingSteerFamily(conversationId));
|
||||
const interruptPending = useMemo(
|
||||
() => steers.some((steer) => steer.preempt === true && steer.status !== 'failed'),
|
||||
[steers],
|
||||
() => escalating || steers.some((steer) => steer.preempt === true && steer.status !== 'failed'),
|
||||
[escalating, steers],
|
||||
);
|
||||
|
||||
if (failedSteers.length === 0 && queued.length === 0) {
|
||||
|
|
|
|||
|
|
@ -1,10 +1,10 @@
|
|||
import React from 'react';
|
||||
import { RecoilRoot } from 'recoil';
|
||||
import { getDefaultStore } from 'jotai';
|
||||
import { render, screen, fireEvent, act } from '@testing-library/react';
|
||||
import { render, screen, within, fireEvent, act } from '@testing-library/react';
|
||||
import type { SteeringControls } from '~/hooks/Chat/useSteering';
|
||||
import type { PendingSteer } from '~/store/families';
|
||||
import { steerOverlayHeightFamily } from '~/store/steer';
|
||||
import { steerOverlayHeightFamily, escalatingSteerFamily } from '~/store/steer';
|
||||
import InFlightSteers from '../InFlightSteers';
|
||||
import store from '~/store';
|
||||
|
||||
|
|
@ -599,9 +599,13 @@ describe('InFlightSteers', () => {
|
|||
describe('InFlightSteers — interrupt-now escalation', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockArmMutateAsync.mockReset();
|
||||
mockCancelMutateAsync.mockResolvedValue({ removed: true });
|
||||
mockArmMutateAsync.mockResolvedValue({ armed: true });
|
||||
mockRestoreToComposer.mockReturnValue(true);
|
||||
act(() => {
|
||||
getDefaultStore().set(escalatingSteerFamily(CONVO_ID), false);
|
||||
});
|
||||
});
|
||||
|
||||
it("arms the interrupt in place, keeping the steer's id and position", async () => {
|
||||
|
|
@ -670,6 +674,43 @@ describe('InFlightSteers — interrupt-now escalation', () => {
|
|||
expect(screen.getByText('still queued')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('locks every escalation control while an arm request is in flight', async () => {
|
||||
/** The chip-derived gate cannot see an arm until its response lands, so
|
||||
* the flag flips synchronously at click — otherwise two bubbles could
|
||||
* both arm on a slow connection, belying the disabled controls. */
|
||||
let resolveArm: (value: { armed: boolean }) => void = () => {};
|
||||
mockArmMutateAsync.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveArm = resolve;
|
||||
}),
|
||||
);
|
||||
renderSteers([
|
||||
{ steerId: 's1', text: 'first', status: 'pending', createdAt: 1 },
|
||||
{ steerId: 's2', text: 'second', status: 'pending', createdAt: 2 },
|
||||
]);
|
||||
/** Every bubble's menu content mounts (hidden) up front, so item lookups
|
||||
* must scope to the menu the clicked button controls. */
|
||||
const menuFor = (button: HTMLElement) =>
|
||||
document.getElementById(button.getAttribute('aria-controls') ?? '') as HTMLElement;
|
||||
const menus = screen.getAllByLabelText('com_ui_more_options');
|
||||
fireEvent.click(menus[0]);
|
||||
fireEvent.click(await within(menuFor(menus[0])).findByText('com_ui_interrupt_steer_now'));
|
||||
|
||||
fireEvent.click(menus[1]);
|
||||
const secondItem = await within(menuFor(menus[1])).findByText('com_ui_interrupt_steer_now');
|
||||
expect(secondItem.closest('[role="menuitem"]')).toHaveAttribute('aria-disabled', 'true');
|
||||
await act(async () => {
|
||||
fireEvent.click(secondItem);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
resolveArm({ armed: true });
|
||||
});
|
||||
expect(mockArmMutateAsync).toHaveBeenCalledTimes(1);
|
||||
expect(mockArmMutateAsync).toHaveBeenCalledWith({ conversationId: CONVO_ID, steerId: 's1' });
|
||||
});
|
||||
|
||||
it('disables escalation on every bubble while an interrupt is unresolved', async () => {
|
||||
renderSteers([
|
||||
{ steerId: 's1', text: 'plain steer', status: 'pending', createdAt: 1 },
|
||||
|
|
|
|||
|
|
@ -1,8 +1,10 @@
|
|||
import React from 'react';
|
||||
import { RecoilRoot } from 'recoil';
|
||||
import { render, screen, fireEvent } from '@testing-library/react';
|
||||
import { getDefaultStore } from 'jotai';
|
||||
import { render, screen, fireEvent, act } from '@testing-library/react';
|
||||
import type { PendingSteer, QueuedMessage } from '~/store/families';
|
||||
import type { SteeringControls } from '~/hooks/Chat/useSteering';
|
||||
import { escalatingSteerFamily } from '~/store/steer';
|
||||
import PendingSteerChips from '../PendingSteerChips';
|
||||
import store from '~/store';
|
||||
|
||||
|
|
@ -134,6 +136,21 @@ describe('PendingSteerChips — queued interrupt-now', () => {
|
|||
expect(mockSendQueuedNow).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('disables escalation while a bubble arm request is in flight', () => {
|
||||
const jotai = getDefaultStore();
|
||||
act(() => {
|
||||
jotai.set(escalatingSteerFamily(CONVO_ID), true);
|
||||
});
|
||||
try {
|
||||
renderChips([queuedMessage], { steering: liveRun });
|
||||
expect(screen.getByTestId('queued-interrupt-now')).toBeDisabled();
|
||||
} finally {
|
||||
act(() => {
|
||||
jotai.set(escalatingSteerFamily(CONVO_ID), false);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('stays visible but disabled while the run is paused on approval', () => {
|
||||
/** The real hook invariant: `canSteer = hasRealConvoId && !pausedOnApproval`,
|
||||
* so a paused run always reads `canSteer: false`. The control must remain
|
||||
|
|
|
|||
|
|
@ -11,3 +11,12 @@ import { atomFamily } from 'jotai/utils';
|
|||
* each holds a single number per visited conversation.
|
||||
*/
|
||||
export const steerOverlayHeightFamily = atomFamily((_conversationId: string) => atom<number>(0));
|
||||
|
||||
/**
|
||||
* Set synchronously before a bubble's arm request and cleared on settlement.
|
||||
* Purely a UX gate: with the atomic in-place arm, a double-arm is harmless
|
||||
* server-side (the run seals once and drains the whole queue in order), but
|
||||
* every escalation control advertises "one interrupt at a time" by disabling,
|
||||
* and the chip-derived check cannot see an arm until its response lands.
|
||||
*/
|
||||
export const escalatingSteerFamily = atomFamily((_conversationId: string) => atom<boolean>(false));
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue