test(ai-chat): pin the run-detach abortSignal wiring (#184)
F3: the load-bearing `effectiveSignal = handle.signal` -> streamText `abortSignal` had no test; a regression to the socket-bound signal would pass green and silently break Stop + durability. Add a happy-path test (runHooks.begin returns the run signal -> streamText is driven with abortSignal === handle.signal, NOT the socket) and a legacy-path test (no runHooks -> the socket signal is used). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
import { ConflictException } from '@nestjs/common';
|
||||
import { ConflictException, Logger } from '@nestjs/common';
|
||||
|
||||
// Mock the AI SDK so we can PROVE no provider call is made for the turn we are
|
||||
// about to reject. The race rejection happens at runHooks.begin(), long before
|
||||
@@ -98,3 +98,148 @@ describe('AiChatService.stream — concurrent-run race rejection (#184)', () =>
|
||||
expect(aiChatMessageRepo.insert).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* F3 — the LOAD-BEARING run-detach wiring: `effectiveSignal = handle.signal`
|
||||
* after runHooks.begin, then `abortSignal: effectiveSignal` passed to streamText.
|
||||
* That single line is what makes a run survive a browser disconnect (the agent
|
||||
* loop's abort is governed by the RUN's signal, not the socket): a regression to
|
||||
* the socket-bound signal would still pass every other test green while silently
|
||||
* breaking Stop + durability. These two tests pin the exact signal streamText
|
||||
* consumes on both paths.
|
||||
*/
|
||||
describe('AiChatService.stream — abortSignal wiring (#184 F3)', () => {
|
||||
const streamTextMock = streamText as unknown as jest.Mock;
|
||||
|
||||
// A streamText result stub: the post-call drain + pipe are no-ops here; we only
|
||||
// care WHICH abortSignal streamText was handed.
|
||||
function makeStreamResult() {
|
||||
return {
|
||||
consumeStream: jest.fn(),
|
||||
pipeUIMessageStreamToResponse: jest.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
// A raw-response stub sufficient for the post-streamText wiring
|
||||
// (stripStreamingHopByHopHeaders binds writeHead; startSseHeartbeat registers
|
||||
// close/finish listeners; flushHeaders is belt-and-braces).
|
||||
function makeRes() {
|
||||
return {
|
||||
raw: {
|
||||
writeHead: jest.fn(),
|
||||
write: jest.fn(),
|
||||
once: jest.fn(),
|
||||
on: jest.fn(),
|
||||
flushHeaders: jest.fn(),
|
||||
writableEnded: false,
|
||||
destroyed: false,
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
// Wire only the deps reached on the way to streamText: resolve the existing
|
||||
// chat, persist the user + seed the assistant row, load (empty) history, the
|
||||
// admin settings, an empty external toolset + Docmost toolset.
|
||||
function makeService() {
|
||||
const aiChatRepo = {
|
||||
findById: jest.fn(async () => ({ id: 'chat-1', workspaceId: 'ws-1' })),
|
||||
insert: jest.fn(),
|
||||
};
|
||||
const aiChatMessageRepo = {
|
||||
insert: jest.fn(async () => ({ id: 'msg-1' })),
|
||||
findAllByChat: jest.fn(async () => []),
|
||||
update: jest.fn(async () => ({ id: 'msg-1' })),
|
||||
};
|
||||
const aiSettings = { resolve: jest.fn(async () => ({})) };
|
||||
const tools = { forUser: jest.fn(async () => ({})) };
|
||||
const mcpClients = {
|
||||
toolsFor: jest.fn(async () => ({
|
||||
tools: {},
|
||||
clients: [],
|
||||
outcomes: [],
|
||||
instructions: [],
|
||||
})),
|
||||
};
|
||||
const svc = new AiChatService(
|
||||
{} as never, // ai
|
||||
aiChatRepo as never,
|
||||
aiChatMessageRepo as never,
|
||||
aiSettings as never,
|
||||
tools as never,
|
||||
mcpClients as never,
|
||||
{} as never, // aiAgentRoleRepo
|
||||
{} as never, // pageRepo (openPage undefined -> never touched)
|
||||
{} as never, // pageAccess
|
||||
);
|
||||
return { svc };
|
||||
}
|
||||
|
||||
const body = {
|
||||
chatId: 'chat-1',
|
||||
messages: [
|
||||
{ id: 'm1', role: 'user', parts: [{ type: 'text', text: 'hi' }] },
|
||||
],
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
streamTextMock.mockReset();
|
||||
streamTextMock.mockImplementation(() => makeStreamResult());
|
||||
jest
|
||||
.spyOn(Logger.prototype, 'log')
|
||||
.mockImplementation(() => undefined as never);
|
||||
});
|
||||
|
||||
afterEach(() => jest.restoreAllMocks());
|
||||
|
||||
it('happy path (run-wrapped): streamText is driven with abortSignal === handle.signal (the RUN signal, NOT the socket)', async () => {
|
||||
const { svc } = makeService();
|
||||
const runController = new AbortController();
|
||||
const runSignal = runController.signal;
|
||||
const socketSignal = new AbortController().signal;
|
||||
|
||||
const begin = jest.fn(async () => ({ runId: 'run-1', signal: runSignal }));
|
||||
await svc.stream({
|
||||
user: { id: 'user-1' } as never,
|
||||
workspace: { id: 'ws-1' } as never,
|
||||
sessionId: 'sess-1',
|
||||
body: body as never,
|
||||
res: makeRes() as never,
|
||||
signal: socketSignal,
|
||||
model: {} as never,
|
||||
role: null,
|
||||
runHooks: {
|
||||
begin,
|
||||
onAssistantSeeded: jest.fn(),
|
||||
onStep: jest.fn(),
|
||||
onSettled: jest.fn(),
|
||||
} as never,
|
||||
});
|
||||
|
||||
expect(begin).toHaveBeenCalledTimes(1);
|
||||
expect(streamTextMock).toHaveBeenCalledTimes(1);
|
||||
// THE assertion: the agent loop's abort is wired to the RUN, so a browser
|
||||
// disconnect (which aborts only `socketSignal`) cannot end the turn.
|
||||
expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(runSignal);
|
||||
expect(streamTextMock.mock.calls[0][0].abortSignal).not.toBe(socketSignal);
|
||||
});
|
||||
|
||||
it('legacy path (no runHooks): streamText is driven with the SOCKET signal', async () => {
|
||||
const { svc } = makeService();
|
||||
const socketSignal = new AbortController().signal;
|
||||
|
||||
await svc.stream({
|
||||
user: { id: 'user-1' } as never,
|
||||
workspace: { id: 'ws-1' } as never,
|
||||
sessionId: 'sess-1',
|
||||
body: body as never,
|
||||
res: makeRes() as never,
|
||||
signal: socketSignal,
|
||||
model: {} as never,
|
||||
role: null,
|
||||
// No runHooks -> the turn stays socket-bound (flag off / default).
|
||||
});
|
||||
|
||||
expect(streamTextMock).toHaveBeenCalledTimes(1);
|
||||
expect(streamTextMock.mock.calls[0][0].abortSignal).toBe(socketSignal);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user