From 43f9b332cd5a8840d451b9148e38fef3c5908a2a Mon Sep 17 00:00:00 2001 From: Cameron Date: Mon, 23 Feb 2026 17:11:31 -0800 Subject: [PATCH] fix(api): surface failed stream results and validate content types (#379) --- src/api/openai-compat.test.ts | 81 +++++++++++++++++++++++++++++++++++ src/api/openai-compat.ts | 8 +++- src/api/server.ts | 13 ++++-- 3 files changed, 96 insertions(+), 6 deletions(-) diff --git a/src/api/openai-compat.test.ts b/src/api/openai-compat.test.ts index 655b3c4..86a6685 100644 --- a/src/api/openai-compat.test.ts +++ b/src/api/openai-compat.test.ts @@ -123,6 +123,23 @@ describe('openai-compat utilities', () => { expect(extractLastUserMessage([])).toBeNull(); }); + it('skips non-string content (number, object, array)', () => { + const messages = [ + { role: 'user', content: 42 }, + { role: 'user', content: { text: 'hi' } }, + { role: 'user', content: ['hi'] }, + ] as unknown as OpenAIChatMessage[]; + expect(extractLastUserMessage(messages)).toBeNull(); + }); + + it('finds string content after non-string content', () => { + const messages = [ + { role: 'user', content: 'valid' }, + { role: 'user', content: 42 }, + ] as unknown as OpenAIChatMessage[]; + expect(extractLastUserMessage(messages)).toBe('valid'); + }); + it('handles only one user message', () => { const messages: OpenAIChatMessage[] = [{ role: 'user', content: 'Only message' }]; expect(extractLastUserMessage(messages)).toBe('Only message'); @@ -328,6 +345,37 @@ describe('openai-compat utilities', () => { }); expect(result).toBeNull(); }); + + it('returns 400 for non-string content (number)', () => { + const err = validateChatRequest({ + messages: [{ role: 'user', content: 42 }], + }); + expect(err).not.toBeNull(); + expect(err!.status).toBe(400); + expect(err!.body.error.message).toContain('string or null'); + }); + + it('returns 400 for object content', () => { + const err = validateChatRequest({ + messages: [{ role: 'user', content: { text: 'hi' } }], + }); + expect(err).not.toBeNull(); + expect(err!.status).toBe(400); + }); + + it('accepts null content', () => { + const result = validateChatRequest({ + messages: [{ role: 'assistant', content: null }], + }); + expect(result).toBeNull(); + }); + + it('accepts undefined content (absent)', () => { + const result = validateChatRequest({ + messages: [{ role: 'user' }], + }); + expect(result).toBeNull(); + }); }); }); @@ -831,4 +879,37 @@ describe('POST /v1/chat/completions', () => { expect(toolChunks[0].choices[0].delta.tool_calls[0].function.name).toBe('tool1'); expect(toolChunks[1].choices[0].delta.tool_calls[0].function.name).toBe('tool2'); }); + + it('emits error content when stream result indicates failure', async () => { + (router as any).streamToAgent = vi.fn().mockReturnValue( + (async function* () { + yield { type: 'assistant', content: 'Partial' }; + yield { type: 'result', success: false, error: 'Rate limited' }; + })(), + ); + + const body = JSON.stringify({ + model: 'LettaBot', + messages: [{ role: 'user', content: 'Test' }], + stream: true, + }); + const res = await request(port, 'POST', '/v1/chat/completions', body, { + 'content-type': 'application/json', + 'x-api-key': TEST_API_KEY, + }); + + expect(res.status).toBe(200); + const events = res.body + .split('\n\n') + .filter((line) => line.startsWith('data: ')) + .map((line) => line.replace('data: ', '')) + .filter((line) => line !== '[DONE]') + .map((line) => JSON.parse(line)); + + // Should have: role chunk, 'Partial' chunk, error chunk, stop chunk + const contentChunks = events.filter((e: any) => e.choices[0].delta.content); + const contents = contentChunks.map((e: any) => e.choices[0].delta.content).join(''); + expect(contents).toContain('Partial'); + expect(contents).toContain('[Error: Rate limited]'); + }); }); diff --git a/src/api/openai-compat.ts b/src/api/openai-compat.ts index 9c06db1..f78231a 100644 --- a/src/api/openai-compat.ts +++ b/src/api/openai-compat.ts @@ -164,7 +164,7 @@ export function generateCompletionId(): string { */ export function extractLastUserMessage(messages: OpenAIChatMessage[]): string | null { for (let i = messages.length - 1; i >= 0; i--) { - if (messages[i].role === 'user' && messages[i].content) { + if (messages[i].role === 'user' && typeof messages[i].content === 'string' && messages[i].content) { return messages[i].content as string; } } @@ -301,7 +301,7 @@ export function validateChatRequest(body: unknown): { status: number; body: Open return buildErrorResponse('messages is required and must be a non-empty array', 'invalid_request_error', 400); } - // Validate each message has role and content + // Validate each message has role and valid content for (const msg of req.messages) { if (!msg || typeof msg !== 'object') { return buildErrorResponse('Each message must be an object', 'invalid_request_error', 400); @@ -310,6 +310,10 @@ export function validateChatRequest(body: unknown): { status: number; body: Open if (!m.role || typeof m.role !== 'string') { return buildErrorResponse('Each message must have a role', 'invalid_request_error', 400); } + // content must be string, null, or absent -- reject numbers, arrays, objects + if (m.content !== undefined && m.content !== null && typeof m.content !== 'string') { + return buildErrorResponse('Message content must be a string or null', 'invalid_request_error', 400); + } } return null; diff --git a/src/api/server.ts b/src/api/server.ts index 66f5a0c..2778ab5 100644 --- a/src/api/server.ts +++ b/src/api/server.ts @@ -326,7 +326,7 @@ export function createApiServer(deliverer: AgentRouter, options: ServerOptions): res.writeHead(200, { 'Content-Type': 'application/json' }); res.end(JSON.stringify(models)); } catch (error: any) { - console.error('[API] Models error:', error); + log.error('Models error:', error); const err = buildErrorResponse(error.message || 'Internal server error', 'server_error', 500); res.writeHead(err.status, { 'Content-Type': 'application/json' }); res.end(JSON.stringify(err.body)); @@ -409,7 +409,7 @@ export function createApiServer(deliverer: AgentRouter, options: ServerOptions): const completionId = generateCompletionId(); const context = { type: 'webhook' as const, outputMode: 'silent' as const }; - console.log(`[API] OpenAI chat: model="${modelName}", stream=${!!chatReq.stream}, msg="${userMessage.slice(0, 100)}..."`); + log.info(`OpenAI chat: model="${modelName}", stream=${!!chatReq.stream}, msg="${userMessage.slice(0, 100)}..."`); if (chatReq.stream) { // ---- Streaming response ---- @@ -443,7 +443,12 @@ export function createApiServer(deliverer: AgentRouter, options: ServerOptions): completionId, modelName, toolIndex++, toolCallId, toolName, args, ))); } else if (msg.type === 'result') { - // Final chunk + if (!(msg as any).success) { + const errMsg = (msg as any).error || 'Agent run failed'; + res.write(formatSSE(buildChunk(completionId, modelName, { + content: `\n\n[Error: ${errMsg}]`, + }))); + } break; } // Skip 'reasoning', 'tool_result', and other internal types @@ -471,7 +476,7 @@ export function createApiServer(deliverer: AgentRouter, options: ServerOptions): res.end(JSON.stringify(completion)); } } catch (error: any) { - console.error('[API] OpenAI chat error:', error); + log.error('OpenAI chat error:', error); const err = buildErrorResponse(error.message || 'Internal server error', 'server_error', 500); res.writeHead(err.status, { 'Content-Type': 'application/json' }); res.end(JSON.stringify(err.body));