diff --git a/api/server/controllers/agents/__tests__/callbacks.spec.js b/api/server/controllers/agents/__tests__/callbacks.spec.js index 3c990bc7a0..0ba20d409c 100644 --- a/api/server/controllers/agents/__tests__/callbacks.spec.js +++ b/api/server/controllers/agents/__tests__/callbacks.spec.js @@ -575,3 +575,39 @@ describe('createToolEndCallback', () => { }); }); }); + +describe('isStreamWritable', () => { + /* Direct parametric coverage of the predicate that gates SSE writes + * in both the chat-completions and Open Responses callbacks. The + * existing deferred-preview tests cover this indirectly via the + * `writeAttachmentUpdate` writableEnded path; these tests pin down + * each individual branch so a future modification (e.g. adding a + * new condition) can't silently regress. + * (Comprehensive review NIT on PR #12957.) */ + const { isStreamWritable } = require('../callbacks'); + + it('returns true when streamId is truthy regardless of res state', () => { + /* Resumable mode writes go to the job emitter; res state is + * irrelevant. Even a closed res with no headers should not block. */ + expect(isStreamWritable(null, 'stream-1')).toBe(true); + expect(isStreamWritable({ headersSent: false, writableEnded: true }, 'stream-1')).toBe(true); + expect(isStreamWritable(undefined, 'stream-1')).toBe(true); + }); + + it('returns false when streamId is falsy and res is null/undefined', () => { + expect(isStreamWritable(null, null)).toBe(false); + expect(isStreamWritable(undefined, null)).toBe(false); + }); + + it('returns false when headers have not been sent yet', () => { + expect(isStreamWritable({ headersSent: false, writableEnded: false }, null)).toBe(false); + }); + + it('returns false when the stream has already ended', () => { + expect(isStreamWritable({ headersSent: true, writableEnded: true }, null)).toBe(false); + }); + + it('returns true on the happy path: headers sent, not ended, no streamId', () => { + expect(isStreamWritable({ headersSent: true, writableEnded: false }, null)).toBe(true); + }); +}); diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index 58eda1ed1b..c9612c6b62 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -641,8 +641,16 @@ function createToolEndCallback({ req, res, artifactPromises, streamId = null }) /* Initial emit: ship the attachment to the client immediately * (carries `status: 'pending'` for office buckets so the UI * shows "preparing preview…"). The agent's response stops - * blocking on extraction here. */ - if (streamId || res.headersSent) { + * blocking on extraction here. + * + * Use the shared `isStreamWritable` predicate rather than the + * narrower `streamId || res.headersSent` check that lived + * here before — a client disconnect mid-stream + * (`res.writableEnded`) would otherwise hit `res.write` and + * raise `ERR_STREAM_WRITE_AFTER_END` (caught by the outer + * IIFE catch but logged as noise). Same gate the Responses + * path uses below. */ + if (isStreamWritable(res, streamId)) { writeAttachment(res, streamId, fileMetadata); } /* Deferred preview rendering: extraction continues running @@ -1017,6 +1025,7 @@ module.exports = { agentLogHandlerObj, getDefaultHandlers, createToolEndCallback, + isStreamWritable, markSummarizationUsage, buildSummarizationHandlers, createResponsesToolEndCallback,