Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/informal-green-porpoise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@inkeep/agents-api": minor
---

BREAKING: File parts on `/run/api/chat` use a Vercel-compatible shape (`url` and required `mediaType`; no `text` or `mimeType`). Add PDF URL ingestion for chat attachments with explicit bad-request errors on PDF URL ingest failures.
24 changes: 15 additions & 9 deletions agents-api/__snapshots__/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -39214,22 +39214,20 @@
"mediaType": {
"type": "string"
},
"mimeType": {
"type": "string"
},
"text": {
"type": "string"
},
"type": {
"enum": [
"file"
],
"type": "string"
},
"url": {
"type": "string"
}
},
"required": [
"type",
"text"
"url",
"mediaType"
],
"type": "object"
},
Expand Down Expand Up @@ -39903,8 +39901,16 @@
"file": {
"properties": {
"file_data": {
"pattern": "^data:application\\/pdf;base64,",
"type": "string"
"anyOf": [
{
"pattern": "^data:application\\/pdf;base64,",
"type": "string"
},
{
"format": "uri",
"type": "string"
}
]
},
"filename": {
"type": "string"
Expand Down
49 changes: 49 additions & 0 deletions agents-api/src/__tests__/run/routes/chat.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { HTTPException } from 'hono/http-exception';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as execModule from '../../../domains/run/handlers/executionHandler';
import { PdfUrlIngestionError } from '../../../domains/run/services/blob-storage/file-security-errors';
import { makeRequest } from '../../utils/testRequest';

// Mock context exports used by the chat route (routes/chat.ts imports from ../context)
Expand All @@ -16,6 +17,20 @@ vi.mock('../../../domains/run/context', () => ({
}),
}));

vi.mock(
'../../../domains/run/services/blob-storage/file-upload-helpers',
async (importOriginal) => {
const actual =
await importOriginal<
typeof import('../../../domains/run/services/blob-storage/file-upload-helpers')
>();
return {
...actual,
inlineExternalPdfUrlParts: vi.fn(async (parts) => parts),
};
}
);

// Mock Management API calls used by projectConfigMiddleware so tests don't hit network
const getFullProjectMock = vi.fn();

Expand Down Expand Up @@ -286,6 +301,40 @@ describe('Chat Routes', () => {
expect(response.headers.get('content-type')).toBe('text/event-stream');
});

it('should return 400 when PDF URL ingestion fails', async () => {
const { inlineExternalPdfUrlParts } = await import(
'../../../domains/run/services/blob-storage/file-upload-helpers'
);
vi.mocked(inlineExternalPdfUrlParts).mockRejectedValueOnce(
new PdfUrlIngestionError('https://example.com/report.pdf')
);

const response = await makeRequest('/run/v1/chat/completions', {
method: 'POST',
body: JSON.stringify({
model: 'claude-3-sonnet',
messages: [
{
role: 'user',
content: [
{ type: 'text', text: 'Summarize this document' },
{
type: 'file',
file: {
file_data: 'https://example.com/report.pdf',
filename: 'document.pdf',
},
},
],
},
],
conversationId: 'conv-123',
}),
});

expect(response.status).toBe(400);
});

it('should handle conversation creation', async () => {
const response = await makeRequest('/run/v1/chat/completions', {
method: 'POST',
Expand Down
174 changes: 149 additions & 25 deletions agents-api/src/__tests__/run/routes/chat/dataChat.test.ts
Original file line number Diff line number Diff line change
@@ -1,40 +1,60 @@
import { describe, expect, it, vi } from 'vitest';
import type { ExecutionHandlerParams } from '../../../../domains/run/handlers/executionHandler';
import { PdfUrlIngestionError } from '../../../../domains/run/services/blob-storage/file-security-errors';
import { pendingToolApprovalManager } from '../../../../domains/run/session/PendingToolApprovalManager';
import { toolApprovalUiBus } from '../../../../domains/run/session/ToolApprovalUiBus';

// Logger mock is now in setup.ts globally

const executionHandlerExecuteMock = vi
.fn()
.mockImplementation(async (args: ExecutionHandlerParams) => {
if (args.sseHelper && typeof args.sseHelper.writeRole === 'function') {
await args.sseHelper.writeRole();
await args.sseHelper.writeContent('[{"type":"text", "text":"Test response from agent"}]');
}

// Allow tests to simulate delegated approval UI propagation by publishing to the bus.
if (args.userMessage === '__trigger_approval_ui_bus__') {
await toolApprovalUiBus.publish(args.requestId, {
type: 'approval-needed',
toolCallId: 'call_bus_1',
toolName: 'delete_file',
input: { filePath: 'user/readme.md' },
providerMetadata: { openai: { itemId: 'fc_test' } },
approvalId: 'aitxt-call_bus_1',
});
await toolApprovalUiBus.publish(args.requestId, {
type: 'approval-resolved',
toolCallId: 'call_bus_1',
approved: true,
});
}
return { success: true, iterations: 1 };
});

vi.mock('../../../../domains/run/handlers/executionHandler', () => {
return {
ExecutionHandler: vi.fn().mockImplementation(() => ({
execute: vi.fn().mockImplementation(async (args: any) => {
if (args.sseHelper && typeof args.sseHelper.writeRole === 'function') {
await args.sseHelper.writeRole();
await args.sseHelper.writeContent('[{"type":"text", "text":"Test response from agent"}]');
}

// Allow tests to simulate delegated approval UI propagation by publishing to the bus.
if (args.userMessage === '__trigger_approval_ui_bus__') {
await toolApprovalUiBus.publish(args.requestId, {
type: 'approval-needed',
toolCallId: 'call_bus_1',
toolName: 'delete_file',
input: { filePath: 'user/readme.md' },
providerMetadata: { openai: { itemId: 'fc_test' } },
approvalId: 'aitxt-call_bus_1',
});
await toolApprovalUiBus.publish(args.requestId, {
type: 'approval-resolved',
toolCallId: 'call_bus_1',
approved: true,
});
}
return { success: true, iterations: 1 };
}),
execute: executionHandlerExecuteMock,
})),
};
});

vi.mock(
'../../../../domains/run/services/blob-storage/file-upload-helpers',
async (importOriginal) => {
const actual =
await importOriginal<
typeof import('../../../../domains/run/services/blob-storage/file-upload-helpers')
>();
return {
...actual,
inlineExternalPdfUrlParts: vi.fn(async (parts) => parts),
};
}
);

import { makeRequest } from '../../../utils/testRequest';

// Mock context exports used by the chat data stream routes
Expand Down Expand Up @@ -134,6 +154,48 @@ vi.mock('@inkeep/agents-core', async (importOriginal) => {
// No longer need beforeAll/afterAll since ExecutionHandler is mocked at module level

describe('Chat Data Stream Route', () => {
it('should pass inline image file parts through to ExecutionHandler', async () => {
executionHandlerExecuteMock.mockClear();

const body = {
stream: false,
messages: [
{
role: 'user',
content: 'Describe this image',
parts: [
{ type: 'text', text: 'Describe this image' },
{
type: 'file',
url: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO5w8R8AAAAASUVORK5CYII=',
mediaType: 'image/png',
filename: 'pixel.png',
},
],
},
],
};

const res = await makeRequest('/run/api/chat', {
method: 'POST',
body: JSON.stringify(body),
});

expect(res.status).toBe(200);
expect(executionHandlerExecuteMock).toHaveBeenCalledWith(
expect.objectContaining({
userMessage: 'Describe this image',
messageParts: expect.arrayContaining([
expect.objectContaining({ kind: 'text', text: 'Describe this image' }),
expect.objectContaining({
kind: 'file',
file: expect.objectContaining({ mimeType: 'image/png' }),
}),
]),
})
);
});

it('should stream response using Vercel data stream protocol', async () => {
// Ensure project exists first
// await createTestProject(dbClient, tenantId, projectId);
Expand Down Expand Up @@ -203,7 +265,7 @@ describe('Chat Data Stream Route', () => {
{ type: 'text', text: 'Summarize this PDF' },
{
type: 'file',
text: 'data:application/pdf;base64,JVBERi0xLjQK',
url: 'data:application/pdf;base64,JVBERi0xLjQK',
mediaType: 'application/pdf',
filename: 'doc.pdf',
},
Expand All @@ -221,6 +283,68 @@ describe('Chat Data Stream Route', () => {
expect(res.headers.get('x-vercel-ai-data-stream')).toBe('v2');
});

it('should accept inline image file part using Vercel FileUIPart shape', async () => {
const body = {
messages: [
{
role: 'user',
content: 'Describe this image',
parts: [
{ type: 'text', text: 'Describe this image' },
{
type: 'file',
url: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO5w8R8AAAAASUVORK5CYII=',
mediaType: 'image/png',
filename: 'pixel.png',
},
],
},
],
};

const res = await makeRequest('/run/api/chat', {
method: 'POST',
body: JSON.stringify(body),
});

expect(res.status).toBe(200);
expect(res.headers.get('x-vercel-ai-data-stream')).toBe('v2');
});

it('should return 400 when PDF URL ingestion fails', async () => {
const { inlineExternalPdfUrlParts } = await import(
'../../../../domains/run/services/blob-storage/file-upload-helpers'
);
vi.mocked(inlineExternalPdfUrlParts).mockRejectedValueOnce(
new PdfUrlIngestionError('https://example.com/report.pdf')
);

const body = {
messages: [
{
role: 'user',
content: 'Summarize this PDF',
parts: [
{ type: 'text', text: 'Summarize this PDF' },
{
type: 'file',
url: 'https://example.com/report.pdf',
mediaType: 'application/pdf',
filename: 'doc.pdf',
},
],
},
],
};

const res = await makeRequest('/run/api/chat', {
method: 'POST',
body: JSON.stringify(body),
});

expect(res.status).toBe(400);
});

it('should stream approval UI events published to ToolApprovalUiBus (simulating delegated agent approval)', async () => {
// Ensure deterministic requestId inside route subscription (chatds-${Date.now()})
const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(12345);
Expand Down
2 changes: 1 addition & 1 deletion agents-api/src/domains/run/handlers/executionHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import { tracer } from '../utils/tracer.js';

const logger = getLogger('ExecutionHandler');

interface ExecutionHandlerParams {
export interface ExecutionHandlerParams {
executionContext: FullExecutionContext;
conversationId: string;
userMessage: string;
Expand Down
15 changes: 13 additions & 2 deletions agents-api/src/domains/run/routes/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ import { flushBatchProcessor } from '../../../instrumentation';
import { getLogger } from '../../../logger';
import { contextValidationMiddleware, handleContextResolution } from '../context';
import { ExecutionHandler } from '../handlers/executionHandler';
import { buildPersistedMessageContent } from '../services/blob-storage/file-upload-helpers';
import { PdfUrlIngestionError } from '../services/blob-storage/file-security-errors';
import {
buildPersistedMessageContent,
inlineExternalPdfUrlParts,
} from '../services/blob-storage/file-upload-helpers';
import { toolApprovalUiBus } from '../session/ToolApprovalUiBus';
import { createSSEStreamHelper } from '../stream/stream-helpers';
import type { Message } from '../types/chat';
Expand Down Expand Up @@ -314,9 +318,10 @@ app.openapi(chatCompletionsRoute, async (c) => {
.slice(-1)[0];

// Build Part[] for execution (text + image parts), validated against core PartSchema
const messageParts = z
const parsedMessageParts = z
.array(PartSchema)
.parse(lastUserMessage ? getMessagePartsFromOpenAIContent(lastUserMessage.content) : []);
const messageParts = await inlineExternalPdfUrlParts(parsedMessageParts);

// Extract text content from parts
const userMessage = extractTextFromParts(messageParts);
Expand Down Expand Up @@ -547,6 +552,12 @@ app.openapi(chatCompletionsRoute, async (c) => {
});
});
} catch (error) {
if (error instanceof PdfUrlIngestionError) {
throw createApiError({
code: 'bad_request',
message: error.message,
});
}
Comment on lines +555 to +560
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 Consider: Add logging before converting to API error

Issue: The cause chain from PdfUrlIngestionError is not logged before it's converted to a generic 400. Operators won't have visibility into why ingestion failed (DNS failure? Timeout? Invalid bytes?).

Why: Debugging production issues becomes harder without the underlying cause in logs.

Fix:

Suggested change
if (error instanceof PdfUrlIngestionError) {
throw createApiError({
code: 'bad_request',
message: error.message,
});
}
if (error instanceof PdfUrlIngestionError) {
logger.warn(
{ sourceUrl: error.sourceUrl, cause: error.cause?.message },
'PDF URL ingestion failed'
);
throw createApiError({
code: 'bad_request',
message: error.message,
});
}

if (error instanceof HTTPException) {
throw error;
}
Expand Down
Loading
Loading