diff --git a/packages/attachment/attachment-local/src/index.ts b/packages/attachment/attachment-local/src/index.ts index 516280548c..4fb200345b 100644 --- a/packages/attachment/attachment-local/src/index.ts +++ b/packages/attachment/attachment-local/src/index.ts @@ -93,7 +93,11 @@ class SharedRequest { wait(signal?: AbortSignal): Promise { signal?.throwIfAborted() this.waiters += 1 - if (signal === undefined) return this.promise.finally(() => this.release(false)) + if (signal === undefined) { + return this.promise.finally(() => { + this.release(false) + }) + } let released = false const release = (cancelled: boolean): void => { if (released) return @@ -113,6 +117,8 @@ class SharedRequest { }, (error: unknown) => { signal.removeEventListener('abort', abort) release(false) + // CompressionLimiter normalizes task rejections before this handler. + // oxlint-disable-next-line typescript/prefer-promise-reject-errors reject(error) }) }) diff --git a/packages/attachment/attachment-local/tests/encoding.spec.ts b/packages/attachment/attachment-local/tests/encoding.spec.ts index d75fc9b4b3..8cd7a60540 100644 --- a/packages/attachment/attachment-local/tests/encoding.spec.ts +++ b/packages/attachment/attachment-local/tests/encoding.spec.ts @@ -83,6 +83,7 @@ describe('CompressionLimiter', () => { it('normalizes a non-Error rejection and releases its slot', async () => { const limiter = new CompressionLimiter(1) + // oxlint-disable-next-line typescript/prefer-promise-reject-errors -- Native bindings can reject non-Error values. const failed = limiter.run(() => Promise.reject('native failure')) const next = limiter.run(() => Promise.resolve('next')) diff --git a/packages/attachment/attachment-local/tests/request-image.spec.ts b/packages/attachment/attachment-local/tests/request-image.spec.ts index e33522c104..726837a242 100644 --- a/packages/attachment/attachment-local/tests/request-image.spec.ts +++ b/packages/attachment/attachment-local/tests/request-image.spec.ts @@ -340,7 +340,9 @@ describe('local request-image cache', () => { const read = vi.spyOn(attachments, 'readImage').mockImplementation((_ref, signal) => { readSignal = signal return new Promise((_resolve, reject) => { - signal?.addEventListener('abort', () => reject(signal.reason), { once: true }) + signal?.addEventListener('abort', () => { + reject(new Error('request transform aborted', { cause: signal.reason })) + }, { once: true }) }) }) const controller = new AbortController() @@ -349,7 +351,9 @@ describe('local request-image cache', () => { { maxPixels: 640_000, maxBytes: 1024 * 1024 }, controller.signal, ) - await vi.waitFor(() => expect(read).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => { + expect(read).toHaveBeenCalledTimes(1) + }) const reason = new Error('cancel only transform waiter') controller.abort(reason) @@ -369,7 +373,9 @@ describe('local request-image cache', () => { calls += 1 if (calls === 1) { return new Promise((_resolve, reject) => { - signal?.addEventListener('abort', () => reject(signal.reason), { once: true }) + signal?.addEventListener('abort', () => { + reject(new Error('request transform aborted', { cause: signal.reason })) + }, { once: true }) }) } return actualRead(ref, signal) @@ -377,7 +383,9 @@ describe('local request-image cache', () => { const controller = new AbortController() const policy = { maxPixels: 640_000, maxBytes: 1024 * 1024 } const cancelled = attachments.readImageRequest(master, policy, controller.signal) - await vi.waitFor(() => expect(calls).toBe(1)) + await vi.waitFor(() => { + expect(calls).toBe(1) + }) controller.abort('cancelled') const replacement = attachments.readImageRequest(master, policy) @@ -389,4 +397,5 @@ describe('local request-image cache', () => { await expect(replacement).resolves.toMatchObject({ width: 1130, height: 565 }) expect(calls).toBe(2) }) + }) diff --git a/packages/fs/tool-fs/tests/read-image.spec.ts b/packages/fs/tool-fs/tests/read-image.spec.ts index 3bf86d27f5..03616911b5 100644 --- a/packages/fs/tool-fs/tests/read-image.spec.ts +++ b/packages/fs/tool-fs/tests/read-image.spec.ts @@ -251,6 +251,33 @@ describe('read_image_region', () => { expect(result.isError).toBe(false) }) + it('continues across an earlier session message without the requested image', async () => { + const ctx = await setup() + const source = await ctx.attachments.saveImage({ data: PNG_3X3, mediaType: 'image/png' }) + const history = [ + createUserMessage({ + content: [{ type: 'text', text: 'before image' }], + source: { kind: 'plugin', plugin: 'test' }, + }), + createUserMessage({ + content: [{ type: 'image', attachment: source.ref }], + source: { kind: 'plugin', plugin: 'test' }, + }), + ] + + const result = await call(ctx, 'read_image_region', { + attachment_id: source.ref.attachmentId, + preview_width: 3, + preview_height: 3, + x: 0, + y: 0, + width: 1, + height: 1, + }, agentOn('vision-model', 'visual', history)) + + expect(result.isError).toBe(false) + }) + it('rejects a missing session, empty id, and invalid coordinate arguments', async () => { const ctx = await setup() const base = { diff --git a/packages/interaction/commands/tests/commands.spec.ts b/packages/interaction/commands/tests/commands.spec.ts index c65fb49bed..85806a1d36 100644 --- a/packages/interaction/commands/tests/commands.spec.ts +++ b/packages/interaction/commands/tests/commands.spec.ts @@ -487,9 +487,10 @@ describe('image attachments', () => { }) }), validateImageBatch(inputs: readonly unknown[]) { - return (AttachmentStore.prototype as unknown as { + const validate = AttachmentStore.prototype as unknown as { validateImageBatch(this: unknown, batch: readonly unknown[]): void - }).validateImageBatch.call(this, inputs) + } + validate.validateImageBatch.call(this, inputs) }, // The real base-class batch method over this double's limits and members. saveImages(inputs: readonly unknown[]) { diff --git a/packages/llm/llm-deepseek/src/file-store.ts b/packages/llm/llm-deepseek/src/file-store.ts index ddaefd1eee..0757b42db2 100644 --- a/packages/llm/llm-deepseek/src/file-store.ts +++ b/packages/llm/llm-deepseek/src/file-store.ts @@ -50,33 +50,44 @@ function abortReason(signal: AbortSignal): Error { : new Error('DeepSeek file upload cancelled with a non-Error reason.', { cause: reason }) } +function uploadFailure(error: unknown): Error { + return error instanceof Error + ? error + : new Error('DeepSeek file upload failed with a non-Error reason.', { cause: error }) +} + function waitForUpload(operation: SharedUpload, signal: AbortSignal | undefined): Promise { signal?.throwIfAborted() operation.waiters += 1 let released = false - const release = (cancelled: boolean): void => { + const release = (cancelledReason?: Error): void => { if (released) return released = true operation.waiters -= 1 - if (cancelled && operation.waiters === 0 && !operation.settled) { - operation.controller.abort(signal === undefined ? undefined : abortReason(signal)) + if (cancelledReason !== undefined && operation.waiters === 0 && !operation.settled) { + operation.controller.abort(cancelledReason) } } - if (signal === undefined) return operation.promise.finally(() => release(false)) + if (signal === undefined) { + return operation.promise.finally(() => { + release() + }) + } return new Promise((resolve, reject) => { const abort = (): void => { - release(true) - reject(abortReason(signal)) + const reason = abortReason(signal) + release(reason) + reject(reason) } signal.addEventListener('abort', abort, { once: true }) void operation.promise.then((value) => { signal.removeEventListener('abort', abort) - release(false) + release() resolve(value) }, (error: unknown) => { signal.removeEventListener('abort', abort) - release(false) - reject(error) + release() + reject(uploadFailure(error)) }) }) } @@ -155,7 +166,7 @@ export class DeepSeekFileStore { return value }, (error: unknown) => { shared.settled = true - throw error + throw uploadFailure(error) }) this.inflight.set(key, shared) void shared.promise.finally(() => { @@ -168,7 +179,7 @@ export class DeepSeekFileStore { version: RequestImageAttachment, connection: DeepSeekFileConnection, policy: DeepSeekFilePolicy, - signal?: AbortSignal, + signal: AbortSignal, ): Promise { if (version.bytes > MAX_CHAT_IMAGE_BYTES) { throw new LlmError('DeepSeek chat image exceeds the 32 MiB per-image limit.', 'INVALID_REQUEST') @@ -186,9 +197,9 @@ export class DeepSeekFileStore { mediaType: version.mediaType, filename: filename(version), expiresAfterSeconds: policy.expiresAfterSeconds, - ...signal === undefined ? {} : { signal }, + signal, }) - if (remote.bytes !== version.data.byteLength || remote.expiresAt === undefined) { + if (remote.bytes !== version.data.byteLength) { throw new LlmError('DeepSeek Files API upload response does not match the submitted image.', 'INVALID_RESPONSE') } return { diff --git a/packages/llm/llm-deepseek/src/files-api.ts b/packages/llm/llm-deepseek/src/files-api.ts index f19823100e..cc998b2e7e 100644 --- a/packages/llm/llm-deepseek/src/files-api.ts +++ b/packages/llm/llm-deepseek/src/files-api.ts @@ -143,7 +143,6 @@ export class DeepSeekFilesClient { let response: Response try { const headers = new Headers(attributionHeaders()) - for (const [name, value] of new Headers(init.headers)) headers.set(name, value) headers.set('authorization', `Bearer ${this.apiKey}`) response = await this.fetchImpl(`${this.baseURL}${path}`, { ...init, @@ -180,7 +179,7 @@ export class DeepSeekFilesClient { filename: string expiresAfterSeconds: number signal?: AbortSignal - }): Promise { + }): Promise { if (input.data.byteLength > MAX_FILE_UPLOAD_BYTES) { throw new LlmError('DeepSeek Files API upload exceeds 128 MiB.', 'INVALID_REQUEST') } @@ -197,7 +196,7 @@ export class DeepSeekFilesClient { const response = await this.request('/files', { method: 'POST', body: form }, input.signal) const file = parseFileObject(await response.json(), 'upload') if (file.expiresAt === undefined) throw invalidResponse('upload') - return file + return { ...file, expiresAt: file.expiresAt } } /** diff --git a/packages/llm/llm-deepseek/src/serialize.ts b/packages/llm/llm-deepseek/src/serialize.ts index a65c9750c3..b998b23a8b 100644 --- a/packages/llm/llm-deepseek/src/serialize.ts +++ b/packages/llm/llm-deepseek/src/serialize.ts @@ -317,7 +317,7 @@ export async function serializeMessagesWithImages( wire.push({ role: 'tool', tool_call_id: result.toolCallId, - content: text || (fileParts.length > 0 ? '(see attached image)' : '(no output)'), + content: text || '(no output)', }) pendingToolImages.push(...fileParts) } diff --git a/packages/llm/llm-deepseek/tests/adapter.spec.ts b/packages/llm/llm-deepseek/tests/adapter.spec.ts index 731df2eb0f..2663c4cd78 100644 --- a/packages/llm/llm-deepseek/tests/adapter.spec.ts +++ b/packages/llm/llm-deepseek/tests/adapter.spec.ts @@ -176,6 +176,7 @@ describe('DeepSeekAdapter against a mock server', () => { await drain(adapter.stream({ provider: 'deepseek-official', model: 'deepseek-v4-flash-vision-exp', + tools: [{ name: 'read_image_region', description: 'crop', parameters: { type: 'object' } }], messages: [createUserMessage({ content: [ { type: 'text', text: 'describe ' }, @@ -191,7 +192,7 @@ describe('DeepSeekAdapter against a mock server', () => { role: 'user', content: [ { type: 'text', text: 'describe ' }, - { type: 'text', text: expect.stringContaining(`Image ${imageRef.attachmentId}`) as string }, + { type: 'text', text: expect.stringContaining('Call read_image_region') as string }, { type: 'file', file_id: 'file-api-1' }, ], }], @@ -1499,6 +1500,23 @@ describe('plugin registration and config', () => { .toThrow(/maxTokens must be a positive integer/) }) + it('rejects image request limits on a text-only catalog model', () => { + expect(() => resolveAdapterOptions({ + models: [{ id: 'text-only', inputModalities: ['text'], imagePixelBudget: 1 }], + })).toThrow(/text-only catalog model .* cannot declare image request limits/) + }) + + it.each([ + ['imagePixelBudget', 0, /imagePixelBudget must be a positive safe integer/], + ['imagePixelBudget', Number.MAX_SAFE_INTEGER + 1, /imagePixelBudget must be a positive safe integer/], + ['imageMaxBytes', 0, /imageMaxBytes must be a positive safe integer/], + ['imageMaxBytes', 1.5, /imageMaxBytes must be a positive safe integer/], + ] as const)('rejects per-model %s=%s', (field, value, message) => { + expect(() => resolveAdapterOptions({ + models: [{ id: 'vision', inputModalities: ['image'], [field]: value }], + })).toThrow(message) + }) + it('prefers a model\'s own output cap over the profile default', async () => { // The profile default stays what an unlisted or uncapped model resolves // to, so adding a per-model cap changes one model rather than the route. @@ -1569,6 +1587,23 @@ describe('plugin registration and config', () => { })).toThrow(/imageOffloadCountQuantum must not exceed maxImagesPerRequest/) }) + it.each([ + ['maxImagesPerRequest', 0, /maxImagesPerRequest must be a positive safe integer/], + ['maxImagesPerRequest', 1.5, /maxImagesPerRequest must be a positive safe integer/], + ['imageOffloadByteQuantum', 0, /imageOffloadByteQuantum must be a positive safe integer/], + ['imageOffloadByteQuantum', Number.MAX_SAFE_INTEGER + 1, /imageOffloadByteQuantum must be a positive safe integer/], + ['imageOffloadCountQuantum', 0, /imageOffloadCountQuantum must be a positive safe integer/], + ['imageOffloadCountQuantum', 1.5, /imageOffloadCountQuantum must be a positive safe integer/], + ['fileExpiresAfterSeconds', 3_599, /fileExpiresAfterSeconds must be an integer from 3600 through 2592000/], + ['fileExpiresAfterSeconds', 2_592_001, /fileExpiresAfterSeconds must be an integer from 3600 through 2592000/], + ['fileRefreshMarginSeconds', -1, /fileRefreshMarginSeconds must be a non-negative integer/], + ['fileRefreshMarginSeconds', 604_800, /fileRefreshMarginSeconds must be a non-negative integer/], + ['fileQuotaCleanupBatch', 0, /fileQuotaCleanupBatch must be an integer from 1 through 1000/], + ['fileQuotaCleanupBatch', 1_001, /fileQuotaCleanupBatch must be an integer from 1 through 1000/], + ] as const)('rejects %s=%s', (field, value, message) => { + expect(() => resolveAdapterOptions({ [field]: value })).toThrow(message) + }) + it.each([0, 1.5, Number.MAX_SAFE_INTEGER + 1])( 'rejects invalid request file bound %s', async (maxRequestFilesBytes) => { diff --git a/packages/llm/llm-deepseek/tests/file-store.spec.ts b/packages/llm/llm-deepseek/tests/file-store.spec.ts index 6d9e940786..069ff47c9d 100644 --- a/packages/llm/llm-deepseek/tests/file-store.spec.ts +++ b/packages/llm/llm-deepseek/tests/file-store.spec.ts @@ -4,8 +4,9 @@ import { join } from 'node:path' import { describe, expect, it, vi } from 'vitest' import { AttachmentId, ImageVariantId } from '@deepseek-ai/dsh-attachment' import type { ImageAttachmentRef, RequestImageAttachment } from '@deepseek-ai/dsh-attachment' -import { DeepSeekFileStore } from '../src/file-store.ts' -import { DeepSeekUploadIndex } from '../src/upload-index.ts' +import { DeepSeekFileStore, MAX_CHAT_IMAGE_BYTES } from '../src/file-store.ts' +import { DeepSeekFileId } from '../src/file-id.ts' +import { deepSeekFileScope, DeepSeekUploadIndex } from '../src/upload-index.ts' const REF: ImageAttachmentRef = { attachmentId: AttachmentId(`sha256:${'a'.repeat(64)}`), @@ -90,7 +91,9 @@ describe('DeepSeekFileStore', () => { uploadSignal = init?.signal ?? undefined return new Promise((resolve, reject) => { complete = resolve - uploadSignal?.addEventListener('abort', () => reject(uploadSignal?.reason), { once: true }) + uploadSignal?.addEventListener('abort', () => { + reject(new Error('upload aborted', { cause: uploadSignal?.reason })) + }, { once: true }) }) }) as typeof fetch const store = new DeepSeekFileStore({ index, now: () => NOW, fetch: fetchImpl }) @@ -98,7 +101,9 @@ describe('DeepSeekFileStore', () => { const cancelled = store.ensureUploaded(VERSION, CONNECTION, POLICY, controller.signal) const completed = store.ensureUploaded(VERSION, CONNECTION, POLICY) - await vi.waitFor(() => expect(fetchImpl).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => { + expect(fetchImpl).toHaveBeenCalledTimes(1) + }) const reason = new Error('cancel one upload waiter') controller.abort(reason) @@ -123,13 +128,17 @@ describe('DeepSeekFileStore', () => { const fetchImpl = vi.fn((_url: string | URL | Request, init?: RequestInit) => { uploadSignal = init?.signal ?? undefined return new Promise((_resolve, reject) => { - uploadSignal?.addEventListener('abort', () => reject(uploadSignal?.reason), { once: true }) + uploadSignal?.addEventListener('abort', () => { + reject(new Error('upload aborted', { cause: uploadSignal?.reason })) + }, { once: true }) }) }) as typeof fetch const store = new DeepSeekFileStore({ index, now: () => NOW, fetch: fetchImpl }) const controller = new AbortController() const upload = store.ensureUploaded(VERSION, CONNECTION, POLICY, controller.signal) - await vi.waitFor(() => expect(fetchImpl).toHaveBeenCalledTimes(1)) + await vi.waitFor(() => { + expect(fetchImpl).toHaveBeenCalledTimes(1) + }) const reason = new Error('cancel only upload waiter') controller.abort(reason) @@ -138,6 +147,79 @@ describe('DeepSeekFileStore', () => { expect(uploadSignal?.reason).toBe(reason) }) + it('normalizes a non-Error cancellation reason', async () => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + const fetchImpl = vi.fn((_url: string | URL | Request, init?: RequestInit) => ( + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + reject(new Error('upload aborted', { cause: init.signal?.reason })) + }, { once: true }) + }) + )) as typeof fetch + const store = new DeepSeekFileStore({ + index: new DeepSeekUploadIndex(join(dir, 'index.json')), + now: () => NOW, + fetch: fetchImpl, + }) + const controller = new AbortController() + const upload = store.ensureUploaded(VERSION, CONNECTION, POLICY, controller.signal) + await vi.waitFor(() => { + expect(fetchImpl).toHaveBeenCalledOnce() + }) + controller.abort('cancelled') + + await expect(upload).rejects.toMatchObject({ + message: 'DeepSeek file upload cancelled with a non-Error reason.', + cause: 'cancelled', + }) + }) + + it('starts a fresh upload while the cancelled transport is settling', async () => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + let requests = 0 + const fetchImpl = vi.fn((_url: string | URL | Request, init?: RequestInit) => { + requests += 1 + if (requests === 1) { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + queueMicrotask(() => { + reject(new Error('upload aborted', { cause: init.signal?.reason })) + }) + }, { once: true }) + }) + } + return Promise.resolve(new Response(JSON.stringify({ + id: 'file-api-retry', object: 'file', bytes: 3, created_at: NOW / 1_000, + filename: 'dsh-retry.png', purpose: 'user_data', + expires_at: NOW / 1_000 + POLICY.expiresAfterSeconds, + }), { status: 200 })) + }) as typeof fetch + const store = new DeepSeekFileStore({ + index: new DeepSeekUploadIndex(join(dir, 'index.json')), + now: () => NOW, + fetch: fetchImpl, + }) + const controller = new AbortController() + const cancelled = store.ensureUploaded(VERSION, CONNECTION, POLICY, controller.signal) + await vi.waitFor(() => { + expect(fetchImpl).toHaveBeenCalledOnce() + }) + controller.abort(new Error('cancel first')) + const retried = store.ensureUploaded(VERSION, CONNECTION, POLICY) + + await expect(cancelled).rejects.toThrow('cancel first') + await expect(retried).resolves.toMatchObject({ record: { fileId: 'file-api-retry' } }) + }) + + it('rejects a request version above the chat per-image limit before transport', async () => { + const fetchImpl = vi.fn() as typeof fetch + const store = new DeepSeekFileStore({ now: () => NOW, fetch: fetchImpl }) + const oversized = { ...VERSION, bytes: MAX_CHAT_IMAGE_BYTES + 1 } + await expect(store.ensureUploaded(oversized, CONNECTION, POLICY)) + .rejects.toMatchObject({ code: 'INVALID_REQUEST' }) + expect(fetchImpl).not.toHaveBeenCalled() + }) + it('does not persist an upload whose response is missing and retries on the next request', async () => { const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) const index = new DeepSeekUploadIndex(join(dir, 'index.json')) @@ -158,6 +240,55 @@ describe('DeepSeekFileStore', () => { .resolves.toMatchObject({ record: { fileId: 'file-api-1' }, uploaded: true }) }) + it('rejects an upload response whose byte count differs from the request version', async () => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + const fetchImpl = vi.fn(() => Promise.resolve(new Response(JSON.stringify({ + id: 'file-api-wrong-size', object: 'file', bytes: 2, created_at: NOW / 1_000, + filename: 'dsh-wrong.png', purpose: 'user_data', + expires_at: NOW / 1_000 + POLICY.expiresAfterSeconds, + }), { status: 200 }))) as typeof fetch + const store = new DeepSeekFileStore({ + index: new DeepSeekUploadIndex(join(dir, 'index.json')), + now: () => NOW, + fetch: fetchImpl, + }) + await expect(store.ensureUploaded(VERSION, CONNECTION, POLICY)) + .rejects.toMatchObject({ code: 'INVALID_RESPONSE' }) + }) + + it.each([ + ['image/jpeg', 'jpeg'], + ['image/webp', 'webp'], + ['image/gif', 'gif'], + ] as const)('uses the %s filename extension for uploads', async (mediaType, extension) => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + const remote = uploadFetch() + const store = new DeepSeekFileStore({ + index: new DeepSeekUploadIndex(join(dir, `${extension}.json`)), + now: () => NOW, + fetch: remote.fetchImpl, + }) + await store.ensureUploaded({ ...VERSION, mediaType }, CONNECTION, POLICY) + const form = vi.mocked(remote.fetchImpl).mock.calls[0]?.[1]?.body + expect(form).toBeInstanceOf(FormData) + const file = (form as FormData).get('file') + expect(file).toBeInstanceOf(File) + if (!(file instanceof File)) throw new Error('expected multipart file') + expect(file.name).toMatch(new RegExp(`\\.${extension}$`, 'u')) + }) + + it('normalizes a non-Error failure from the durable upload index', async () => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + const index = new DeepSeekUploadIndex(join(dir, 'index.json')) + vi.spyOn(index, 'get').mockRejectedValue('index unavailable') + const store = new DeepSeekFileStore({ index, now: () => NOW, fetch: vi.fn() as typeof fetch }) + + await expect(store.ensureUploaded(VERSION, CONNECTION, POLICY)).rejects.toMatchObject({ + message: 'DeepSeek file upload failed with a non-Error reason.', + cause: 'index unavailable', + }) + }) + it('reuses local expires_at above the refresh margin and uploads again at the margin', async () => { const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) const index = new DeepSeekUploadIndex(join(dir, 'index.json')) @@ -190,6 +321,101 @@ describe('DeepSeekFileStore', () => { expect(remote.fetchImpl).toHaveBeenCalledTimes(2) }) + it('removes a losing upload and keeps the winning durable mapping when duplicate cleanup fails', async () => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + const index = new DeepSeekUploadIndex(join(dir, 'index.json')) + vi.spyOn(index, 'commit').mockResolvedValue({ + accepted: false, + record: { + scope: deepSeekFileScope(CONNECTION.baseURL, CONNECTION.apiKey), + masterAttachmentId: VERSION.master.attachmentId, + variantId: VERSION.variantId, + fileId: DeepSeekFileId('file-api-winner'), + bytes: 3, + createdAt: NOW, + expiresAt: NOW + POLICY.expiresAfterSeconds * 1_000, + }, + }) + const remote = uploadFetch() + const fetchImpl = vi.fn((url: string | URL | Request, init?: RequestInit) => { + if (init?.method === 'DELETE') return Promise.resolve(new Response('failed', { status: 500 })) + return remote.fetchImpl(url, init) + }) as typeof fetch + const store = new DeepSeekFileStore({ index, now: () => NOW, fetch: fetchImpl }) + + await expect(store.ensureUploaded(VERSION, CONNECTION, POLICY)).resolves.toMatchObject({ + record: { fileId: 'file-api-winner' }, + uploaded: false, + }) + expect(fetchImpl).toHaveBeenCalledTimes(2) + }) + + it('reclaims one owned file after quota rejection and retries the upload once', async () => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + let uploads = 0 + const fetchImpl = vi.fn((input: string | URL | Request, init?: RequestInit) => { + if (init?.method === 'POST') { + uploads += 1 + if (uploads === 1) return Promise.resolve(new Response(JSON.stringify({ + error: { message: 'stored file quota exceeded', code: 'file_quota' }, + }), { status: 400 })) + return Promise.resolve(new Response(JSON.stringify({ + id: 'file-api-recovered', object: 'file', bytes: 3, created_at: NOW / 1_000, + filename: 'dsh-recovered.png', purpose: 'user_data', + expires_at: NOW / 1_000 + POLICY.expiresAfterSeconds, + }), { status: 200 })) + } + if (init?.method === 'DELETE') { + return Promise.resolve(new Response(JSON.stringify({ + id: 'file-api-old', object: 'file', deleted: true, + }), { status: 200 })) + } + expect(new URL(requestUrl(input)).pathname).toBe('/files') + return Promise.resolve(new Response(JSON.stringify({ + object: 'list', + data: [{ + id: 'file-api-old', object: 'file', bytes: 3, created_at: NOW / 1_000, + filename: 'dsh-old.png', purpose: 'user_data', + }], + first_id: 'file-api-old', last_id: 'file-api-old', has_more: false, + }), { status: 200 })) + }) as typeof fetch + const store = new DeepSeekFileStore({ + index: new DeepSeekUploadIndex(join(dir, 'index.json')), + now: () => NOW, + fetch: fetchImpl, + }) + + await expect(store.ensureUploaded(VERSION, CONNECTION, POLICY)).resolves.toMatchObject({ + record: { fileId: 'file-api-recovered' }, uploaded: true, + }) + expect(uploads).toBe(2) + }) + + it('preserves a quota error when no harness-owned file can be reclaimed', async () => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + const fetchImpl = vi.fn((_input: string | URL | Request, init?: RequestInit) => { + if (init?.method === 'POST') return Promise.resolve(new Response(JSON.stringify({ + error: { message: 'file count quota exceeded', code: 'file_quota' }, + }), { status: 400 })) + return Promise.resolve(new Response(JSON.stringify({ + object: 'list', + data: [{ + id: 'file-api-foreign', object: 'file', bytes: 3, created_at: NOW / 1_000, + filename: 'foreign.png', purpose: 'user_data', + }], + has_more: false, + }), { status: 200 })) + }) as typeof fetch + const store = new DeepSeekFileStore({ + index: new DeepSeekUploadIndex(join(dir, 'index.json')), + now: () => NOW, + fetch: fetchImpl, + }) + + await expect(store.ensureUploaded(VERSION, CONNECTION, POLICY)).rejects.toMatchObject({ code: 'FILES_API' }) + }) + it('finishes pagination before deleting cursor files during quota recovery', async () => { const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) const deleted = new Set() @@ -227,4 +453,44 @@ describe('DeepSeekFileStore', () => { await expect(store.reclaimOldestOwned(CONNECTION, 2)).resolves.toBe(2) expect([...deleted]).toEqual(['file-api-oldest', 'file-api-next']) }) + + it('stops pagination when a page omits or repeats its cursor', async () => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + for (const mode of ['missing', 'repeated'] as const) { + let page = 0 + const fetchImpl = vi.fn((input: string | URL | Request, init?: RequestInit) => { + if (init?.method === 'DELETE') { + const id = requestUrl(input).split('/').at(-1) + return Promise.resolve(new Response(JSON.stringify({ id, object: 'file', deleted: true }), { status: 200 })) + } + page += 1 + const lastId = mode === 'missing' ? undefined : 'file-api-same' + return Promise.resolve(new Response(JSON.stringify({ + object: 'list', data: [], has_more: true, + ...lastId === undefined ? {} : { last_id: lastId }, + }), { status: 200 })) + }) as typeof fetch + const store = new DeepSeekFileStore({ + index: new DeepSeekUploadIndex(join(dir, `${mode}.json`)), + now: () => NOW, + fetch: fetchImpl, + }) + await expect(store.reclaimOldestOwned(CONNECTION, 1)).resolves.toBe(0) + expect(page).toBe(mode === 'missing' ? 1 : 2) + } + }) + + it('releases every batch and clears the scoped upload index', async () => { + const dir = await mkdtemp(join(tmpdir(), 'dsh-file-store-')) + const index = new DeepSeekUploadIndex(join(dir, 'index.json')) + const store = new DeepSeekFileStore({ index, now: () => NOW, fetch: vi.fn() as typeof fetch }) + const reclaim = vi.spyOn(store, 'reclaimOldestOwned') + .mockResolvedValueOnce(1_000) + .mockResolvedValueOnce(2) + const clear = vi.spyOn(index, 'clear') + + await expect(store.releaseAll(CONNECTION)).resolves.toBe(1_002) + expect(reclaim).toHaveBeenCalledTimes(2) + expect(clear).toHaveBeenCalledOnce() + }) }) diff --git a/packages/llm/llm-deepseek/tests/files-api.spec.ts b/packages/llm/llm-deepseek/tests/files-api.spec.ts index 466c9be83b..e6a1aa59ea 100644 --- a/packages/llm/llm-deepseek/tests/files-api.spec.ts +++ b/packages/llm/llm-deepseek/tests/files-api.spec.ts @@ -119,7 +119,7 @@ describe('DeepSeekFilesClient', () => { const client = new DeepSeekFilesClient({ baseURL: 'https://api.deepseek.com', apiKey: 'key', - fetch: vi.fn(() => Promise.resolve(new Response('not-json', { status }))) as typeof fetch, + fetch: vi.fn(() => Promise.resolve(new Response('not-json', { status }))), }) await expect(client.retrieve(DeepSeekFileId('missing'))).rejects.toMatchObject({ name: 'DeepSeekFilesError', @@ -139,7 +139,7 @@ describe('DeepSeekFilesClient', () => { const client = new DeepSeekFilesClient({ baseURL: 'https://api.deepseek.com', apiKey: 'key', - fetch: vi.fn(() => Promise.resolve(new Response(JSON.stringify(body), { status: 400 }))) as typeof fetch, + fetch: vi.fn(() => Promise.resolve(new Response(JSON.stringify(body), { status: 400 }))), }) const error = await client.retrieve(DeepSeekFileId('missing')).catch((caught: unknown) => caught) expect(error).toBeInstanceOf(DeepSeekFilesError) @@ -151,7 +151,7 @@ describe('DeepSeekFilesClient', () => { const client = new DeepSeekFilesClient({ baseURL: 'https://api.deepseek.com', apiKey: 'key', - fetch: vi.fn(() => Promise.reject(transport)) as typeof fetch, + fetch: vi.fn(() => Promise.reject(transport)), }) await expect(client.retrieve(DeepSeekFileId('one'))).rejects.toMatchObject({ code: 'TRANSPORT', @@ -183,7 +183,7 @@ describe('DeepSeekFilesClient', () => { const client = new DeepSeekFilesClient({ baseURL: 'https://api.deepseek.com', apiKey: 'key', - fetch: vi.fn(() => Promise.resolve(new Response(JSON.stringify(body), { status: 200 }))) as typeof fetch, + fetch: vi.fn(() => Promise.resolve(new Response(JSON.stringify(body), { status: 200 }))), }) await expect(client.retrieve(DeepSeekFileId('one'))).rejects.toMatchObject({ code: 'INVALID_RESPONSE' }) }) @@ -224,7 +224,7 @@ describe('DeepSeekFilesClient', () => { const client = new DeepSeekFilesClient({ baseURL: 'https://api.deepseek.com', apiKey: 'key', - fetch: vi.fn(() => Promise.resolve(new Response(JSON.stringify(body), { status: 200 }))) as typeof fetch, + fetch: vi.fn(() => Promise.resolve(new Response(JSON.stringify(body), { status: 200 }))), }) await expect(client.list()).rejects.toMatchObject({ code: 'INVALID_RESPONSE' }) }) @@ -253,7 +253,7 @@ describe('DeepSeekFilesClient', () => { const client = new DeepSeekFilesClient({ baseURL: 'https://api.deepseek.com', apiKey: 'key', - fetch: vi.fn(() => Promise.resolve(new Response(JSON.stringify(body), { status: 200 }))) as typeof fetch, + fetch: vi.fn(() => Promise.resolve(new Response(JSON.stringify(body), { status: 200 }))), }) await expect(client.delete(DeepSeekFileId('file-api-one'))).rejects.toMatchObject({ code: 'INVALID_RESPONSE' }) }) diff --git a/packages/llm/llm-deepseek/tests/serialize.spec.ts b/packages/llm/llm-deepseek/tests/serialize.spec.ts index 8e04b9b3b0..06af757c9a 100644 --- a/packages/llm/llm-deepseek/tests/serialize.spec.ts +++ b/packages/llm/llm-deepseek/tests/serialize.spec.ts @@ -399,6 +399,14 @@ describe('image serialization', () => { }) }) + it('rejects an image whose prepared request version is absent', async () => { + const ref = imageRef() + await expect(serializeMessagesWithImages([createUserMessage({ + content: [{ type: 'image', attachment: ref }], + source: { kind: 'plugin', plugin: 'test' }, + })], imageOptions([]))).rejects.toMatchObject({ code: 'INVALID_REQUEST' }) + }) + it('keeps tool content textual and groups consecutive tool-result images afterward', async () => { const messages = [ createUserMessage({ @@ -561,6 +569,17 @@ describe('image serialization', () => { expect(resolveFileId.mock.calls[0]?.[0]).toMatchObject({ master: { mediaType: 'image/jpeg' } }) }) + it('rejects an unprepared image while computing exact request bytes', async () => { + const ref = imageRef() + await expect(serializeRequestWithImages(request({ + model: 'deepseek-v4-flash-vision-exp', + messages: [createUserMessage({ + content: [{ type: 'image', attachment: ref }], + source: { kind: 'plugin', plugin: 'test' }, + })], + }), imageOptions([]))).rejects.toMatchObject({ code: 'INVALID_REQUEST' }) + }) + it.each(['system', 'assistant'] as const)('rejects an image in %s history before reading attachments', async (role) => { const resolveFileId = vi.fn() await expect(serializeMessagesWithImages([createMessage({ diff --git a/packages/llm/llm-pi-ai/tests/config.spec.ts b/packages/llm/llm-pi-ai/tests/config.spec.ts index 55444228de..61a2535a9d 100644 --- a/packages/llm/llm-pi-ai/tests/config.spec.ts +++ b/packages/llm/llm-pi-ai/tests/config.spec.ts @@ -64,3 +64,26 @@ describe('modality schema boundary', () => { expect(absent.providers['acme-gateway']?.defaultInput).toEqual(['text']) }) }) + +describe('request image policy bounds', () => { + it.each([ + ['requestImagePixelBudget', 0, /requestImagePixelBudget must be a positive safe integer/], + ['requestImagePixelBudget', Number.MAX_SAFE_INTEGER + 1, /requestImagePixelBudget must be a positive safe integer/], + ['requestImageMaxBytes', 0, /requestImageMaxBytes must be a positive safe integer/], + ['requestImageMaxBytes', 1.5, /requestImageMaxBytes must be a positive safe integer/], + ] as const)('rejects %s=%s at service resolution', (field, value, message) => { + const programmatic = { + providers: { + 'acme-gateway': { + api: 'openai-completions', + baseURL: 'https://acme.test', + models: [{ id: 'm' }], + [field]: value, + }, + }, + } as unknown as Config + expect(() => { + assertServiceable(programmatic) + }).toThrow(message) + }) +}) diff --git a/packages/llm/llm-pi-ai/tests/context.spec.ts b/packages/llm/llm-pi-ai/tests/context.spec.ts index a0fb671c95..8a3f7bd084 100644 --- a/packages/llm/llm-pi-ai/tests/context.spec.ts +++ b/packages/llm/llm-pi-ai/tests/context.spec.ts @@ -428,4 +428,14 @@ describe('pi-ai request context conversion', () => { history('assistant', [{ type: 'image', attachment: ref }]), )).toThrow(/assistant image output/) }) + + it('rejects an attachment service that omits a requested image version', async () => { + const store = { + readImageRequests: vi.fn(() => Promise.resolve([])), + } as unknown as AttachmentStore + await expect(toPiContext( + request([user([{ type: 'image', attachment: ref }])]), + store, + )).rejects.toMatchObject({ code: 'INVALID_REQUEST' }) + }) }) diff --git a/packages/llm/llm/src/content.ts b/packages/llm/llm/src/content.ts index 73a2aee889..72e452e005 100644 --- a/packages/llm/llm/src/content.ts +++ b/packages/llm/llm/src/content.ts @@ -74,7 +74,9 @@ function collectImageLengths( ): void { for (const block of blocks) { if (block.type === 'image') { - const bytes = policy.byteLength?.(block.attachment) ?? block.attachment.bytes + const bytes = policy.byteLength === undefined + ? block.attachment.bytes + : policy.byteLength(block.attachment) lengths.push(policy.representation === 'base64' ? base64Length(bytes) : bytes) } else if (block.type === 'tool-result') { collectImageLengths(block.content, lengths, policy) diff --git a/packages/llm/llm/tests/content.spec.ts b/packages/llm/llm/tests/content.spec.ts index d1b02fa011..6a0eb02c63 100644 --- a/packages/llm/llm/tests/content.spec.ts +++ b/packages/llm/llm/tests/content.spec.ts @@ -1,6 +1,13 @@ import { describe, expect, it } from 'vitest' import { AttachmentId } from '@deepseek-ai/dsh-attachment' -import { CallId, createUserMessage, OFFLOADED_IMAGE_TEXT, offloadRequestImages, offloadRequestImagesWithPolicy } from '../src/index.ts' +import { + CallId, + createUserMessage, + OFFLOADED_IMAGE_TEXT, + offloadRequestImages, + offloadRequestImagesWithPolicy, + projectImagesForTextModel, +} from '../src/index.ts' import type { ContentBlock } from '../src/index.ts' const source = { kind: 'plugin' as const, plugin: 'test' } @@ -19,6 +26,11 @@ function image(bytes: number): ContentBlock { } describe('offloadRequestImages', () => { + it('preserves every image when no payload bound is configured', () => { + const messages = [createUserMessage({ content: [image(300)], source })] + expect(offloadRequestImages(messages, undefined)).toBe(messages) + }) + it('preserves the original request when its base64 payload fits exactly', () => { const messages = [createUserMessage({ content: [image(3), image(3)], source })] expect(offloadRequestImages(messages, 8)).toBe(messages) @@ -116,4 +128,58 @@ describe('offloadRequestImagesWithPolicy', () => { expect(projected[0]?.content.filter(block => block.type === 'text')).toHaveLength(20) expect(projected[0]?.content.filter(block => block.type === 'image')).toHaveLength(581) }) + + it('uses route-owned request byte lengths when supplied', () => { + const messages = [createUserMessage({ content: [image(100), image(100)], source })] + const projected = offloadRequestImagesWithPolicy(messages, { + representation: 'raw', + maxBytes: 3, + byteLength: () => 2, + }) + expect(projected[0]?.content).toEqual([ + { type: 'text', text: OFFLOADED_IMAGE_TEXT }, + image(100), + ]) + }) +}) + +describe('projectImagesForTextModel', () => { + it('returns image-free history unchanged', () => { + const messages = [createUserMessage({ content: [{ type: 'text', text: 'plain' }], source })] + expect(projectImagesForTextModel(messages)).toBe(messages) + }) + + it('replaces direct and nested images while retaining unaffected messages and blocks', () => { + const plain = createUserMessage({ content: [{ type: 'text', text: 'plain' }], source }) + const nested = { + type: 'tool-result' as const, + toolCallId: CallId('nested-image'), + content: [{ type: 'text' as const, text: 'before' }, image(3), { type: 'text' as const, text: 'after' }], + } + const unchangedNested = { + type: 'tool-result' as const, + toolCallId: CallId('text-only'), + content: [{ type: 'text' as const, text: 'unchanged' }], + } + const visual = createUserMessage({ + content: [{ type: 'text', text: 'lead' }, image(3), unchangedNested, nested], + source, + }) + + const projected = projectImagesForTextModel([plain, visual]) + expect(projected[0]).toBe(plain) + expect(projected[1]?.content).toEqual([ + { type: 'text', text: 'lead' }, + { type: 'text', text: '[image omitted because this model accepts text only; attachment sha256:aaaaaaaa]' }, + unchangedNested, + { + ...nested, + content: [ + { type: 'text', text: 'before' }, + { type: 'text', text: '[image omitted because this model accepts text only; attachment sha256:aaaaaaaa]' }, + { type: 'text', text: 'after' }, + ], + }, + ]) + }) }) diff --git a/packages/llm/llm/tests/service.spec.ts b/packages/llm/llm/tests/service.spec.ts index b2e5399964..4c624fa862 100644 --- a/packages/llm/llm/tests/service.spec.ts +++ b/packages/llm/llm/tests/service.spec.ts @@ -986,6 +986,18 @@ describe('LlmRuntime', () => { type: 'text', text: '[image omitted because this model accepts text only; attachment sha256:aaaaaaaa]', }]) + + const frozen = Object.freeze({ + provider: 'route', + model: 'text-only', + messages: [createUserMessage({ + content: [{ type: 'image', attachment }], + source: { kind: 'plugin' as const, plugin: 'test' }, + })], + }) + await collect(ctx.llm.stream(frozen)) + expect(Object.isFrozen(seen[1])).toBe(true) + expect(Object.isFrozen(seen[1]?.messages)).toBe(true) }) it('passes cancellation through exact-model resolution', async () => {