queued messages survive switching to another chat
First report from somebody who is not us. Queue messages in a chat that is working, open a different conversation, come back: gone. Not a bug in the sense of code doing the wrong thing — `ChatPanelInner` is keyed on the session id, so opening another conversation REMOUNTS it, and that key is load-bearing (`useChat` seeds its state from the first session id it sees, so a remount is the honest way to say "this is a different chat"). The queue was local state and died with the mount. It was built for the minute between writing a message and a turn ending; switching away mid-turn was never in that picture. Now it lives in a module-level store, seeded on mount and written through on every change. Losing text somebody typed is the worst thing this could do, and it was doing it silently. ── Keyed by panel AND session ── Two panels can both be on a new chat, where the session id is `new` for both. Keying on the session alone would hand one panel's queue to the other — the same class of mistake wearing the opposite face. ── One decision that was not in the report ── A restored queue now DELIVERS on return if the agent is free. The existing drain only fires when a turn ends, and a remount has no such transition to watch, so without this you would come back to your messages sitting in the tray with nothing about to happen — which is the second report I would get. "These go when the agent is free" is the promise; the agent being free is exactly that case. Only on mount, and only when nothing is running: while a turn IS running the ordinary drain owns them, and firing here too would deliver twice. ── Not persisted, deliberately ── Module state, so it dies with the tab. A queue is a promise about the next minute; restoring one on a page loaded tomorrow would fire a message into a conversation whose context has moved on. Surviving a panel switch is the whole requirement. 11 tests, including the two-panels-on-one-conversation and both-on-a-new-chat collisions. tsgo clean, frontend builds, 898 pass / 7 fail unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -38,6 +38,8 @@ type ChatPanelInnerProps = {
|
||||
onSessionId?: (id: string | null) => void;
|
||||
/** The transcript to open on, already fetched. Absent for a new conversation. */
|
||||
resume?: ResumeState;
|
||||
/** Identifies this panel's queue, so switching conversations does not hand it to another panel. */
|
||||
panelId?: string;
|
||||
};
|
||||
|
||||
const ChatPanelInner = ({
|
||||
@@ -51,6 +53,7 @@ const ChatPanelInner = ({
|
||||
agentPanel,
|
||||
onSessionId,
|
||||
resume,
|
||||
panelId,
|
||||
}: ChatPanelInnerProps) => {
|
||||
/**
|
||||
* Resuming needs the TRANSCRIPT, not just the id.
|
||||
@@ -99,6 +102,8 @@ const ChatPanelInner = ({
|
||||
cwd={cwdParam}
|
||||
replaceUrl={false}
|
||||
promptPrefix={promptPrefix}
|
||||
panelId={panelId}
|
||||
sessionId={sessionKey ?? undefined}
|
||||
{...chatContext}
|
||||
/>
|
||||
</div>
|
||||
@@ -241,6 +246,7 @@ export const ChatPanelWrapper = ({ panelId, promptPrefix }: ChatPanelWrapperProp
|
||||
return (
|
||||
<ChatPanelInner
|
||||
key={key}
|
||||
panelId={panelId}
|
||||
scoped={scoped}
|
||||
cwdParam={cwdParam}
|
||||
promptPrefix={promptPrefix}
|
||||
|
||||
@@ -5,6 +5,8 @@ import { InputArea } from '../components/InputArea';
|
||||
|
||||
type EmbeddableChatProps = {
|
||||
sessionId?: string;
|
||||
/** Which panel this is — see `queue-store`. Absent on the screen, where there is only one chat. */
|
||||
panelId?: string;
|
||||
initialModel?: string | null;
|
||||
initialMessage?: {
|
||||
text: string;
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
import { beforeEach, describe, expect, test } from 'bun:test';
|
||||
import { queueKey, readQueue, writeQueue } from './queue-store';
|
||||
import type { QueuedPrompt } from './useEmbeddableChat';
|
||||
|
||||
// Reported by a user: queue messages in a chat that is working, switch to another chat, and they are gone.
|
||||
// The panel is keyed on the session id, so opening another conversation remounts it — and the queue was
|
||||
// local state. Losing text somebody typed is the worst thing this could do.
|
||||
|
||||
const prompt = (raw: string): QueuedPrompt => ({ id: raw, raw, prefix: '', ids: [], images: [] });
|
||||
|
||||
const KEYS = ['p1:s1', 'p2:s1', 'p1:new', 'p2:new', 'screen:s1'];
|
||||
beforeEach(() => KEYS.forEach((k) => writeQueue(k, [])));
|
||||
|
||||
describe('queueKey', () => {
|
||||
test('a panel and a session together identify a queue', () => {
|
||||
expect(queueKey('p1', 's1')).toBe('p1:s1');
|
||||
});
|
||||
|
||||
test('two panels on the SAME conversation do not share a queue', () => {
|
||||
expect(queueKey('p1', 's1')).not.toBe(queueKey('p2', 's1'));
|
||||
});
|
||||
|
||||
test('two panels both on a new chat do not share one either', () => {
|
||||
// The case that would otherwise collide: `sessionId` is undefined for both until the first turn.
|
||||
expect(queueKey('p1', undefined)).not.toBe(queueKey('p2', undefined));
|
||||
});
|
||||
|
||||
test('one conversation in one panel is stable across remounts — that is the whole point', () => {
|
||||
expect(queueKey('p1', 's1')).toBe(queueKey('p1', 's1'));
|
||||
});
|
||||
|
||||
test('the screen has no panel and still gets a key of its own', () => {
|
||||
expect(queueKey(undefined, 's1')).toBe('screen:s1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('the store', () => {
|
||||
test('what was queued is what comes back', () => {
|
||||
writeQueue('p1:s1', [prompt('first'), prompt('second')]);
|
||||
expect(readQueue('p1:s1').map((p) => p.raw)).toEqual(['first', 'second']);
|
||||
});
|
||||
|
||||
test('switching panels does not leak a queue sideways', () => {
|
||||
writeQueue('p1:s1', [prompt('mine')]);
|
||||
expect(readQueue('p2:s1')).toEqual([]);
|
||||
});
|
||||
|
||||
test('an unknown key is empty, not undefined — callers seed state from this', () => {
|
||||
expect(readQueue('never:seen')).toEqual([]);
|
||||
});
|
||||
|
||||
test('emptying a queue forgets it rather than keeping an empty entry', () => {
|
||||
writeQueue('p1:s1', [prompt('x')]);
|
||||
writeQueue('p1:s1', []);
|
||||
expect(readQueue('p1:s1')).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,41 @@
|
||||
import type { QueuedPrompt } from './useEmbeddableChat';
|
||||
|
||||
/**
|
||||
* Queued prompts, kept across a switch to another conversation.
|
||||
*
|
||||
* ── Why they were being lost ──
|
||||
*
|
||||
* `ChatPanelInner` is keyed on the session id, so opening a different conversation REMOUNTS it. That is
|
||||
* deliberate and load-bearing — `useChat` seeds its state from the first session id it sees, so a remount
|
||||
* is the honest way to say "this is a different chat". The queue was local state, so it died with the
|
||||
* mount.
|
||||
*
|
||||
* Not a bug so much as a case nobody had reached: the queue was built for the minute between writing a
|
||||
* message and a turn ending, and switching away mid-turn was never part of that picture. It is now, and
|
||||
* losing text somebody typed is the worst thing this could do.
|
||||
*
|
||||
* ── Keyed by panel AND session ──
|
||||
*
|
||||
* Two panels can both be on a new chat, where the session id is `new` for both. Keying on the session
|
||||
* alone would hand one panel's queue to the other — which is the same class of mistake as a queue being
|
||||
* dropped, wearing the opposite face.
|
||||
*
|
||||
* ── Module state, so it dies with the tab ──
|
||||
*
|
||||
* Deliberately not persisted. A queue is "these go when the agent is free", which is a promise about the
|
||||
* next minute; restoring one on a page loaded tomorrow would fire a message into a conversation whose
|
||||
* context has moved on. Surviving a panel switch is the whole requirement.
|
||||
*/
|
||||
const queues = new Map<string, QueuedPrompt[]>();
|
||||
|
||||
/** A queue belongs to one panel's view of one conversation. */
|
||||
export const queueKey = (panelId: string | undefined, sessionId: string | undefined): string =>
|
||||
`${panelId ?? 'screen'}:${sessionId ?? 'new'}`;
|
||||
|
||||
export const readQueue = (key: string): QueuedPrompt[] => queues.get(key) ?? [];
|
||||
|
||||
/** Empty is the same as absent — an entry per conversation ever opened would grow without bound. */
|
||||
export function writeQueue(key: string, items: QueuedPrompt[]): void {
|
||||
if (items.length === 0) queues.delete(key);
|
||||
else queues.set(key, items);
|
||||
}
|
||||
@@ -4,6 +4,7 @@ import { useUserVisibleModels } from 'state/useModels';
|
||||
import { useChat, type UseChatType } from '../../../hooks/useChat';
|
||||
import { useAttachments } from '../useAttachments';
|
||||
import { useSlashCommands } from '../useSlashCommands';
|
||||
import { queueKey, readQueue, writeQueue } from './queue-store';
|
||||
import { usePromptHistory } from './usePromptHistory';
|
||||
|
||||
/** A prompt written mid-turn, held with the attachments it was composed with. */
|
||||
@@ -26,6 +27,11 @@ export type QueuedPrompt = {
|
||||
|
||||
type UseEmbeddableChatParams = {
|
||||
sessionId?: string;
|
||||
/**
|
||||
* Which panel this is, so a queue survives switching conversations without being handed to the wrong
|
||||
* panel. Absent on the full screen, where there is only one chat.
|
||||
*/
|
||||
panelId?: string;
|
||||
initialModel?: string | null;
|
||||
initialMessage?: {
|
||||
text: string;
|
||||
@@ -81,6 +87,24 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp
|
||||
const attachmentManager = useAttachments({ sessionId });
|
||||
const slashCommands = useSlashCommands({ sessionId });
|
||||
|
||||
/**
|
||||
* A queue restored by coming back to this conversation goes now, if the agent is free.
|
||||
*
|
||||
* The drain below only fires on a turn ENDING, and a remount has no such transition to watch — so
|
||||
* without this, returning to a chat whose turn finished while you were elsewhere would leave your
|
||||
* messages sitting in the tray with nothing about to happen. "These go when the agent is free" is the
|
||||
* whole promise of queueing; the agent being free is exactly this case.
|
||||
*
|
||||
* Only on mount, and only when nothing is running: while a turn IS running the ordinary drain owns
|
||||
* them, and firing here as well would deliver twice.
|
||||
*/
|
||||
const restoredRef = useRef(false);
|
||||
useEffect(() => {
|
||||
if (restoredRef.current) return;
|
||||
restoredRef.current = true;
|
||||
if (!isGenerating && queuedRef.current.length > 0) void drainQueueRef.current();
|
||||
}, []);
|
||||
|
||||
// Detect when agent finishes a message (isGenerating: true → false)
|
||||
const wasGeneratingRef = useRef(false);
|
||||
useEffect(() => {
|
||||
@@ -98,11 +122,16 @@ export function useEmbeddableChat(params: UseEmbeddableChatParams, onMessageComp
|
||||
// Prompts written while a turn was running, waiting their turn. The ref is what the drain reads — it
|
||||
// runs from an effect and from async continuations, both of which would otherwise see the array as it
|
||||
// was a render ago — and the state is only so the tray can show them.
|
||||
const queuedRef = useRef<QueuedPrompt[]>([]);
|
||||
const [queued, setQueued] = useState<QueuedPrompt[]>([]);
|
||||
//
|
||||
// Seeded from `queue-store`, because opening another conversation remounts this whole component and the
|
||||
// queue would otherwise go with it. Text somebody typed must not be lost by navigating.
|
||||
const storeKey = queueKey(params.panelId, params.sessionId);
|
||||
const queuedRef = useRef<QueuedPrompt[]>(readQueue(storeKey));
|
||||
const [queued, setQueued] = useState<QueuedPrompt[]>(queuedRef.current);
|
||||
const setQueue = (next: QueuedPrompt[]) => {
|
||||
queuedRef.current = next;
|
||||
setQueued(next);
|
||||
writeQueue(storeKey, next);
|
||||
};
|
||||
const drainQueueRef = useRef<() => Promise<void>>(async () => {});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user