Skip to content

Commit d27ab7b

Browse files
authored
fix(react)+test: lock down PR #5 regressions and close three useChat gaps (#7)
* fix(react): close three useChat gaps surfaced by PR #5 review - send(): sync messagesRef before runStream so rapid synchronous sends both reach the outgoing request body - useState init: hydrate pendingDecisions from initialMessages so rehydrated paused tool calls render decision UI immediately - unmount: abort the in-flight fetch on cleanup to prevent leaked streams when the consumer unmounts mid-request * test: lock down regressions surfaced by PR #5 review Adds four regression tests that act as acceptance criteria for the fixes shipped in PR #5 and the companion useChat fixes in this branch. agent.test.ts: - injectDeferralResults() + prompt() places the synthetic toolResult adjacent to its assistant block (verifies the documented "user typed instead of deciding" recovery pattern produces provider-valid order). use-chat.test.ts: - initialMessages with a paused tool call hydrates pendingDecisions. - Two rapid synchronous send() calls both reach the outgoing body. - Unmount aborts the in-flight fetch.
1 parent cd00eaf commit d27ab7b

3 files changed

Lines changed: 130 additions & 1 deletion

File tree

packages/agent/__tests__/agent.test.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {
22
type AssistantMessage,
33
type Context,
44
createAssistantMessageEventStream,
5+
injectDeferralResults,
56
type Message,
67
type ModelDescriptor,
78
type ToolCallContent,
@@ -372,6 +373,53 @@ describe('@agentic-kit/agent — pausable tools', () => {
372373
expect(execute).not.toHaveBeenCalled();
373374
});
374375

376+
it('injectDeferralResults + prompt() places the synthetic toolResult adjacent to its assistant block', async () => {
377+
const provider = createScriptedProvider({
378+
responses: [pauseResponse(), finalResponse()],
379+
});
380+
const execute = jest.fn(async () => ({
381+
content: [{ type: 'text' as const, text: 'should not run' }],
382+
}));
383+
384+
const agent = new Agent({
385+
initialState: { model: makeFakeModel() },
386+
streamFn: provider.stream,
387+
});
388+
agent.setTools([makeApprovalTool(execute)]);
389+
390+
await agent.prompt('approve thing');
391+
392+
const withDeferrals = injectDeferralResults(agent.state.messages);
393+
agent.replaceMessages(withDeferrals);
394+
395+
const typed: Message = {
396+
role: 'user',
397+
content: 'never mind',
398+
timestamp: Date.now(),
399+
};
400+
await agent.prompt(typed);
401+
402+
const messages = agent.state.messages;
403+
const assistantIdx = messages.findIndex(
404+
(m) =>
405+
m.role === 'assistant' &&
406+
(m as AssistantMessage).content.some(
407+
(b) => b.type === 'toolCall' && b.id === 'tool_1'
408+
)
409+
);
410+
const toolResultIdx = messages.findIndex(
411+
(m) => m.role === 'toolResult' && m.toolCallId === 'tool_1'
412+
);
413+
const typedIdx = messages.findIndex(
414+
(m) => m.role === 'user' && m.content === 'never mind'
415+
);
416+
417+
expect(assistantIdx).toBeGreaterThanOrEqual(0);
418+
expect(toolResultIdx).toBe(assistantIdx + 1);
419+
expect(typedIdx).toBe(toolResultIdx + 1);
420+
expect(execute).not.toHaveBeenCalled();
421+
});
422+
375423
it('abort() while paused stops further work without throwing', async () => {
376424
const provider = createScriptedProvider({ responses: [pauseResponse()] });
377425

packages/react/__tests__/use-chat.test.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,49 @@ describe('useChat', () => {
5454
expect(result.current.executingToolCallIds.size).toBe(0);
5555
});
5656

57+
it('hydrates pendingDecisions from initialMessages when a paused tool call is present', () => {
58+
const initial: Message[] = [
59+
makeUser('hi'),
60+
makeAssistantWithToolCall('call_pending'),
61+
];
62+
63+
const { result } = renderHook(() =>
64+
useChat({ api: '/chat', initialMessages: initial })
65+
);
66+
67+
expect(result.current.pendingDecisions.has('call_pending')).toBe(true);
68+
expect(result.current.pendingDecisions.get('call_pending')).toMatchObject({
69+
toolCallId: 'call_pending',
70+
toolName: 'echo',
71+
});
72+
});
73+
74+
it('send(): two rapid synchronous sends both reach the outgoing request body', async () => {
75+
const final = makeFinalAssistant('ok');
76+
const fetchFn = jest.fn(
77+
async (_url: RequestInfo | URL, _init?: RequestInit): Promise<Response> =>
78+
streamFromEvents([
79+
{ type: 'agent_start' },
80+
{ type: 'agent_end', messages: [makeUser('first'), makeUser('second'), final] },
81+
])
82+
);
83+
84+
const { result } = renderHook(() => useChat({ api: '/chat', fetch: fetchFn }));
85+
86+
await act(async () => {
87+
const p1 = result.current.send('first');
88+
const p2 = result.current.send('second');
89+
await Promise.allSettled([p1, p2]);
90+
});
91+
92+
const lastInit = fetchFn.mock.calls.at(-1)![1] as RequestInit;
93+
const sent = JSON.parse(lastInit.body as string);
94+
const contents = sent.messages.map((m: Message) =>
95+
typeof m.content === 'string' ? m.content : null
96+
);
97+
expect(contents).toEqual(['first', 'second']);
98+
});
99+
57100
it('sends, streams, and folds messages into the log', async () => {
58101
const final = makeFinalAssistant('world');
59102
const userEcho = makeUser('hello');
@@ -656,6 +699,36 @@ describe('useChat', () => {
656699
expect(result.current.isStreaming).toBe(false);
657700
});
658701

702+
it('unmount aborts the in-flight fetch', async () => {
703+
let capturedSignal: AbortSignal | undefined;
704+
const fetchFn = jest.fn(
705+
(_url: RequestInfo | URL, init?: RequestInit): Promise<Response> => {
706+
capturedSignal = init?.signal ?? undefined;
707+
return new Promise<Response>((_resolve, reject) => {
708+
init?.signal?.addEventListener('abort', () => {
709+
const err = new Error('aborted');
710+
err.name = 'AbortError';
711+
reject(err);
712+
});
713+
});
714+
}
715+
);
716+
717+
const { result, unmount } = renderHook(() =>
718+
useChat({ api: '/chat', fetch: fetchFn })
719+
);
720+
721+
act(() => {
722+
void result.current.send('hi');
723+
});
724+
await waitFor(() => expect(fetchFn).toHaveBeenCalled());
725+
expect(capturedSignal?.aborted).toBe(false);
726+
727+
unmount();
728+
729+
expect(capturedSignal?.aborted).toBe(true);
730+
});
731+
659732
it('drops events that arrive after abort', async () => {
660733
let pushFn!: (event: AgentEvent) => void;
661734
let closeFn!: () => void;

packages/react/src/use-chat.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export function useChat(options: UseChatOptions): UseChatResult {
9494
const [isStreaming, setIsStreaming] = useState(false);
9595
const [pendingDecisions, setPendingDecisions] = useState<
9696
ReadonlyMap<string, ToolDecisionPendingEvent>
97-
>(() => new Map());
97+
>(() => rederivePendingDecisions(options.initialMessages ?? []));
9898
const [executingToolCallIds, setExecutingToolCallIds] = useState<ReadonlySet<string>>(
9999
() => new Set()
100100
);
@@ -293,6 +293,7 @@ export function useChat(options: UseChatOptions): UseChatResult {
293293
const userMessage: Message =
294294
typeof input === 'string' ? createUserMessage(input) : input;
295295
const requestMessages = [...messagesRef.current, userMessage];
296+
messagesRef.current = requestMessages;
296297
await runStream(requestMessages, userMessage);
297298
},
298299
[runStream]
@@ -358,6 +359,13 @@ export function useChat(options: UseChatOptions): UseChatResult {
358359
[]
359360
);
360361

362+
useEffect(() => {
363+
return () => {
364+
abortControllerRef.current?.abort();
365+
abortControllerRef.current = null;
366+
};
367+
}, []);
368+
361369
const abort = useCallback(() => {
362370
abortControllerRef.current?.abort();
363371
abortControllerRef.current = null;

0 commit comments

Comments
 (0)