fix: address ds-review-bot v6/v7 findings on the image-input assembly
- resolveLlmRoute: reuse the yml pi-ai row for providers it already routes (DUPLICATE_ADAPTER boot failure) and detect an unset model by origin, not by comparison against one deployment default; covered by a new spec. - LlmService.resolveModelInfoFor preserves (and validates) modality metadata, arming the host image preflight for exact-route resolution. - session.selectModel refuses a text-only target once the session log carries an image on any replayed route; an accepted switch would strand every later turn with no in-product recovery. - The composer no longer gates image intake on the handshake activeModel snapshot (wrong authority for a per-session decision); the host preflight plus the error strip own capability, deployment limits stay client-side. - InputHub shell teardown releases the scope's draft images (File objects and object URLs leaked for the page lifetime). - session.prompt image parts carry optional alt into the durable block; ImageBlock documents assistant-side rendering as forward compatibility. - Assembled built-client lane apps/web/tests/image-display.snapshot.ts pins the history galleries over the authorized attachment route, the lightbox, and the composer paste rail; the attachment rail is an accessible group. - Docs: validateImage on the seam page, fixture byte metadata matches its PNG, and the Agent Note claims now match the shipped coverage.
This commit is contained in:
@@ -108,7 +108,9 @@ const FIXTURE_IMAGE_DATA = 'iVBORw0KGgoAAAANSUhEUgAAAKAAAABaCAYAAAA/xl1SAAAAvklE
|
||||
const FIXTURE_IMAGE_REF: ImageAttachmentRef = {
|
||||
attachmentId: 'fixture:image' as AttachmentIdType,
|
||||
mediaType: 'image/png',
|
||||
bytes: 68,
|
||||
// Matches the decoded FIXTURE_IMAGE_DATA exactly (the real backend serves
|
||||
// verified metadata; a mismatched fixture would mislead comparisons).
|
||||
bytes: 247,
|
||||
width: 160,
|
||||
height: 90,
|
||||
name: 'fixture-image.png',
|
||||
|
||||
@@ -28,6 +28,7 @@ interface ConversationAttachmentFace {
|
||||
mode: 'queue' | 'steer',
|
||||
imageIds: readonly string[],
|
||||
): Promise<void>
|
||||
releaseDraftImage(id: string): void
|
||||
}
|
||||
|
||||
/** Session-addressed input facade registry (InputService face + composer-layer extras). */
|
||||
@@ -84,8 +85,13 @@ export class InputHub implements InputService {
|
||||
]
|
||||
return () => {
|
||||
for (const off of offs) off()
|
||||
// Draft attachments die with the scope: the shell only holds ids, so
|
||||
// the service-owned File objects and object URLs must be released
|
||||
// here or they leak for the page lifetime.
|
||||
const drafts = shell.snapshot.imageIds
|
||||
shell.dispose()
|
||||
this.shells.delete(id)
|
||||
for (const imageId of drafts) this.conversation().releaseDraftImage(imageId)
|
||||
}
|
||||
}, 'conversation.input: session shell')
|
||||
return shell
|
||||
|
||||
@@ -330,11 +330,13 @@ export class ConversationService extends Service implements IConversation {
|
||||
current: readonly ComposerAttachment[],
|
||||
): void {
|
||||
if (files.length === 0 && current.length === 0) return
|
||||
// Deployment-wide limits only. Model capability is deliberately NOT
|
||||
// checked here: the handshake's activeModel is the host default, not the
|
||||
// session's current target (session.selectModel never refreshes it), so a
|
||||
// client-side modality gate refuses sessions the host would accept and
|
||||
// vice versa. The host preflight on session.prompt is the authority; its
|
||||
// rejection renders through the composer error strip.
|
||||
const description = this.requireSessions().hostDescription()
|
||||
const modalities = description?.activeModel?.inputModalities
|
||||
if (modalities !== undefined && !modalities.includes('image')) {
|
||||
throw new Error('当前模型不支持图片输入')
|
||||
}
|
||||
const limits = description?.imageLimits
|
||||
const all = [...current.map(attachment => attachment.file), ...files]
|
||||
if (limits !== undefined && all.length > limits.maxImagesPerMessage) {
|
||||
|
||||
@@ -385,7 +385,7 @@ export function InputBar({
|
||||
{overlay !== undefined && <div className={css.overlayAnchor}>{overlay}</div>}
|
||||
{accessory !== undefined && <div className={css.accessory}>{accessory}</div>}
|
||||
{attachments.length > 0 && (
|
||||
<div className={css.attachments} aria-label="待发送图片">
|
||||
<div className={css.attachments} role="group" aria-label="待发送图片">
|
||||
{attachments.map(attachment => (
|
||||
<div key={attachment.id} className={css.attachment}>
|
||||
<button
|
||||
|
||||
@@ -20,13 +20,12 @@ async function bench() {
|
||||
})
|
||||
// config.input is required (the apply shares its hub with the inject
|
||||
// factories); the bench passes its own instance explicitly.
|
||||
const fiber = runtime.ctx.plugin(ConversationService, {
|
||||
input: new InputHub(runtime.ctx),
|
||||
})
|
||||
const hub = new InputHub(runtime.ctx)
|
||||
const fiber = runtime.ctx.plugin(ConversationService, { input: hub })
|
||||
await fiber.await()
|
||||
const root = runtime.ctx.get('conversation') as ConversationService
|
||||
const scoped = runtime.sessions.scope('s1')!.get('conversation') as ConversationService
|
||||
return { runtime, root, scoped, prompt, cancel, loadOlder }
|
||||
return { runtime, hub, root, scoped, prompt, cancel, loadOlder }
|
||||
}
|
||||
|
||||
describe('ConversationService', () => {
|
||||
@@ -50,6 +49,26 @@ describe('ConversationService', () => {
|
||||
await b.runtime.dispose()
|
||||
})
|
||||
|
||||
it('releases draft images when the session scope is disposed', async () => {
|
||||
const b = await bench()
|
||||
const created = vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:draft-1')
|
||||
const revoked = vi.spyOn(URL, 'revokeObjectURL').mockReturnValue(undefined)
|
||||
try {
|
||||
const [attachment] = b.root.createDraftImages([new File([new Uint8Array(4)], 'a.png', { type: 'image/png' })])
|
||||
if (attachment === undefined) throw new Error('draft attachment missing')
|
||||
// Land the id in the session shell exactly as the composer does.
|
||||
b.hub.shell(b.runtime.sessions.behavior('s1').sessionId).addImages([attachment.id])
|
||||
await b.runtime.sessions.remove('s1')
|
||||
// Scope teardown released the service-held File and its object URL.
|
||||
expect(b.root.draftImages([attachment.id])).toEqual([])
|
||||
expect(revoked).toHaveBeenCalledWith('blob:draft-1')
|
||||
} finally {
|
||||
created.mockRestore()
|
||||
revoked.mockRestore()
|
||||
}
|
||||
await b.runtime.dispose()
|
||||
})
|
||||
|
||||
it('fails loudly from the root scope, on an unbound session, or without SessionsService', async () => {
|
||||
const b = await bench()
|
||||
await expect(b.root.send('x', 'queue')).rejects.toThrow(/requires a session scope/)
|
||||
|
||||
@@ -106,7 +106,7 @@ async function durablePromptContent(ctx: Context, content: readonly PromptConten
|
||||
mediaType: item.part.mediaType,
|
||||
...item.part.name === undefined ? {} : { name: item.part.name },
|
||||
})
|
||||
return { type: 'image', attachment }
|
||||
return { type: 'image', attachment, ...item.part.alt === undefined ? {} : { alt: item.part.alt } }
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -127,6 +127,32 @@ function imageInContent(content: unknown, attachmentId: string): ImageAttachment
|
||||
return undefined
|
||||
}
|
||||
|
||||
/** True when any block (nested tool-result content included) is an image block. */
|
||||
function contentHasImage(content: unknown): boolean {
|
||||
if (!Array.isArray(content)) return false
|
||||
for (const value of content) {
|
||||
if (typeof value !== 'object' || value === null || Array.isArray(value)) continue
|
||||
const block = value as { type?: unknown; content?: unknown }
|
||||
if (block.type === 'image') return true
|
||||
if (block.type === 'tool-result' && contentHasImage(block.content)) return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* True when the session log already carries image content on any route a
|
||||
* model request replays (message content, wrapped messages, streamed blocks).
|
||||
* The log is immutable, so a true here is permanent for the session's life.
|
||||
*/
|
||||
function sessionHasImage(events: readonly SessionEvent[]): boolean {
|
||||
return events.some((event) => {
|
||||
const data = event.data as { content?: unknown; message?: { content?: unknown }; chunk?: { type?: unknown; block?: unknown } }
|
||||
if (contentHasImage(data.content)) return true
|
||||
if (data.message !== undefined && contentHasImage(data.message.content)) return true
|
||||
return event.type === 'assistant/chunk' && data.chunk?.type === 'block-end' && contentHasImage([data.chunk.block])
|
||||
})
|
||||
}
|
||||
|
||||
function referencedImage(events: readonly SessionEvent[], attachmentId: string): ImageAttachmentRef | undefined {
|
||||
for (const event of events) {
|
||||
const data = event.data as { content?: unknown; chunk?: { type?: unknown; block?: unknown } }
|
||||
@@ -1111,6 +1137,20 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro
|
||||
? {}
|
||||
: { reasoningEffort: ReasoningEffortId(reasoningEffort) },
|
||||
})
|
||||
// An image-bearing log replays into every later request, and both
|
||||
// wire routes reject image content on text-only models — accepting
|
||||
// this selection would strand the session (every turn fails, no
|
||||
// in-product recovery). Refuse at the selection boundary instead.
|
||||
if (sessionHasImage(found.agent.session.events)) {
|
||||
const info = await ctx.llm.resolveModelInfo(resolved.provider, resolved.model)
|
||||
if (info.inputModalities !== undefined && !info.inputModalities.includes('image')) {
|
||||
return err(request, {
|
||||
code: 'model-unavailable',
|
||||
message: `Model "${resolved.model}" does not accept image input, but this session's history already contains images; select an image-capable model.`,
|
||||
details: { provider, model },
|
||||
})
|
||||
}
|
||||
}
|
||||
const selected: AgentLlmTarget = {
|
||||
provider: resolved.provider,
|
||||
model: resolved.model,
|
||||
|
||||
@@ -198,7 +198,7 @@ export const imageMediaTypeSchema = z.union([
|
||||
/** Prompt wire content is intentionally narrower than merge-extensible durable core content. */
|
||||
export const promptContentPartSchema = z.discriminatedUnion('type', [
|
||||
z.object({ type: z.literal('text'), text: z.string() }),
|
||||
z.object({ type: z.literal('image'), mediaType: imageMediaTypeSchema, data: z.string(), name: z.string().optional() }),
|
||||
z.object({ type: z.literal('image'), mediaType: imageMediaTypeSchema, data: z.string(), name: z.string().optional(), alt: z.string().optional() }),
|
||||
])
|
||||
|
||||
/** session.prompt request payload. */
|
||||
|
||||
@@ -162,7 +162,7 @@ export interface SessionSummary {
|
||||
/** Browser-submitted prompt content; image bytes are promoted to durable references by the host. */
|
||||
export type PromptContentPart =
|
||||
| { type: 'text'; text: string }
|
||||
| { type: 'image'; mediaType: ImageMediaType; data: string; name?: string }
|
||||
| { type: 'image'; mediaType: ImageMediaType; data: string; name?: string; alt?: string }
|
||||
|
||||
/** Session-domain unary methods (the map keys session.* of RpcMethodMap). */
|
||||
export interface SessionsApi {
|
||||
|
||||
@@ -230,4 +230,91 @@ describe('Web session model selection', () => {
|
||||
.toEqual({ provider: 'deepseek', model: 'private-preview', reasoningEffort: 'max' })
|
||||
await ctx.fiber.dispose()
|
||||
})
|
||||
|
||||
it('refuses a text-only selection once the session log carries an image', async () => {
|
||||
const { ctx, sessionId, agent } = await harness()
|
||||
ctx.llm.registerAdapter(['text-only'], new class extends CatalogAdapter {
|
||||
override resolveModel(provider: string, model: string): Promise<LlmResolvedModelInfo> {
|
||||
return Promise.resolve({ provider, id: model, name: model, inputModalities: ['text'] })
|
||||
}
|
||||
}('Text Only', []))
|
||||
ctx.llm.registerAdapter(['vision'], new class extends CatalogAdapter {
|
||||
override resolveModel(provider: string, model: string): Promise<LlmResolvedModelInfo> {
|
||||
return Promise.resolve({ provider, id: model, name: model, inputModalities: ['text', 'image'] })
|
||||
}
|
||||
}('Vision', []))
|
||||
const api = createApiProxy(ctx, { provider: 'deepseek', model: 'deepseek-chat', cwd: '/tmp', workspaceRoot: '/tmp' })
|
||||
|
||||
// Before any image lands, a text-only selection is legitimate.
|
||||
expect(expectValue(await api.sessions.selectModel(request({
|
||||
sessionId, provider: 'text-only', model: 'plain',
|
||||
}))).selected).toEqual({ provider: 'text-only', model: 'plain' })
|
||||
|
||||
agent.session.append('user/message', {
|
||||
id: 'msg-image', role: 'user', source: { kind: 'user' },
|
||||
content: [{ type: 'image', attachment: { attachmentId: 'att-1', mediaType: 'image/png', bytes: 8, width: 1, height: 1 } }],
|
||||
} as never, { surfaceOp: 'append' })
|
||||
|
||||
// The log is immutable: a text-only route would fail every later turn.
|
||||
const stranded = await api.sessions.selectModel(request({
|
||||
sessionId, provider: 'text-only', model: 'plain',
|
||||
}))
|
||||
expect(stranded.result).toMatchObject({
|
||||
ok: false,
|
||||
error: { code: 'model-unavailable', message: expect.stringMatching(/history already contains images/) as unknown },
|
||||
})
|
||||
|
||||
// Image-capable and modality-unknown routes stay selectable.
|
||||
expect(expectValue(await api.sessions.selectModel(request({
|
||||
sessionId, provider: 'vision', model: 'sees',
|
||||
}))).selected).toEqual({ provider: 'vision', model: 'sees' })
|
||||
expect(expectValue(await api.sessions.selectModel(request({
|
||||
sessionId, provider: 'deepseek', model: 'deepseek-chat',
|
||||
}))).selected).toEqual({ provider: 'deepseek', model: 'deepseek-chat', reasoningEffort: 'high' })
|
||||
await ctx.fiber.dispose()
|
||||
})
|
||||
|
||||
it('detects images on every replayed route: wrapped messages, streamed blocks, nested tool results', async () => {
|
||||
const image = { type: 'image', attachment: { attachmentId: 'att-x', mediaType: 'image/png', bytes: 8, width: 1, height: 1 } }
|
||||
const cases: { label: string; append: (agent: Agent) => void }[] = [
|
||||
{
|
||||
label: 'steering message wrapper',
|
||||
append: (agent) => {
|
||||
agent.session.append('steering/message', {
|
||||
turn: 1, message: { id: 'st-1', role: 'user', source: { kind: 'user' }, content: [image] },
|
||||
} as never, { surfaceOp: 'append' })
|
||||
},
|
||||
},
|
||||
{
|
||||
label: 'streamed assistant block',
|
||||
append: (agent) => {
|
||||
agent.session.append('assistant/chunk', {
|
||||
turn: 1, step: 0, chunk: { type: 'block-end', index: 0, block: image },
|
||||
} as never)
|
||||
},
|
||||
},
|
||||
{
|
||||
label: 'nested tool-result content',
|
||||
append: (agent) => {
|
||||
agent.session.append('user/message', {
|
||||
id: 'tr-1', role: 'user', source: { kind: 'tool', callId: 'c1' },
|
||||
content: [{ type: 'tool-result', toolCallId: 'c1', content: [image], isError: false }],
|
||||
} as never, { surfaceOp: 'append' })
|
||||
},
|
||||
},
|
||||
]
|
||||
for (const { label, append } of cases) {
|
||||
const { ctx, sessionId, agent } = await harness()
|
||||
ctx.llm.registerAdapter(['text-only'], new class extends CatalogAdapter {
|
||||
override resolveModel(provider: string, model: string): Promise<LlmResolvedModelInfo> {
|
||||
return Promise.resolve({ provider, id: model, name: model, inputModalities: ['text'] })
|
||||
}
|
||||
}('Text Only', []))
|
||||
const api = createApiProxy(ctx, { provider: 'deepseek', model: 'deepseek-chat', cwd: '/tmp', workspaceRoot: '/tmp' })
|
||||
append(agent)
|
||||
const stranded = await api.sessions.selectModel(request({ sessionId, provider: 'text-only', model: 'plain' }))
|
||||
expect(stranded.result.ok, label).toBe(false)
|
||||
await ctx.fiber.dispose()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -333,11 +333,23 @@ export class LlmService extends Service {
|
||||
'INVALID_MODEL_CONTEXT',
|
||||
)
|
||||
}
|
||||
for (const modalities of [resolved.inputModalities, resolved.outputModalities]) {
|
||||
if (modalities !== undefined && (!Array.isArray(modalities) || modalities.some(m => typeof m !== 'string'))) {
|
||||
throw new LlmError(
|
||||
`adapter returned invalid modality metadata for provider "${provider}" model "${model}"`,
|
||||
'INVALID_MODEL_INFO',
|
||||
)
|
||||
}
|
||||
}
|
||||
const info: LlmResolvedModelInfo = {
|
||||
provider,
|
||||
id: model,
|
||||
name: resolved.name,
|
||||
...resolved.description === undefined ? {} : { description: resolved.description },
|
||||
// Capability metadata rides through: an explicit modality omission is
|
||||
// negative capability downstream preflights act on (image admission).
|
||||
...resolved.inputModalities === undefined ? {} : { inputModalities: resolved.inputModalities },
|
||||
...resolved.outputModalities === undefined ? {} : { outputModalities: resolved.outputModalities },
|
||||
...context === undefined ? {} : { context: { contextWindow: context.contextWindow } },
|
||||
}
|
||||
const reasoning = resolved.reasoning
|
||||
|
||||
@@ -47,12 +47,17 @@ export interface ReasoningBlock {
|
||||
text: string
|
||||
}
|
||||
|
||||
/** A durable raster image reference, valid in user or assistant content. */
|
||||
/**
|
||||
* A durable raster image reference, valid in user or assistant content. The
|
||||
* block is deliberately role-neutral; assistant-side rendering is forward
|
||||
* compatibility — the current production adapters declare text-only output,
|
||||
* so only user content carries images today.
|
||||
*/
|
||||
export interface ImageBlock {
|
||||
type: 'image'
|
||||
/** Immutable bytes and intrinsic display metadata owned by the attachment service. */
|
||||
attachment: ImageAttachmentRef
|
||||
/** Optional provider- and UI-facing alternative text. */
|
||||
/** Optional provider- and UI-facing alternative text, carried from the prompt wire's image part. */
|
||||
alt?: string
|
||||
}
|
||||
|
||||
|
||||
@@ -857,6 +857,8 @@ describe('LlmService', () => {
|
||||
[{ provider: 'route', id: 'model', name: 1 }, 'non-string name'],
|
||||
[{ provider: 'route', id: 'model', name: '' }, 'empty name'],
|
||||
[{ provider: 'route', id: 'model', name: 'Model', description: 1 }, 'non-string description'],
|
||||
[{ provider: 'route', id: 'model', name: 'Model', inputModalities: 'text' }, 'non-array input modalities'],
|
||||
[{ provider: 'route', id: 'model', name: 'Model', outputModalities: [1] }, 'non-string output modality'],
|
||||
] as const)('rejects invalid exact model metadata (%s: %s)', async (metadata, _label) => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LlmService)
|
||||
@@ -871,6 +873,27 @@ describe('LlmService', () => {
|
||||
.rejects.toMatchObject({ code: 'INVALID_MODEL_INFO' })
|
||||
})
|
||||
|
||||
it('preserves modality metadata through exact model resolution', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LlmService)
|
||||
const adapter = new class extends ScriptedAdapter {
|
||||
override resolveModel(): Promise<LlmResolvedModelInfo> {
|
||||
return Promise.resolve({
|
||||
provider: 'route', id: 'model', name: 'Model',
|
||||
inputModalities: ['text', 'image'], outputModalities: ['text'],
|
||||
})
|
||||
}
|
||||
}(SCRIPT)
|
||||
ctx.llm.registerAdapter(['route'], adapter)
|
||||
|
||||
// Downstream preflights (image admission) act on this exact field; a
|
||||
// rebuild that drops it silently reads as "modalities unknown".
|
||||
await expect(ctx.llm.resolveModelInfo('route', 'model')).resolves.toEqual({
|
||||
provider: 'route', id: 'model', name: 'Model',
|
||||
inputModalities: ['text', 'image'], outputModalities: ['text'],
|
||||
})
|
||||
})
|
||||
|
||||
it('resolves detached model context independently of advisory catalog membership', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(LlmService)
|
||||
|
||||
Reference in New Issue
Block a user