fix(attachment): distinguish admission from storage failures
This commit is contained in:
@@ -2,7 +2,7 @@
|
|||||||
|
|
||||||
import type { ContentBlock as AcpContentBlock } from '@agentclientprotocol/sdk'
|
import type { ContentBlock as AcpContentBlock } from '@agentclientprotocol/sdk'
|
||||||
import type { Context } from '@deepseek-ai/cordis'
|
import type { Context } from '@deepseek-ai/cordis'
|
||||||
import { AttachmentError } from '@deepseek-ai/dsh-attachment'
|
import { isImageAdmissionError } from '@deepseek-ai/dsh-attachment'
|
||||||
import type { ImageAttachmentRef, ImageMediaType, SaveImageAttachment } from '@deepseek-ai/dsh-attachment'
|
import type { ImageAttachmentRef, ImageMediaType, SaveImageAttachment } from '@deepseek-ai/dsh-attachment'
|
||||||
import type { Agent } from '@deepseek-ai/dsh-agent'
|
import type { Agent } from '@deepseek-ai/dsh-agent'
|
||||||
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||||
@@ -72,7 +72,7 @@ async function assertImageRoute(ctx: Context, agent: Agent, signal: AbortSignal)
|
|||||||
try {
|
try {
|
||||||
info = await llm.resolveModelInfo(provider, model, signal)
|
info = await llm.resolveModelInfo(provider, model, signal)
|
||||||
} catch (error: unknown) {
|
} catch (error: unknown) {
|
||||||
throw new AcpContentError('the current model route could not be verified for image input', 'invalid', { cause: error })
|
throw new AcpContentError('the current model route could not be verified for image input', 'internal', { cause: error })
|
||||||
}
|
}
|
||||||
if (info.inputModalities === undefined || !info.inputModalities.includes('image')) {
|
if (info.inputModalities === undefined || !info.inputModalities.includes('image')) {
|
||||||
throw new AcpContentError(`model "${model}" does not declare image input`, 'invalid')
|
throw new AcpContentError(`model "${model}" does not declare image input`, 'invalid')
|
||||||
@@ -157,7 +157,7 @@ export async function admitAcpPrompt(
|
|||||||
try {
|
try {
|
||||||
refs = await attachments.saveImages(images)
|
refs = await attachments.saveImages(images)
|
||||||
} catch (error: unknown) {
|
} catch (error: unknown) {
|
||||||
if (error instanceof AttachmentError && error.code !== 'ATTACHMENT_WRITE_FAILED') {
|
if (isImageAdmissionError(error)) {
|
||||||
throw new AcpContentError(error.message, 'invalid', { cause: error })
|
throw new AcpContentError(error.message, 'invalid', { cause: error })
|
||||||
}
|
}
|
||||||
throw new AcpContentError('unable to persist the prompt image batch', 'internal', { cause: error })
|
throw new AcpContentError('unable to persist the prompt image batch', 'internal', { cause: error })
|
||||||
|
|||||||
@@ -133,8 +133,9 @@ describe('ACP rich content codec', () => {
|
|||||||
|
|
||||||
const broken = admissionFixture()
|
const broken = admissionFixture()
|
||||||
broken.resolveModelInfo.mockRejectedValueOnce(new Error('catalog down'))
|
broken.resolveModelInfo.mockRejectedValueOnce(new Error('catalog down'))
|
||||||
await expect(admitAcpPrompt(broken.ctx, broken.agent, prompt, true, new AbortController().signal))
|
const routeFailure = admitAcpPrompt(broken.ctx, broken.agent, prompt, true, new AbortController().signal)
|
||||||
.rejects.toThrow(/route could not be verified/)
|
await expect(routeFailure).rejects.toMatchObject({ kind: 'internal' })
|
||||||
|
await expect(routeFailure).rejects.toThrow(/route could not be verified/)
|
||||||
const unknown = admissionFixture()
|
const unknown = admissionFixture()
|
||||||
unknown.resolveModelInfo.mockResolvedValueOnce({ provider: 'mock', id: 'vision', name: 'vision' })
|
unknown.resolveModelInfo.mockResolvedValueOnce({ provider: 'mock', id: 'vision', name: 'vision' })
|
||||||
await expect(admitAcpPrompt(unknown.ctx, unknown.agent, prompt, true, new AbortController().signal))
|
await expect(admitAcpPrompt(unknown.ctx, unknown.agent, prompt, true, new AbortController().signal))
|
||||||
@@ -158,6 +159,9 @@ describe('ACP rich content codec', () => {
|
|||||||
await expect(admitAcpPrompt(fixture.ctx, fixture.agent, prompt, true, new AbortController().signal))
|
await expect(admitAcpPrompt(fixture.ctx, fixture.agent, prompt, true, new AbortController().signal))
|
||||||
.rejects.toMatchObject({ kind: 'invalid', message: 'too many' })
|
.rejects.toMatchObject({ kind: 'invalid', message: 'too many' })
|
||||||
fixture.saveImages.mockRejectedValueOnce(new AttachmentError('disk failed', 'ATTACHMENT_WRITE_FAILED'))
|
fixture.saveImages.mockRejectedValueOnce(new AttachmentError('disk failed', 'ATTACHMENT_WRITE_FAILED'))
|
||||||
|
await expect(admitAcpPrompt(fixture.ctx, fixture.agent, prompt, true, new AbortController().signal))
|
||||||
|
.rejects.toMatchObject({ kind: 'internal', message: 'unable to persist the prompt image batch' })
|
||||||
|
fixture.saveImages.mockRejectedValueOnce(new AttachmentError('corrupt object', 'ATTACHMENT_CORRUPT'))
|
||||||
await expect(admitAcpPrompt(fixture.ctx, fixture.agent, prompt, true, new AbortController().signal))
|
await expect(admitAcpPrompt(fixture.ctx, fixture.agent, prompt, true, new AbortController().signal))
|
||||||
.rejects.toMatchObject({ kind: 'internal', message: 'unable to persist the prompt image batch' })
|
.rejects.toMatchObject({ kind: 'internal', message: 'unable to persist the prompt image batch' })
|
||||||
fixture.saveImages.mockRejectedValueOnce(new Error('unknown store failure'))
|
fixture.saveImages.mockRejectedValueOnce(new Error('unknown store failure'))
|
||||||
|
|||||||
@@ -2,5 +2,5 @@
|
|||||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||||
# after editing either side, bring the other along and re-record with:
|
# after editing either side, bring the other along and re-record with:
|
||||||
# pnpm run verify-translation-pairing --write packages/attachment/attachment/README.md
|
# pnpm run verify-translation-pairing --write packages/attachment/attachment/README.md
|
||||||
README.md: c0a86d324da8c27ec386103f40ac50534c2483d7
|
README.md: 05c4bce5498f3c0bf172264be3e4b834ea0925e2
|
||||||
README.zh.md: 562c8af0df20634ac2072c4a8d422b4e0b4b47dd
|
README.zh.md: 91a454da09d32b0a87d02ca7ccb482e95c485a37
|
||||||
|
|||||||
@@ -4,7 +4,7 @@ English | [中文](README.zh.md)
|
|||||||
|
|
||||||
The durable attachment seam. `ctx.attachments` validates and durably commits immutable image bytes, then returns a serializable `ImageAttachmentRef`; consumers never persist browser paths, object URLs, provider URLs, or base64 in session events.
|
The durable attachment seam. `ctx.attachments` validates and durably commits immutable image bytes, then returns a serializable `ImageAttachmentRef`; consumers never persist browser paths, object URLs, provider URLs, or base64 in session events.
|
||||||
|
|
||||||
Unsent composer images remain browser-owned temporary drafts. `validateImage` runs the same admission policy without persisting. `saveImages` owns batch count and aggregate-byte limits, validates every member before writing any member, then commits in order and returns references only after the complete batch succeeds. A later storage failure returns no partial references, although an earlier immutable content-addressed object may remain unreachable until reference-aware garbage collection exists. `saveImage` commits one accepted image before any model-visible session event is published, and `readImage` verifies the content-addressed object against its logged metadata. Callers may cancel `readImage`; implementations observe cancellation around backend and verification work and preserve it instead of translating it into a storage failure.
|
Unsent composer images remain browser-owned temporary drafts. `validateImage` runs the same admission policy without persisting. `saveImages` owns batch count and aggregate-byte limits, validates every member before writing any member, then commits in order and returns references only after the complete batch succeeds. A later storage failure returns no partial references, although an earlier immutable content-addressed object may remain unreachable until reference-aware garbage collection exists. `isImageAdmissionError` distinguishes caller-correctable image-policy failures from storage faults so each protocol adapter can map its own error vocabulary. `saveImage` commits one accepted image before any model-visible session event is published, and `readImage` verifies the content-addressed object against its logged metadata. Callers may cancel `readImage`; implementations observe cancellation around backend and verification work and preserve it instead of translating it into a storage failure.
|
||||||
|
|
||||||
## Model Experience
|
## Model Experience
|
||||||
|
|
||||||
|
|||||||
@@ -4,7 +4,7 @@
|
|||||||
|
|
||||||
持久附件服务边界。`ctx.attachments` 校验并持久提交不可变图片字节,随后返回可序列化的 `ImageAttachmentRef`;消费方绝不会在会话事件中持久保存浏览器路径、对象 URL、提供方 URL 或 base64。
|
持久附件服务边界。`ctx.attachments` 校验并持久提交不可变图片字节,随后返回可序列化的 `ImageAttachmentRef`;消费方绝不会在会话事件中持久保存浏览器路径、对象 URL、提供方 URL 或 base64。
|
||||||
|
|
||||||
未发送的输入区图片仍是由浏览器持有的临时草稿。`validateImage` 运行相同的准入策略,但不执行持久化。`saveImages` 负责批次图片数量和总字节限制,先校验全部成员,再按顺序提交,并且只在完整批次成功后返回引用。后续存储失败不会返回部分引用,但较早写入的不可变内容寻址对象可能保持不可达,直至具备按引用感知的垃圾回收。`saveImage` 会在发布任何模型可见的会话事件前提交一张已接受的图片,`readImage` 则根据已记录的元数据校验内容寻址对象。调用方可以取消 `readImage`;实现会在后端读取与校验工作的边界观察取消,并保留取消语义,而不会将其转换为存储失败。
|
未发送的输入区图片仍是由浏览器持有的临时草稿。`validateImage` 运行相同的准入策略,但不执行持久化。`saveImages` 负责批次图片数量和总字节限制,先校验全部成员,再按顺序提交,并且只在完整批次成功后返回引用。后续存储失败不会返回部分引用,但较早写入的不可变内容寻址对象可能保持不可达,直至具备按引用感知的垃圾回收。`isImageAdmissionError` 区分可由调用方修正的图片策略失败与存储故障,使每个协议适配器可以映射自己的错误词汇。`saveImage` 会在发布任何模型可见的会话事件前提交一张已接受的图片,`readImage` 则根据已记录的元数据校验内容寻址对象。调用方可以取消 `readImage`;实现会在后端读取与校验工作的边界观察取消,并保留取消语义,而不会将其转换为存储失败。
|
||||||
|
|
||||||
## 模型体验
|
## 模型体验
|
||||||
|
|
||||||
|
|||||||
@@ -24,3 +24,26 @@ export class AttachmentError extends Error {
|
|||||||
this.code = code
|
this.code = code
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Attachment failures caused by the caller's proposed image batch. */
|
||||||
|
const IMAGE_ADMISSION_ERROR_CODES = new Set([
|
||||||
|
'TOO_MANY_IMAGES',
|
||||||
|
'IMAGES_TOO_LARGE',
|
||||||
|
'UNSUPPORTED_IMAGE_TYPE',
|
||||||
|
'INVALID_IMAGE',
|
||||||
|
'IMAGE_TYPE_MISMATCH',
|
||||||
|
'IMAGE_TOO_LARGE',
|
||||||
|
'IMAGE_TOO_MANY_PIXELS',
|
||||||
|
])
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Distinguish caller-correctable image admission failures from storage faults.
|
||||||
|
* @param error - failure raised while validating or persisting an image batch.
|
||||||
|
* @returns whether the caller can correct the proposed image content or batch.
|
||||||
|
*/
|
||||||
|
export function isImageAdmissionError(error: unknown): error is AttachmentError {
|
||||||
|
return error instanceof Error
|
||||||
|
&& 'code' in error
|
||||||
|
&& typeof error.code === 'string'
|
||||||
|
&& IMAGE_ADMISSION_ERROR_CODES.has(error.code)
|
||||||
|
}
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ import type {
|
|||||||
} from './types.ts'
|
} from './types.ts'
|
||||||
|
|
||||||
export { AttachmentId } from './brand.ts'
|
export { AttachmentId } from './brand.ts'
|
||||||
export { AttachmentError } from './error.ts'
|
export { AttachmentError, isImageAdmissionError } from './error.ts'
|
||||||
export type {
|
export type {
|
||||||
AttachmentId as AttachmentIdType,
|
AttachmentId as AttachmentIdType,
|
||||||
ImageAttachmentLimits,
|
ImageAttachmentLimits,
|
||||||
|
|||||||
@@ -1,7 +1,9 @@
|
|||||||
import { Context } from '@deepseek-ai/cordis'
|
import { Context } from '@deepseek-ai/cordis'
|
||||||
import { describe, expect, it } from 'vitest'
|
import { describe, expect, it } from 'vitest'
|
||||||
import AttachmentStore, {
|
import AttachmentStore, {
|
||||||
|
AttachmentError,
|
||||||
AttachmentId,
|
AttachmentId,
|
||||||
|
isImageAdmissionError,
|
||||||
type ImageAttachmentRef,
|
type ImageAttachmentRef,
|
||||||
type ImageMediaType,
|
type ImageMediaType,
|
||||||
type SaveImageAttachment,
|
type SaveImageAttachment,
|
||||||
@@ -93,3 +95,14 @@ describe('AttachmentStore.saveImages', () => {
|
|||||||
expect(store.calls).toEqual(['validate:1', 'validate:2', 'save:1', 'save:2'])
|
expect(store.calls).toEqual(['validate:1', 'validate:2', 'save:1', 'save:2'])
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe('isImageAdmissionError', () => {
|
||||||
|
it('separates caller-correctable image policy failures from storage faults', () => {
|
||||||
|
expect(isImageAdmissionError(new AttachmentError('bad bytes', 'INVALID_IMAGE'))).toBe(true)
|
||||||
|
expect(isImageAdmissionError(new AttachmentError('too many', 'TOO_MANY_IMAGES'))).toBe(true)
|
||||||
|
expect(isImageAdmissionError(Object.assign(new Error('foreign policy error'), { code: 'IMAGE_TOO_LARGE' }))).toBe(true)
|
||||||
|
expect(isImageAdmissionError(new AttachmentError('corrupt object', 'ATTACHMENT_CORRUPT'))).toBe(false)
|
||||||
|
expect(isImageAdmissionError(new AttachmentError('disk failed', 'ATTACHMENT_WRITE_FAILED'))).toBe(false)
|
||||||
|
expect(isImageAdmissionError(new Error('unknown failure'))).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import type { Client } from '@modelcontextprotocol/sdk/client/index.js'
|
|||||||
import { ListToolsResultSchema } from '@modelcontextprotocol/sdk/types.js'
|
import { ListToolsResultSchema } from '@modelcontextprotocol/sdk/types.js'
|
||||||
import { z } from 'zod'
|
import { z } from 'zod'
|
||||||
import type { Context } from '@deepseek-ai/cordis'
|
import type { Context } from '@deepseek-ai/cordis'
|
||||||
|
import { isImageAdmissionError } from '@deepseek-ai/dsh-attachment'
|
||||||
import type { AttachmentStore, ImageAttachmentRef, ImageMediaType, SaveImageAttachment } from '@deepseek-ai/dsh-attachment'
|
import type { AttachmentStore, ImageAttachmentRef, ImageMediaType, SaveImageAttachment } from '@deepseek-ai/dsh-attachment'
|
||||||
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||||
import type { ToolDefinition, ToolExecution, ToolExecutionResult } from '@deepseek-ai/dsh-tools'
|
import type { ToolDefinition, ToolExecution, ToolExecutionResult } from '@deepseek-ai/dsh-tools'
|
||||||
@@ -474,10 +475,13 @@ async function prepareImageProjection(
|
|||||||
type: 'image',
|
type: 'image',
|
||||||
attachment: byIndex.get(index) as ImageAttachmentRef,
|
attachment: byIndex.get(index) as ImageAttachmentRef,
|
||||||
}))
|
}))
|
||||||
} catch {
|
} catch (error: unknown) {
|
||||||
|
const reason = isImageAdmissionError(error)
|
||||||
|
? `image admission rejected the result: ${error.message}`
|
||||||
|
: 'durable image storage rejected the result'
|
||||||
return projectContent(content, toolName, block => ({
|
return projectContent(content, toolName, block => ({
|
||||||
type: 'text',
|
type: 'text',
|
||||||
text: imageDiagnostic(block, 'durable image storage rejected the result'),
|
text: imageDiagnostic(block, reason),
|
||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,7 +2,7 @@ import { describe, expect, it, vi, beforeEach } from 'vitest'
|
|||||||
import { Client } from '@modelcontextprotocol/sdk/client/index.js'
|
import { Client } from '@modelcontextprotocol/sdk/client/index.js'
|
||||||
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'
|
import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js'
|
||||||
import { Context } from '@deepseek-ai/cordis'
|
import { Context } from '@deepseek-ai/cordis'
|
||||||
import AttachmentStore, { AttachmentId } from '@deepseek-ai/dsh-attachment'
|
import AttachmentStore, { AttachmentError, AttachmentId } from '@deepseek-ai/dsh-attachment'
|
||||||
import type { ImageAttachmentLimits, ImageAttachmentRef, SaveImageAttachment, StoredImageAttachment } from '@deepseek-ai/dsh-attachment'
|
import type { ImageAttachmentLimits, ImageAttachmentRef, SaveImageAttachment, StoredImageAttachment } from '@deepseek-ai/dsh-attachment'
|
||||||
import { CallId, LlmAdapter, LlmService } from '@deepseek-ai/dsh-llm'
|
import { CallId, LlmAdapter, LlmService } from '@deepseek-ai/dsh-llm'
|
||||||
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
import type { ContentBlock } from '@deepseek-ai/dsh-llm'
|
||||||
@@ -676,6 +676,29 @@ describe('tool execution', () => {
|
|||||||
expect(textAt(result.content)).toContain('durable image storage rejected the result')
|
expect(textAt(result.content)).toContain('durable image storage rejected the result')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('reports attachment policy rejection as image admission rather than storage failure', async () => {
|
||||||
|
const rich = await mountRichRegistry()
|
||||||
|
vi.spyOn(rich.attachments, 'saveImages').mockRejectedValueOnce(
|
||||||
|
new AttachmentError('too many images', 'TOO_MANY_IMAGES'),
|
||||||
|
)
|
||||||
|
const client = createMockClient(
|
||||||
|
[{ name: 'img', inputSchema: { type: 'object' } }],
|
||||||
|
{ content: [{ type: 'image', mimeType: 'image/png', data: 'AQ==' }] },
|
||||||
|
)
|
||||||
|
|
||||||
|
await syncTools(client as never, rich.ctx, defaultOpts, new Map())
|
||||||
|
const result = await rich.ctx.tools.execute({
|
||||||
|
signal: testToolSignal,
|
||||||
|
callId: CallId('policy-rejected'),
|
||||||
|
name: 'mcp__srv__img',
|
||||||
|
arguments: {},
|
||||||
|
agent: agentOn() as never,
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(textAt(result.content)).toContain('image admission rejected the result: too many images')
|
||||||
|
expect(textAt(result.content)).not.toContain('storage rejected')
|
||||||
|
})
|
||||||
|
|
||||||
it('lets post-execute replacement win over a prepared image projection', async () => {
|
it('lets post-execute replacement win over a prepared image projection', async () => {
|
||||||
const rich = await mountRichRegistry()
|
const rich = await mountRichRegistry()
|
||||||
rich.ctx.on('tools/post-execute', async (): Promise<PostToolDecision> => ({
|
rich.ctx.on('tools/post-execute', async (): Promise<PostToolDecision> => ({
|
||||||
|
|||||||
Reference in New Issue
Block a user