diff --git a/docs/design/dingtalk-richtext-multi-image.md b/docs/design/dingtalk-richtext-multi-image.md new file mode 100644 index 0000000000..458b59e7c8 --- /dev/null +++ b/docs/design/dingtalk-richtext-multi-image.md @@ -0,0 +1,70 @@ +# DingTalk rich-text multi-image delivery + +Issue: [#9878](https://github.com/QwenLM/qwen-code/issues/9878) + +## Problem + +A DingTalk message can contain several `picture` parts in one +`content.richText` array. The DingTalk adapter extracts every part into +`downloadCodes[]`, but the processing path calls `attachMedia` only for +`downloadCodes[0]`. Consequently the model and daemon-backed Web Shell receive +only the first image. + +This design covers multiple images in one DingTalk callback. It does not change +how separate messages received during an active turn are steered or collected. + +## Design + +Extend `ChannelAgentBridgePromptOptions` with an ordered `images` collection +whose entries contain base64 data and MIME type. Keep the existing +`imageBase64` and `imageMimeType` fields as compatibility inputs for adapters +and bridge callers that still send one image. + +`ChannelBase` will normalize the legacy image fields and every data-backed +image attachment into one ordered collection. It will pass that collection to +the bridge once per inbound turn. File-backed non-image attachments retain the +existing prompt-path behavior. + +The DingTalk adapter will call `attachMedia` for every download code extracted +from a rich-text callback. Image media from the same callback will remain +data-backed so that `ChannelBase` can pass every image as a native vision +input, rather than degrading later images into file-path instructions. + +`AcpBridge` will emit one ACP image content block per normalized image before +the text block. `DaemonChannelBridge` will upload each image to the owning +session and place every returned attachment reference before the text block. +This lets the daemon persist all references and lets Web Shell render every +image in the transcript. + +## Compatibility + +- Existing adapters using `imageBase64` and `imageMimeType` continue to work. +- Existing one-image callers keep their current ordering and behavior. +- An invalid partial legacy pair is ignored as it is today. +- No new configuration or dependency is introduced. + +## Failure behavior + +DingTalk media downloads remain sequential and preserve callback order. A +failed download keeps the existing per-media warning behavior and does not +prevent successfully downloaded images from reaching the turn. Daemon upload +failure keeps the current prompt failure semantics; the turn is not submitted +with a silently incomplete image set. + +## Tests + +1. DingTalk adapter: one `richText` callback with multiple picture parts + downloads every code and produces ordered image attachments. +2. ChannelBase: legacy and structured image inputs normalize into an ordered + bridge image collection without dropping later attachments. +3. ACP bridge: every image becomes an ACP image content block before text. +4. Daemon bridge: every image is uploaded, referenced in the prompt, and + therefore available to session persistence and Web Shell replay. +5. Existing single-image tests remain green to prove compatibility. + +## Acceptance criteria + +Sending five images together in one DingTalk message produces five attachment +references in the daemon session turn, displays five images in Web Shell, and +provides all five images to the selected multimodal model in the original +order. diff --git a/docs/plans/dingtalk-richtext-multi-image.md b/docs/plans/dingtalk-richtext-multi-image.md new file mode 100644 index 0000000000..738d0711ce --- /dev/null +++ b/docs/plans/dingtalk-richtext-multi-image.md @@ -0,0 +1,313 @@ +# DingTalk Rich-Text Multi-Image Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Deliver every image from one DingTalk `richText` callback to the multimodal model and persist every image reference for Web Shell replay. + +**Architecture:** Normalize legacy single-image input and structured image attachments into an ordered bridge-level image array. DingTalk downloads every picture part, while ACP emits every image block and the daemon uploads every image before submitting one prompt. + +**Tech Stack:** TypeScript, Vitest, DingTalk Stream adapter, Agent Client Protocol, Qwen daemon session attachment API. + +**Spec:** `docs/design/dingtalk-richtext-multi-image.md` + +## Global Constraints + +- Preserve `imageBase64` and `imageMimeType` compatibility for existing adapters. +- Preserve source order from `content.richText[]` through the model prompt and Web Shell transcript. +- Do not change collection of separate messages received during an active turn. +- Add no dependency or configuration. +- Run tests from each package directory. + +--- + +### Task 1: DingTalk downloads every picture part + +**Files:** + +- Modify: `packages/channels/dingtalk/src/DingtalkAdapter.ts:2119-2140,2343-2353` +- Test: `packages/channels/dingtalk/src/DingtalkAdapter.test.ts` + +**Interfaces:** + +- Consumes: `extractContent(data).downloadCodes: string[]` in callback order. +- Produces: `Envelope.attachments` containing one data-backed image per successful download in the same order. + +- [ ] **Step 1: Write the failing adapter test** + +Add a test that sends one `richText` callback with two literal picture parts, returns distinct bytes for each `downloadCode`, and asserts that both codes were downloaded and both base64 attachments reached `handleInbound` in order. + +```ts +expect(downloadCodes).toEqual(['picture-1', 'picture-2']); +expect(envelope.attachments).toEqual([ + { + type: 'image', + data: Buffer.from([1]).toString('base64'), + mimeType: 'image/png', + }, + { + type: 'image', + data: Buffer.from([2]).toString('base64'), + mimeType: 'image/png', + }, +]); +``` + +- [ ] **Step 2: Run the test and verify RED** + +Run: + +```bash +cd packages/channels/dingtalk && npx vitest run src/DingtalkAdapter.test.ts -t "downloads every picture in one richText callback" +``` + +Expected: FAIL because only `picture-1` is requested and only one attachment exists. + +- [ ] **Step 3: Implement ordered multi-download** + +Replace the first-only call with a sequential loop: + +```ts +for (const downloadCode of content.downloadCodes) { + await this.attachMedia( + envelope, + downloadCode, + content.mediaType, + content.fileName, + content.placeholder, + ); +} +``` + +Make every successful image download append a data-backed attachment; retain the existing temp-file path for file, audio, and video media. + +- [ ] **Step 4: Run the targeted adapter tests and verify GREEN** + +```bash +cd packages/channels/dingtalk && npx vitest run src/DingtalkAdapter.test.ts +``` + +Expected: PASS, including updated quoted-two-image expectations that both images are data-backed. + +- [ ] **Step 5: Commit the adapter behavior** + +```bash +git add packages/channels/dingtalk/src/DingtalkAdapter.ts packages/channels/dingtalk/src/DingtalkAdapter.test.ts +git commit -m "fix(channels): retain all DingTalk rich-text images" +``` + +### Task 2: ChannelBase carries an ordered image collection + +**Files:** + +- Modify: `packages/channels/base/src/ChannelAgentBridge.ts:110-116` +- Modify: `packages/channels/base/src/ChannelBase.ts:5251-5284,5656-5661` +- Test: `packages/channels/base/src/ChannelBase.test.ts` + +**Interfaces:** + +- Produces: `ChannelPromptImage { data: string; mimeType: string }` and `ChannelAgentBridgePromptOptions.images?: ChannelPromptImage[]`. +- Compatibility input: `imageBase64?: string` plus `imageMimeType?: string`. + +- [ ] **Step 1: Write failing ChannelBase tests** + +Add one test with two data-backed `Envelope.attachments` and assert the bridge receives: + +```ts +images: [ + { data: 'first', mimeType: 'image/png' }, + { data: 'second', mimeType: 'image/jpeg' }, +]; +``` + +Keep the legacy test and change its assertion to the same one-element `images` shape. + +- [ ] **Step 2: Run the focused tests and verify RED** + +```bash +cd packages/channels/base && npx vitest run src/ChannelBase.test.ts -t "image" +``` + +Expected: FAIL because the bridge options contain only singular image fields. + +- [ ] **Step 3: Add the bridge image type and normalization** + +```ts +export interface ChannelPromptImage { + data: string; + mimeType: string; +} + +export interface ChannelAgentBridgePromptOptions { + images?: ChannelPromptImage[]; + imageBase64?: string; + imageMimeType?: string; + displayText?: string; +} +``` + +Build `images` in legacy-first, attachment-order sequence and pass it to `promptBridge.prompt`. Preserve file-path rendering for attachments without image data. + +- [ ] **Step 4: Run ChannelBase tests and verify GREEN** + +```bash +cd packages/channels/base && npx vitest run src/ChannelBase.test.ts +``` + +Expected: PASS with no change to non-image attachment behavior. + +- [ ] **Step 5: Commit the bridge contract** + +```bash +git add packages/channels/base/src/ChannelAgentBridge.ts packages/channels/base/src/ChannelBase.ts packages/channels/base/src/ChannelBase.test.ts +git commit -m "feat(channels): carry ordered prompt images" +``` + +### Task 3: ACP sends every image as a native content block + +**Files:** + +- Modify: `packages/channels/base/src/AcpBridge.ts:285-299` +- Test: `packages/channels/base/src/AcpBridge.test.ts` + +**Interfaces:** + +- Consumes: `ChannelAgentBridgePromptOptions.images` with legacy single-image fallback. +- Produces: ACP prompt content containing every `{ type: 'image', data, mimeType }` block before text. + +- [ ] **Step 1: Write the failing ACP test** + +Call `prompt` with two literal images and assert the connection receives: + +```ts +prompt: [ + { type: 'image', data: 'first', mimeType: 'image/png' }, + { type: 'image', data: 'second', mimeType: 'image/jpeg' }, + { type: 'text', text: 'describe both' }, +]; +``` + +- [ ] **Step 2: Run the ACP test and verify RED** + +```bash +cd packages/channels/base && npx vitest run src/AcpBridge.test.ts -t "multiple images" +``` + +Expected: FAIL because `images` is not consumed. + +- [ ] **Step 3: Implement image iteration** + +Normalize `options.images` with the legacy pair as fallback, push each image block, then push the text block. Do not alter ACP metadata. + +- [ ] **Step 4: Run ACP tests and verify GREEN** + +```bash +cd packages/channels/base && npx vitest run src/AcpBridge.test.ts +``` + +Expected: PASS, including existing single-image behavior. + +- [ ] **Step 5: Commit ACP support** + +```bash +git add packages/channels/base/src/AcpBridge.ts packages/channels/base/src/AcpBridge.test.ts +git commit -m "feat(channels): send all prompt images over ACP" +``` + +### Task 4: Daemon persists every image for Web Shell + +**Files:** + +- Modify: `packages/channels/base/src/DaemonChannelBridge.ts:43-51,125-142,428-441` +- Test: `packages/channels/base/src/DaemonChannelBridge.test.ts:79-101,1889-1948` + +**Interfaces:** + +- Consumes: ordered `ChannelAgentBridgePromptOptions.images` with legacy fallback. +- Produces: one `session.uploadAttachment` call and one attachment reference prompt block per image. + +- [ ] **Step 1: Extend the existing failing daemon replay test** + +Pass two images, return two distinct attachment references, and assert upload order plus this prompt: + +```ts +prompt: [ + { type: 'image', attachmentId: 'image.png', mimeType: 'image/png', size: 12 }, + { + type: 'image', + attachmentId: 'image-2.jpeg', + mimeType: 'image/jpeg', + size: 13, + }, + { type: 'text', text: 'describe' }, +]; +``` + +- [ ] **Step 2: Run the daemon test and verify RED** + +```bash +cd packages/channels/base && npx vitest run src/DaemonChannelBridge.test.ts -t "stores channel images" +``` + +Expected: FAIL because only the singular image is uploaded. + +- [ ] **Step 3: Implement ordered uploads with unique names** + +Iterate all normalized images, generate deterministic names (`image.png`, `image-2.jpeg`, ...), await each `uploadAttachment`, and push every returned reference before text. + +- [ ] **Step 4: Run daemon tests and verify GREEN** + +```bash +cd packages/channels/base && npx vitest run src/DaemonChannelBridge.test.ts +``` + +Expected: PASS and the prompt contains all persisted attachment references. + +- [ ] **Step 5: Commit daemon persistence** + +```bash +git add packages/channels/base/src/DaemonChannelBridge.ts packages/channels/base/src/DaemonChannelBridge.test.ts +git commit -m "feat(channels): persist all channel images in daemon sessions" +``` + +### Task 5: Verify the complete behavior + +**Files:** + +- Modify only if verification exposes a defect in the files above. +- Record runtime evidence in the final handoff and Issue/PR text; do not commit credentials or callback payloads. + +**Interfaces:** + +- Consumes: built channel packages and the configured local DingTalk daemon. +- Produces: unit, build, typecheck, and user-visible Web Shell evidence. + +- [ ] **Step 1: Run package verification** + +```bash +npm run build +npx tsc --noEmit -p packages/channels/base/tsconfig.json +npx tsc --noEmit -p packages/channels/dingtalk/tsconfig.json +cd packages/channels/base && npx vitest run src/ChannelBase.test.ts src/AcpBridge.test.ts src/DaemonChannelBridge.test.ts +cd packages/channels/dingtalk && npx vitest run src/DingtalkAdapter.test.ts +``` + +Expected: all commands exit 0. + +- [ ] **Step 2: Reload the configured channel worker** + +Run `npm run dev -- channel reload`, then confirm `npm run dev -- channel status` reports the DingTalk worker running. + +- [ ] **Step 3: Perform the five-image E2E** + +Send five hand images together as one DingTalk message and verify: + +- one DingTalk callback is accepted; +- five media downloads complete; +- five daemon attachment uploads return HTTP 201; +- the persisted user turn has five `attachmentReferences` in order; +- Web Shell shows five image previews; +- the model reports five hands. + +- [ ] **Step 4: Self-audit the final diff** + +Read `git diff HEAD^` and all untracked files without filtering for expected changes. Verify no secrets, callback payloads, unrelated lockfile changes, or separate-message buffer behavior entered the patch. diff --git a/packages/channels/base/README.md b/packages/channels/base/README.md index 0bd0de3be1..8f94cdf075 100644 --- a/packages/channels/base/README.md +++ b/packages/channels/base/README.md @@ -197,7 +197,11 @@ interface ChannelAgentBridge { prompt( sessionId: string, text: string, - options?: { imageBase64?: string; imageMimeType?: string }, + options?: { + images?: Array<{ data: string; mimeType: string }>; // ordered, preferred + imageBase64?: string; // legacy fallback (first image only) + imageMimeType?: string; // legacy fallback (first image only) + }, ): Promise; cancelSession(sessionId: string): Promise; shellCommand?( @@ -208,6 +212,8 @@ interface ChannelAgentBridge { } ``` +`prompt` carries images through `options.images` — an ordered array of `{ data, mimeType }` entries delivered to the model in array order. The legacy `imageBase64`/`imageMimeType` pair is a fallback that carries only the first image. Built-in bridges normalize MIME types before use: lowercased, parameters stripped (`image/png; charset=binary` → `image/png`), and the `image/jpg` alias mapped to `image/jpeg`. Daemon-backed bridges upload images to the daemon attachment store and restrict them to its supported subtypes (`image/bmp`, `image/gif`, `image/jpeg`, `image/png`, `image/webp`) and its per-item admission rule (non-empty and at most 8 MiB once decoded); daemons without attachment support take the same images inline in the prompt body, kept under an aggregate base64 budget. Images outside those limits are skipped with a log line instead of failing the turn, while `AcpBridge` delivers any normalized `image/*` inline. + ### AcpBridge `AcpBridge` is the current implementation used by standalone `qwen channel start`. It manages the `qwen-code --acp` child process and implements `ChannelAgentBridge`. @@ -216,14 +222,14 @@ interface ChannelAgentBridge { constructor(options: { cliEntryPath: string; cwd: string; model?: string }) ``` -| Method | Description | -| ----------------------------------- | ----------------------------------------------------------------------------------------------------------------- | -| `start()` | Spawn the agent process | -| `stop()` | Kill the agent process | -| `newSession(cwd)` | Create a new ACP session, returns `sessionId` | -| `loadSession(sessionId, cwd)` | Restore an existing session | -| `prompt(sessionId, text, options?)` | Send a message to the agent, returns the full response text. Supports optional `imageBase64` and `imageMimeType`. | -| `isConnected` | Whether the agent process is alive | +| Method | Description | +| ----------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `start()` | Spawn the agent process | +| `stop()` | Kill the agent process | +| `newSession(cwd)` | Create a new ACP session, returns `sessionId` | +| `loadSession(sessionId, cwd)` | Restore an existing session | +| `prompt(sessionId, text, options?)` | Send a message to the agent, returns the full response text. Supports an ordered `images` array (`{ data, mimeType }` per image, preferred), falling back to the legacy `imageBase64`/`imageMimeType` pair for a single image. | +| `isConnected` | Whether the agent process is alive | **Events** (EventEmitter): diff --git a/packages/channels/base/src/AcpBridge.test.ts b/packages/channels/base/src/AcpBridge.test.ts index c7d807b0ad..4a21304882 100644 --- a/packages/channels/base/src/AcpBridge.test.ts +++ b/packages/channels/base/src/AcpBridge.test.ts @@ -12,6 +12,7 @@ import { ACP_PRIVATE_PARENT_CAPABILITY_META_KEY, CHANNEL_PROMPT_META_KEY, type ChannelLoopToolHandler, + type ChannelPromptImage, } from './ChannelAgentBridge.js'; const child = vi.hoisted(() => { @@ -515,6 +516,85 @@ describe('AcpBridge', () => { }); }); + it('sends multiple images before the text prompt', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }) as unknown as TestableAcpBridge; + const prompt = vi.fn().mockResolvedValue({}); + bridge.child = { killed: false, exitCode: null }; + bridge.connection = { extMethod: vi.fn(), prompt }; + + await bridge.prompt('s-1', 'describe both', { + images: [ + { data: 'first', mimeType: 'image/png' }, + { data: 'second', mimeType: 'image/jpeg' }, + ], + }); + + expect(prompt).toHaveBeenCalledWith({ + sessionId: 's-1', + prompt: [ + { type: 'image', data: 'first', mimeType: 'image/png' }, + { type: 'image', data: 'second', mimeType: 'image/jpeg' }, + { type: 'text', text: 'describe both' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }); + }); + + it('sends a legacy-only image pair as one inline image block', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }) as unknown as TestableAcpBridge; + const prompt = vi.fn().mockResolvedValue({}); + bridge.child = { killed: false, exitCode: null }; + bridge.connection = { extMethod: vi.fn(), prompt }; + + await bridge.prompt('s-1', 'describe', { + imageBase64: 'base64-image', + imageMimeType: 'image/png', + }); + + expect(prompt).toHaveBeenCalledWith({ + sessionId: 's-1', + prompt: [ + { type: 'image', data: 'base64-image', mimeType: 'image/png' }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }); + }); + + it('drops malformed prompt image entries before the text prompt', async () => { + const bridge = new AcpBridge({ + cliEntryPath: '/tmp/qwen', + cwd: '/tmp', + }) as unknown as TestableAcpBridge; + const prompt = vi.fn().mockResolvedValue({}); + bridge.child = { killed: false, exitCode: null }; + bridge.connection = { extMethod: vi.fn(), prompt }; + + await bridge.prompt('s-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { data: '', mimeType: 'image/webp' }, + { data: 'BAUG', mimeType: '' }, + null as unknown as ChannelPromptImage, + ], + }); + + expect(prompt).toHaveBeenCalledWith({ + sessionId: 's-1', + prompt: [ + { type: 'image', data: 'AQID', mimeType: 'image/png' }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }); + }); + it('excludes nested subagent text from the final response', async () => { const bridge = new AcpBridge({ cliEntryPath: '/tmp/qwen', diff --git a/packages/channels/base/src/AcpBridge.ts b/packages/channels/base/src/AcpBridge.ts index d216c7f210..48eb8b7209 100644 --- a/packages/channels/base/src/AcpBridge.ts +++ b/packages/channels/base/src/AcpBridge.ts @@ -19,6 +19,7 @@ import { ACP_PRIVATE_PARENT_CAPABILITY_META_KEY, CHANNEL_PROMPT_DISPLAY_TEXT_META_KEY, CHANNEL_PROMPT_META_KEY, + resolvePromptImages, type AvailableCommand, type ChannelAgentBridge, type ChannelAgentBridgePromptOptions, @@ -283,11 +284,11 @@ export class AcpBridge extends EventEmitter implements ChannelAgentBridge { this.on('responseBoundary', clearChunks); const prompt: Array> = []; - if (options?.imageBase64 && options.imageMimeType) { + for (const image of resolvePromptImages(options)) { prompt.push({ type: 'image', - data: options.imageBase64, - mimeType: options.imageMimeType, + data: image.data, + mimeType: image.mimeType, }); } prompt.push({ type: 'text', text }); diff --git a/packages/channels/base/src/ChannelAgentBridge.ts b/packages/channels/base/src/ChannelAgentBridge.ts index 01fff0cb20..6f46f9a5a3 100644 --- a/packages/channels/base/src/ChannelAgentBridge.ts +++ b/packages/channels/base/src/ChannelAgentBridge.ts @@ -107,7 +107,13 @@ export interface ChannelAgentBridgeSessionOptions { sourceId?: string; } +export interface ChannelPromptImage { + data: string; + mimeType: string; +} + export interface ChannelAgentBridgePromptOptions { + images?: ChannelPromptImage[]; imageBase64?: string; imageMimeType?: string; /** User-authored text shown in transcripts when `text` includes hidden context. @@ -115,6 +121,44 @@ export interface ChannelAgentBridgePromptOptions { displayText?: string; } +/** + * Resolves the ordered `images` contract, falling back to the legacy + * single-image pair, and normalizes MIME types in one place: channel + * adapters forward CDN `content-type` headers verbatim, so values arrive + * with parameters and mixed case (e.g. `image/png; charset=binary`), and + * the non-standard `image/jpg` alias rides them too. Entries missing + * `data` or `mimeType` are dropped so one malformed attachment degrades + * to a prompt without that image, like the legacy field guards did. + */ +export function resolvePromptImages( + options?: ChannelAgentBridgePromptOptions, +): ChannelPromptImage[] { + const images = + options?.images && options.images.length > 0 + ? options.images + : options?.imageBase64 && options.imageMimeType + ? [{ data: options.imageBase64, mimeType: options.imageMimeType }] + : []; + return images + .filter( + (image) => + !!image && + typeof image.data === 'string' && + image.data.length > 0 && + typeof image.mimeType === 'string' && + image.mimeType.length > 0, + ) + .map((image) => { + const cleaned = + image.mimeType.split(';', 1)[0]?.trim().toLowerCase() ?? ''; + return { + data: image.data, + // Normalize the alias like the daemon attachment store's own naming. + mimeType: cleaned === 'image/jpg' ? 'image/jpeg' : cleaned, + }; + }); +} + export interface ChannelAgentBridge { readonly availableCommands: AvailableCommand[]; getAvailableCommands?(sessionId: string): AvailableCommand[]; diff --git a/packages/channels/base/src/ChannelBase.test.ts b/packages/channels/base/src/ChannelBase.test.ts index e2c3e82926..27a376a59d 100644 --- a/packages/channels/base/src/ChannelBase.test.ts +++ b/packages/channels/base/src/ChannelBase.test.ts @@ -10189,7 +10189,7 @@ describe('ChannelBase', () => { expect(pathLine).not.toContain(rlo); }); - it('extracts image from attachments', async () => { + it('forwards every image attachment in order', async () => { const ch = createChannel(); await ch.handleInbound( envelope({ @@ -10200,11 +10200,20 @@ describe('ChannelBase', () => { data: 'base64data', mimeType: 'image/png', }, + { + type: 'image', + data: 'second-image', + mimeType: 'image/jpeg', + }, ], }), ); // eslint-disable-next-line @typescript-eslint/no-explicit-any const options = (bridge.prompt as any).mock.calls[0][2]; + expect(options.images).toEqual([ + { data: 'base64data', mimeType: 'image/png' }, + { data: 'second-image', mimeType: 'image/jpeg' }, + ]); expect(options.imageBase64).toBe('base64data'); expect(options.imageMimeType).toBe('image/png'); }); @@ -10220,7 +10229,33 @@ describe('ChannelBase', () => { ); // eslint-disable-next-line @typescript-eslint/no-explicit-any const options = (bridge.prompt as any).mock.calls[0][2]; - expect(options.imageBase64).toBe('legacydata'); + expect(options.images).toEqual([ + { data: 'legacydata', mimeType: 'image/jpeg' }, + ]); + }); + + it('orders the legacy image before attachment images', async () => { + const ch = createChannel(); + await ch.handleInbound( + envelope({ + text: 'see image', + imageBase64: 'legacydata', + imageMimeType: 'image/jpeg', + attachments: [ + { + type: 'image', + data: 'attachmentdata', + mimeType: 'image/png', + }, + ], + }), + ); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const options = (bridge.prompt as any).mock.calls[0][2]; + expect(options.images).toEqual([ + { data: 'legacydata', mimeType: 'image/jpeg' }, + { data: 'attachmentdata', mimeType: 'image/png' }, + ]); }); it('prepends instructions on first message only', async () => { diff --git a/packages/channels/base/src/ChannelBase.ts b/packages/channels/base/src/ChannelBase.ts index c47dc5b0a7..1505f4723a 100644 --- a/packages/channels/base/src/ChannelBase.ts +++ b/packages/channels/base/src/ChannelBase.ts @@ -54,6 +54,7 @@ import { import type { AvailableCommand, ChannelAgentBridge, + ChannelPromptImage, ChannelLoopToolCreateInput, ChannelLoopToolResult, PermissionRequestEvent, @@ -5248,15 +5249,22 @@ export abstract class ChannelBase { promptText = `[Replying to: "${quoted}"]\n\n${promptText}`; } - // Resolve attachments: extract image for bridge, append file paths to text + // Resolve attachments: extract images for bridge, append file paths to text let imageBase64 = envelope.imageBase64; let imageMimeType = envelope.imageMimeType; + const images: ChannelPromptImage[] = []; + if (imageBase64 && imageMimeType) { + images.push({ data: imageBase64, mimeType: imageMimeType }); + } if (envelope.attachments?.length) { const filePaths: string[] = []; for (const att of envelope.attachments) { - if (att.type === 'image' && att.data && !imageBase64) { - imageBase64 = att.data; - imageMimeType = att.mimeType; + if (att.type === 'image' && att.data) { + images.push({ data: att.data, mimeType: att.mimeType }); + if (!imageBase64) { + imageBase64 = att.data; + imageMimeType = att.mimeType; + } } else if (att.filePath) { const label = att.type === 'file' ? 'file' : att.type; // The filename is attacker-supplied (e.g. DingTalk), so neutralize both @@ -5655,6 +5663,7 @@ export abstract class ChannelBase { try { const response = await promptBridge.prompt(sessionId, promptToSend, { + ...(images.length > 0 ? { images } : {}), imageBase64, imageMimeType, displayText, diff --git a/packages/channels/base/src/DaemonChannelBridge.test.ts b/packages/channels/base/src/DaemonChannelBridge.test.ts index 9b8275a063..d6b53605f6 100644 --- a/packages/channels/base/src/DaemonChannelBridge.test.ts +++ b/packages/channels/base/src/DaemonChannelBridge.test.ts @@ -12,6 +12,7 @@ import { import { CHANNEL_PROMPT_AUTHORIZATION_META_KEY, CHANNEL_PROMPT_META_KEY, + type ChannelPromptImage, } from './ChannelAgentBridge.js'; class EventQueue implements AsyncGenerator { @@ -79,6 +80,8 @@ class EventQueue implements AsyncGenerator { interface FakeSession extends DaemonChannelSessionClient { prompt: ReturnType; + uploadAttachment: ReturnType; + removeAttachment: ReturnType; events: ReturnType; cancel: ReturnType; setModel: ReturnType; @@ -94,6 +97,8 @@ function createFakeSession( workspaceCwd: '/repo', lastEventId: undefined, prompt: vi.fn().mockImplementation(async () => ({})), + uploadAttachment: vi.fn(), + removeAttachment: vi.fn().mockResolvedValue(true), events: vi.fn((opts?: { signal?: AbortSignal }) => { opts?.signal?.addEventListener('abort', () => events.close(), { once: true, @@ -1928,9 +1933,22 @@ describe('DaemonChannelBridge', () => { bridge.stop(); }); - it('passes image prompt blocks and aborts prompts when a session dies', async () => { + it('stores channel images so the daemon can replay them to Web Shell', async () => { const events = new EventQueue(); const session = createFakeSession(events); + session.uploadAttachment + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }) + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image-2.jpeg', + mimeType: 'image/jpeg', + size: 13, + }); session.prompt.mockImplementation( (_req: unknown, signal?: AbortSignal) => new Promise((_resolve, reject) => { @@ -1944,20 +1962,56 @@ describe('DaemonChannelBridge', () => { const bridge = new DaemonChannelBridge({ cwd: '/repo', sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, }); await bridge.start(); await bridge.newSession('/repo'); const promptPromise = bridge.prompt('session-1', 'describe', { - imageBase64: 'base64-image', - imageMimeType: 'image/png', + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { data: 'BAUG', mimeType: 'image/jpeg' }, + ], }); await waitFor(() => expect(session.prompt).toHaveBeenCalledOnce()); + expect(session.uploadAttachment).toHaveBeenNthCalledWith( + 1, + expect.any(Blob), + 'image.png', + 'image/png', + expect.any(AbortSignal), + ); + expect(session.uploadAttachment).toHaveBeenNthCalledWith( + 2, + expect.any(Blob), + 'image-2.jpeg', + 'image/jpeg', + expect.any(AbortSignal), + ); + const firstBlob = session.uploadAttachment.mock.calls[0]![0] as Blob; + const secondBlob = session.uploadAttachment.mock.calls[1]![0] as Blob; + expect(Buffer.from(await firstBlob.arrayBuffer())).toEqual( + Buffer.from([1, 2, 3]), + ); + expect(Buffer.from(await secondBlob.arrayBuffer())).toEqual( + Buffer.from([4, 5, 6]), + ); expect(session.prompt).toHaveBeenCalledWith( { prompt: [ - { type: 'image', data: 'base64-image', mimeType: 'image/png' }, + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }, + { + type: 'image', + attachmentId: 'image-2.jpeg', + mimeType: 'image/jpeg', + size: 13, + }, { type: 'text', text: 'describe' }, ], _meta: { [CHANNEL_PROMPT_META_KEY]: true }, @@ -1977,6 +2031,1251 @@ describe('DaemonChannelBridge', () => { bridge.stop(); }); + it('releases prompt state when a channel image upload fails', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment + .mockRejectedValueOnce(new Error('upload failed')) + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await expect( + bridge.prompt('session-1', 'first', { + images: [{ data: 'first-image', mimeType: 'image/png' }], + }), + ).rejects.toThrow('upload failed'); + expect(bridge.listSessions()).toEqual([ + { + sessionId: 'session-1', + workspaceCwd: '/repo', + hasActivePrompt: false, + }, + ]); + await expect( + bridge.prompt('session-1', 'second', { + images: [{ data: 'second-image', mimeType: 'image/png' }], + }), + ).resolves.toBe(''); + + events.close(); + bridge.stop(); + }); + + it('removes uploaded channel images when a later upload fails', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }) + .mockRejectedValueOnce(new Error('second upload failed')); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await expect( + bridge.prompt('session-1', 'describe', { + images: [ + { data: 'first-image', mimeType: 'image/png' }, + { data: 'second-image', mimeType: 'image/jpeg' }, + ], + }), + ).rejects.toThrow('second upload failed'); + expect(session.removeAttachment).toHaveBeenCalledWith('image.png'); + + events.close(); + bridge.stop(); + }); + + it('releases prompt state before a failed upload rollback settles', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }) + .mockRejectedValueOnce(new Error('second upload failed')); + let resolveRemoval: (removed: boolean) => void = () => {}; + session.removeAttachment.mockReturnValueOnce( + new Promise((resolve) => { + resolveRemoval = resolve; + }), + ); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + const promptPromise = bridge.prompt('session-1', 'describe', { + images: [ + { data: 'first-image', mimeType: 'image/png' }, + { data: 'second-image', mimeType: 'image/jpeg' }, + ], + }); + await waitFor(() => + expect(session.removeAttachment).toHaveBeenCalledOnce(), + ); + expect(bridge.listSessions()[0]?.hasActivePrompt).toBe(false); + resolveRemoval(true); + await expect(promptPromise).rejects.toThrow('second upload failed'); + + events.close(); + bridge.stop(); + }); + + it('normalizes channel image MIME types before uploading', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await bridge.prompt('session-1', 'describe', { + images: [{ data: 'base64-image', mimeType: 'IMAGE/PNG; charset=binary' }], + }); + expect(session.uploadAttachment).toHaveBeenCalledWith( + expect.any(Blob), + 'image.png', + 'image/png', + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('normalizes the image/jpg alias before uploading', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }) + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image-2.jpeg', + mimeType: 'image/jpeg', + size: 13, + }); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { data: 'BAUG', mimeType: 'Image/JPG' }, + ], + }); + expect(session.uploadAttachment).toHaveBeenNthCalledWith( + 2, + expect.any(Blob), + 'image-2.jpeg', + 'image/jpeg', + expect.any(AbortSignal), + ); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [ + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }, + { + type: 'image', + attachmentId: 'image-2.jpeg', + mimeType: 'image/jpeg', + size: 13, + }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('skips channel images with unrecognized MIME subtypes instead of failing the turn', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + let skippedWarning = ''; + try { + await bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { data: 'BAUG', mimeType: 'image/tiff' }, + ], + }); + skippedWarning = stderr.mock.calls.join(''); + } finally { + stderr.mockRestore(); + } + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + expect(skippedWarning).toContain('image/tiff'); + expect(skippedWarning).toContain('for session session-1'); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [ + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('skips channel images above the daemon attachment size limit instead of failing the turn', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + let skippedWarning = ''; + try { + await bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { + data: Buffer.alloc(8 * 1024 * 1024 + 1, 1).toString('base64'), + mimeType: 'image/jpeg', + }, + ], + }); + skippedWarning = stderr.mock.calls.join(''); + } finally { + stderr.mockRestore(); + } + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + expect(skippedWarning).toContain('image/jpeg'); + expect(skippedWarning).toContain('for session session-1'); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [ + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('skips channel images that decode to zero bytes instead of failing the turn', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + let skippedWarning = ''; + try { + await bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + // Invalid base64 decodes to zero bytes; the daemon attachment + // store rejects empty images with 400, which would fail the + // whole turn. + { data: 'A', mimeType: 'image/jpeg' }, + ], + }); + skippedWarning = stderr.mock.calls.join(''); + } finally { + stderr.mockRestore(); + } + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + expect(skippedWarning).toContain('image/jpeg'); + expect(skippedWarning).toContain('for session session-1'); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [ + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('uploads a channel image at exactly the daemon attachment size limit', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 8 * 1024 * 1024, + }); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await bridge.prompt('session-1', 'describe', { + images: [ + { + data: Buffer.alloc(8 * 1024 * 1024, 1).toString('base64'), + mimeType: 'image/png', + }, + ], + }); + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + + events.close(); + bridge.stop(); + }); + + it('rejects oversized channel images from the base64 length without decoding them', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bufferFrom = vi.spyOn(Buffer, 'from'); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + const oversized = Buffer.alloc(8 * 1024 * 1024 + 1, 1).toString('base64'); + bufferFrom.mockClear(); + let decodedOversized = false; + try { + await bridge.prompt('session-1', 'describe', { + images: [{ data: oversized, mimeType: 'image/jpeg' }], + }); + decodedOversized = bufferFrom.mock.calls.some( + (call) => call[0] === oversized, + ); + } finally { + stderr.mockRestore(); + bufferFrom.mockRestore(); + } + expect(session.uploadAttachment).not.toHaveBeenCalled(); + expect(decodedOversized).toBe(false); + + events.close(); + bridge.stop(); + }); + + it('skips malformed-padded channel images that decode past the size limit', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockRejectedValue( + new Error('daemon 413: Request body too large (max 8 MiB)'), + ); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + // Node's lenient base64 decoder ignores trailing padding that does not + // complete a quantum, so both inputs decode to the 8 MiB limit plus one + // byte even though a padding-counting length estimate stays at the limit. + const malformedOnePad = 'A'.repeat(11184812) + '='; + const malformedTwoPad = 'A'.repeat(11184812) + '=='; + let skippedWarning = ''; + try { + await bridge.prompt('session-1', 'describe', { + images: [ + { data: malformedOnePad, mimeType: 'image/png' }, + { data: malformedTwoPad, mimeType: 'image/jpeg' }, + ], + }); + skippedWarning = stderr.mock.calls.join(''); + } finally { + stderr.mockRestore(); + } + expect(session.uploadAttachment).not.toHaveBeenCalled(); + expect(skippedWarning).toContain('above the daemon attachment size limit'); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [{ type: 'text', text: 'describe' }], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('keeps prompt images inline when the daemon lacks session attachments', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { data: 'BAUG', mimeType: 'image/jpeg' }, + ], + }); + expect(session.uploadAttachment).not.toHaveBeenCalled(); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [ + { type: 'image', data: 'AQID', mimeType: 'image/png' }, + { type: 'image', data: 'BAUG', mimeType: 'image/jpeg' }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('keeps legacy session clients compatible when attachment methods are absent', async () => { + const events = new EventQueue(); + const prompt = vi.fn().mockResolvedValue({}); + const legacySession = { + sessionId: 'session-1', + workspaceCwd: '/repo', + lastEventId: undefined, + prompt, + events: vi.fn(() => events), + cancel: vi.fn().mockResolvedValue(undefined), + setModel: vi.fn().mockResolvedValue({}), + respondToPermission: vi.fn().mockResolvedValue(true), + }; + const sessionFactory = async (): Promise => + legacySession; + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory, + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await bridge.prompt('session-1', 'describe', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + expect(prompt).toHaveBeenCalledWith( + { + prompt: [ + { type: 'image', data: 'AQID', mimeType: 'image/png' }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('bounds the inline image payload for daemons without session attachments', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + // Three of these inflate to ~14 MiB of base64, past the daemon's + // 10mb prompt-body limit, so only the first fits the inline budget. + const large = Buffer.alloc(3.5 * 1024 * 1024, 1).toString('base64'); + let skippedWarning = ''; + try { + await bridge.prompt('session-1', 'describe', { + images: [ + { data: large, mimeType: 'image/png' }, + { data: large, mimeType: 'image/jpeg' }, + { data: large, mimeType: 'image/webp' }, + ], + }); + skippedWarning = stderr.mock.calls.join(''); + } finally { + stderr.mockRestore(); + } + expect(session.uploadAttachment).not.toHaveBeenCalled(); + expect(skippedWarning).toContain('image/jpeg'); + expect(skippedWarning).toContain('for session session-1'); + const promptCall = session.prompt.mock.calls[0]?.[0] as { + prompt: Array>; + }; + expect( + promptCall.prompt.filter((block) => block['type'] === 'image'), + ).toEqual([{ type: 'image', data: large, mimeType: 'image/png' }]); + expect(promptCall.prompt).toContainEqual({ + type: 'text', + text: 'describe', + }); + + events.close(); + bridge.stop(); + }); + + it('names the inline budget when skipping oversized inline channel images', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + // A daemon without session_attachments has no attachment store, so the + // skip line must name the inline budget, not the store's size limit. + const oversized = Buffer.alloc(8 * 1024 * 1024 + 1, 1).toString('base64'); + let skippedWarning = ''; + try { + await bridge.prompt('session-1', 'describe', { + images: [{ data: oversized, mimeType: 'image/jpeg' }], + }); + skippedWarning = stderr.mock.calls.join(''); + } finally { + stderr.mockRestore(); + } + expect(skippedWarning).toContain('above the inline image budget'); + expect(skippedWarning).not.toContain( + 'above the daemon attachment size limit', + ); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [{ type: 'text', text: 'describe' }], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('skips inline channel images that decode to nothing', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + let skippedWarning = ''; + try { + await bridge.prompt('session-1', 'describe', { + images: [{ data: 'A', mimeType: 'image/png' }], + }); + skippedWarning = stderr.mock.calls.join(''); + } finally { + stderr.mockRestore(); + } + expect(skippedWarning).toContain('image/png'); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [{ type: 'text', text: 'describe' }], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('uploads a legacy-only prompt image pair', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await bridge.prompt('session-1', 'describe', { + imageBase64: 'AQID', + imageMimeType: 'image/png', + }); + expect(session.uploadAttachment).toHaveBeenCalledWith( + expect.any(Blob), + 'image.png', + 'image/png', + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('uploads the legacy pair when images is empty', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await bridge.prompt('session-1', 'describe', { + images: [], + imageBase64: 'AQID', + imageMimeType: 'image/png', + }); + expect(session.uploadAttachment).toHaveBeenCalledWith( + expect.any(Blob), + 'image.png', + 'image/png', + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('drops malformed prompt image entries instead of failing the turn', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + // Extension adapters are out-of-contract input: entries can lack + // fields the type declares required, be empty, or be null. + { data: 'BAUG' } as ChannelPromptImage, + { mimeType: 'image/jpeg' } as ChannelPromptImage, + { data: '', mimeType: 'image/webp' }, + { data: 'BAUG', mimeType: '' }, + null as unknown as ChannelPromptImage, + ], + }); + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [ + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('skips channel images whose MIME type is not an image type', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + let skippedWarning = ''; + try { + await bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { data: 'BAUG', mimeType: 'audio/png' }, + ], + }); + skippedWarning = stderr.mock.calls.join(''); + } finally { + stderr.mockRestore(); + } + expect(session.uploadAttachment).toHaveBeenCalledOnce(); + expect(skippedWarning).toContain('audio/png'); + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [ + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('uploads channel images concurrently and keeps prompt order', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + let releaseFirst: ((value: Record) => void) | undefined; + session.uploadAttachment + .mockImplementationOnce( + () => + new Promise>((resolve) => { + releaseFirst = resolve; + }), + ) + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image-2.jpeg', + mimeType: 'image/jpeg', + size: 13, + }); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + const promptPromise = bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { data: 'BAUG', mimeType: 'image/jpeg' }, + ], + }); + // Sequential uploads would not start the second one while the first is + // still pending. + await waitFor(() => + expect(session.uploadAttachment).toHaveBeenCalledTimes(2), + ); + releaseFirst?.({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + await promptPromise; + expect(session.prompt).toHaveBeenCalledWith( + { + prompt: [ + { + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }, + { + type: 'image', + attachmentId: 'image-2.jpeg', + mimeType: 'image/jpeg', + size: 13, + }, + { type: 'text', text: 'describe' }, + ], + _meta: { [CHANNEL_PROMPT_META_KEY]: true }, + }, + expect.any(AbortSignal), + ); + + events.close(); + bridge.stop(); + }); + + it('removes uploaded channel images when cancelled before prompt admission', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + let finishUpload!: (value: Record) => void; + session.uploadAttachment.mockImplementationOnce( + () => + new Promise>((resolve) => { + finishUpload = resolve; + }), + ); + session.prompt.mockImplementationOnce(async (_request, signal) => { + signal?.throwIfAborted(); + return {}; + }); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + const promptPromise = bridge.prompt('session-1', 'describe', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + await waitFor(() => + expect(session.uploadAttachment).toHaveBeenCalledOnce(), + ); + finishUpload({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 3, + }); + await bridge.cancelSession('session-1'); + + await expect(promptPromise).rejects.toThrow('aborted'); + expect(session.prompt).not.toHaveBeenCalled(); + expect(session.removeAttachment).toHaveBeenCalledOnce(); + expect(session.removeAttachment).toHaveBeenCalledWith('image.png'); + + events.close(); + bridge.stop(); + }); + + it('removes uploaded channel images when the daemon rejects prompt admission', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }) + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image-2.jpeg', + mimeType: 'image/jpeg', + size: 13, + }); + session.prompt.mockRejectedValueOnce( + Object.assign(new Error('daemon 400: prompt admission denied'), { + name: 'DaemonHttpError', + status: 400, + }), + ); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await expect( + bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { data: 'BAUG', mimeType: 'image/jpeg' }, + ], + }), + ).rejects.toThrow('prompt admission denied'); + expect(session.removeAttachment).toHaveBeenCalledTimes(2); + expect(session.removeAttachment).toHaveBeenCalledWith('image.png'); + expect(session.removeAttachment).toHaveBeenCalledWith('image-2.jpeg'); + + events.close(); + bridge.stop(); + }); + + it('removes uploaded channel images when the local prompt queue is full', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + session.prompt.mockRejectedValueOnce( + Object.assign(new Error('Pending prompts full: "session-1" (1/1)'), { + name: 'DaemonPendingPromptLimitError', + }), + ); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await expect( + bridge.prompt('session-1', 'describe', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }), + ).rejects.toThrow('Pending prompts full'); + expect(session.removeAttachment).toHaveBeenCalledWith('image.png'); + + events.close(); + bridge.stop(); + }); + + it('keeps uploaded channel images when an admitted turn errors', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + session.prompt.mockRejectedValueOnce( + Object.assign(new Error('model_overloaded'), { + name: 'DaemonHttpError', + status: 500, + _daemonTurnError: true, + }), + ); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await expect( + bridge.prompt('session-1', 'describe', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }), + ).rejects.toThrow('model_overloaded'); + expect(session.removeAttachment).not.toHaveBeenCalled(); + + events.close(); + bridge.stop(); + }); + + it('keeps uploaded channel images when the prompt fails with an unrecognized error', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment.mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + session.prompt.mockRejectedValueOnce(new Error('connection reset')); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + await expect( + bridge.prompt('session-1', 'describe', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }), + ).rejects.toThrow('connection reset'); + expect(session.removeAttachment).not.toHaveBeenCalled(); + + events.close(); + bridge.stop(); + }); + + it('logs failed attachment removals while rolling back uploads', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + session.uploadAttachment + .mockResolvedValueOnce({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }) + .mockRejectedValueOnce(new Error('second upload failed')); + session.removeAttachment.mockRejectedValueOnce(new Error('daemon gone')); + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + let rollbackLog = ''; + try { + await expect( + bridge.prompt('session-1', 'describe', { + images: [ + { data: 'AQID', mimeType: 'image/png' }, + { data: 'BAUG', mimeType: 'image/jpeg' }, + ], + }), + ).rejects.toThrow('second upload failed'); + rollbackLog = stderr.mock.calls.join(''); + } finally { + stderr.mockRestore(); + } + expect(session.removeAttachment).toHaveBeenCalledWith('image.png'); + expect(rollbackLog).toContain('daemon gone'); + expect(rollbackLog).toContain('image.png'); + expect(rollbackLog).toContain('session-1'); + + events.close(); + bridge.stop(); + }); + + it('removes uploaded channel images when cancelled before prompt admission', async () => { + const events = new EventQueue(); + const session = createFakeSession(events); + let resolveUpload: (attachment: Record) => void = () => {}; + const upload = new Promise>((resolve) => { + resolveUpload = resolve; + }); + session.uploadAttachment.mockReturnValueOnce(upload); + let promptAdmissions = 0; + session.prompt.mockImplementation( + async (_req: unknown, signal?: AbortSignal) => { + // Mirrors DaemonSessionClient.prompt: an already-aborted signal is + // rejected before any admission request reaches the daemon. + signal?.throwIfAborted(); + promptAdmissions += 1; + return {}; + }, + ); + const bridge = new DaemonChannelBridge({ + cwd: '/repo', + sessionFactory: vi.fn().mockResolvedValue(session), + sessionAttachments: true, + }); + + await bridge.start(); + await bridge.newSession('/repo'); + + const promptPromise = bridge.prompt('session-1', 'describe', { + images: [{ data: 'AQID', mimeType: 'image/png' }], + }); + await waitFor(() => + expect(session.uploadAttachment).toHaveBeenCalledOnce(), + ); + // Attached after the bridge's own reaction on the upload promise, so + // the cancellation runs once the upload fulfills but before the bridge + // resumes into session.prompt. + void upload.then(() => bridge.cancelSession('session-1')); + resolveUpload({ + type: 'image', + attachmentId: 'image.png', + mimeType: 'image/png', + size: 12, + }); + + await expect(promptPromise).rejects.toMatchObject({ name: 'AbortError' }); + expect(promptAdmissions).toBe(0); + expect(session.removeAttachment).toHaveBeenCalledOnce(); + expect(session.removeAttachment).toHaveBeenCalledWith('image.png'); + + events.close(); + bridge.stop(); + }); + it('forwards a distinct user-facing prompt text in daemon metadata', async () => { const events = new EventQueue(); const session = createFakeSession(events); diff --git a/packages/channels/base/src/DaemonChannelBridge.ts b/packages/channels/base/src/DaemonChannelBridge.ts index d60bd6e7ef..6bef808684 100644 --- a/packages/channels/base/src/DaemonChannelBridge.ts +++ b/packages/channels/base/src/DaemonChannelBridge.ts @@ -7,6 +7,7 @@ import { CHANNEL_PROMPT_AUTHORIZATION_META_KEY, CHANNEL_PROMPT_DISPLAY_TEXT_META_KEY, CHANNEL_PROMPT_META_KEY, + resolvePromptImages, type AvailableCommand, type BridgeSessionInfo, type ChannelAgentBridge, @@ -16,6 +17,7 @@ import { type ToolCallEvent, } from './ChannelAgentBridge.js'; import { readAvailableCommandAltNames } from './AcpBridge.js'; +import { sanitizeLogText } from './sanitize.js'; import { ChannelLoopMcpServer, type JsonRpcMessage, @@ -43,6 +45,13 @@ export interface DaemonChannelSessionClient { }, signal?: AbortSignal, ): Promise<{ stopReason?: string; [key: string]: unknown }>; + uploadAttachment?( + data: Blob, + name: string, + mimeType: string, + signal?: AbortSignal, + ): Promise>; + removeAttachment?(attachmentId: string): Promise; events(opts?: { signal?: AbortSignal; lastEventId?: number; @@ -91,6 +100,12 @@ export interface DaemonChannelBridgeOptions { channelLoopMcpHost?: DaemonChannelLoopMcpHost; deleteSessionData?: (sessionId: string) => Promise; promptAuthorization?: string; + /** + * The daemon advertises the `session_attachments` capability. Daemons + * predating the attachment upload routes receive prompt images inline + * instead, as before the upload path existed. + */ + sessionAttachments?: boolean; } export interface DaemonPermissionRequestEvent { @@ -125,6 +140,80 @@ function getTextContent(content: unknown): string | undefined { return getString(content['text']); } +// Mirrors the daemon attachment store's SUPPORTED_IMAGE_MIME_TYPES +// (packages/acp-bridge/src/sessionAttachments.ts): the store rejects uploads +// outside that set, and channels/base keeps no acp-bridge dependency, so the +// set is repeated here and checked before uploading. +const CHANNEL_IMAGE_EXTENSIONS = ['bmp', 'gif', 'jpeg', 'png', 'webp']; + +// Mirrors the store's SESSION_ATTACHMENT_MAX_ITEM_BYTES and empty-image +// rejection (same file): checked before delivery so one inadmissible image +// degrades by omission instead of failing the whole turn. +const CHANNEL_IMAGE_MAX_UPLOAD_BYTES = 8 * 1024 * 1024; + +// Daemons without `session_attachments` parse the prompt body with +// express.json({ limit: '10mb' }), so the inline fallback keeps the +// aggregate base64 payload below that cap with headroom for the text +// prompt and the JSON envelope. +const CHANNEL_IMAGE_INLINE_MAX_BASE64_BYTES = 8 * 1024 * 1024; + +function channelImageName(mimeType: string, index = 0): string | undefined { + if (!mimeType.startsWith('image/')) { + return undefined; + } + const extension = mimeType.slice('image/'.length); + if (!CHANNEL_IMAGE_EXTENSIONS.includes(extension)) { + return undefined; + } + return index === 0 ? `image.${extension}` : `image-${index + 1}.${extension}`; +} + +function decodeChannelImage( + data: string, + oversizedReason: string, +): { bytes: Buffer } | { skip: string } { + // Valid base64 decodes to at most this many bytes, so an oversized image + // is rejected on length alone instead of allocating a buffer the size + // check would discard. Padding is subtracted only when the input length + // completes a quantum: Node's decoder ignores a stray trailing '=' on + // malformed input, and counting it would undercount the decoded size. + let estimatedBytes = Math.floor((data.length * 3) / 4); + if (data.length % 4 === 0) { + if (data.endsWith('==')) estimatedBytes -= 2; + else if (data.endsWith('=')) estimatedBytes -= 1; + } + if (estimatedBytes > CHANNEL_IMAGE_MAX_UPLOAD_BYTES) { + return { skip: oversizedReason }; + } + const bytes = Buffer.from(data, 'base64'); + if (bytes.byteLength === 0) { + return { skip: 'empty once base64-decoded' }; + } + return { bytes }; +} + +/** + * Structural match for the daemon SDK's definite prompt-admission + * rejections: `DaemonHttpError` from the admission request itself, or + * `DaemonPendingPromptLimitError` raised before any request. channels/base + * keeps no dependency on the SDK, so match by shape; post-admission turn + * errors carry `_daemonTurnError` and must NOT match — by then the daemon + * may already have resolved the uploaded attachments. + */ +function isDefinitePromptAdmissionRejection(error: unknown): boolean { + if (!isRecord(error)) { + return false; + } + if (error['name'] === 'DaemonPendingPromptLimitError') { + return true; + } + return ( + error['name'] === 'DaemonHttpError' && + typeof error['status'] === 'number' && + error['_daemonTurnError'] !== true + ); +} + function getSessionUpdate(data: unknown): Record | undefined { if (!isRecord(data) || !isRecord(data['update'])) { return undefined; @@ -407,41 +496,153 @@ export class DaemonChannelBridge this.on('responseBoundary', clearChunks); this.on('sessionDied', onSessionDied); const turnBarrier = this.createTurnBarrier(sessionId); - - const prompt: Array> = []; - if (options?.imageBase64 && options.imageMimeType) { - prompt.push({ - type: 'image', - data: options.imageBase64, - mimeType: options.imageMimeType, - }); - } - prompt.push({ type: 'text', text }); - // Always presented: the daemon validates it for the channel-turn - // classification as well as the display projection, and channel - // prompts without display text still need the classification. - const promptAuthorization = this.options.promptAuthorization; + const uploadedAttachmentIds: string[] = []; + let rollbackUploadedAttachments = false; + const uploadAttachment = session.uploadAttachment?.bind(session); + const removeAttachment = session.removeAttachment?.bind(session); try { - const result = await session.prompt( - { - prompt, - _meta: { - [CHANNEL_PROMPT_META_KEY]: true, - ...(promptAuthorization - ? { - [CHANNEL_PROMPT_AUTHORIZATION_META_KEY]: promptAuthorization, - } - : {}), - ...(options?.displayText !== undefined - ? { - [CHANNEL_PROMPT_DISPLAY_TEXT_META_KEY]: options.displayText, - } - : {}), + const prompt: Array> = []; + const images = resolvePromptImages(options); + if ( + this.options.sessionAttachments && + uploadAttachment && + removeAttachment + ) { + try { + // Fan the uploads out like the webui's attachment path: names are + // index-disambiguated and prompt order comes from the array order, + // so nothing serializes the uploads themselves. + const uploads = await Promise.allSettled( + images.map(async (image, index) => { + const name = channelImageName(image.mimeType, index); + if (!name) { + // One unrecognized subtype must not fail the whole turn; + // degrade by omission. + process.stderr.write( + `[DaemonChannelBridge] skipped channel image with unsupported MIME type ${sanitizeLogText(image.mimeType, 128)} for session ${sanitizeLogText(sessionId, 128)}\n`, + ); + return undefined; + } + const decoded = decodeChannelImage( + image.data, + 'above the daemon attachment size limit', + ); + if ('skip' in decoded) { + process.stderr.write( + `[DaemonChannelBridge] skipped channel image ${decoded.skip} ${sanitizeLogText(image.mimeType, 128)} for session ${sanitizeLogText(sessionId, 128)}\n`, + ); + return undefined; + } + const attachment = await uploadAttachment( + new Blob([decoded.bytes], { + type: image.mimeType, + }), + name, + image.mimeType, + controller.signal, + ); + const attachmentId = getString(attachment['attachmentId']); + if (attachmentId) uploadedAttachmentIds.push(attachmentId); + return attachment; + }), + ); + const failure = uploads.find( + (upload): upload is PromiseRejectedResult => + upload.status === 'rejected', + ); + if (failure) { + throw failure.reason; + } + for (const upload of uploads) { + if (upload.status === 'fulfilled' && upload.value) { + prompt.push(upload.value); + } + } + } catch (error) { + rollbackUploadedAttachments = true; + throw error; + } + } else { + // Daemons without `session_attachments` take images inline. + let inlineBase64Bytes = 0; + for (const image of images) { + const decoded = decodeChannelImage( + image.data, + 'above the inline image budget', + ); + if ('skip' in decoded) { + process.stderr.write( + `[DaemonChannelBridge] skipped channel image ${decoded.skip} ${sanitizeLogText(image.mimeType, 128)} for session ${sanitizeLogText(sessionId, 128)}\n`, + ); + continue; + } + if ( + inlineBase64Bytes + image.data.length > + CHANNEL_IMAGE_INLINE_MAX_BASE64_BYTES + ) { + process.stderr.write( + `[DaemonChannelBridge] skipped channel image to keep the inline prompt under the daemon body limit ${sanitizeLogText(image.mimeType, 128)} for session ${sanitizeLogText(sessionId, 128)}\n`, + ); + continue; + } + inlineBase64Bytes += image.data.length; + prompt.push({ + type: 'image', + data: image.data, + mimeType: image.mimeType, + }); + } + } + prompt.push({ type: 'text', text }); + if (controller.signal.aborted) { + rollbackUploadedAttachments = true; + controller.signal.throwIfAborted(); + } + // Always presented: the daemon validates it for the channel-turn + // classification as well as the display projection, and channel + // prompts without display text still need the classification. + const promptAuthorization = this.options.promptAuthorization; + + // Aborted after the uploads settled but before admission: the SDK + // rejects an already-aborted signal with a pre-request AbortError that + // isDefinitePromptAdmissionRejection does not match, so the uploads + // would leak. Non-admission is certain at this point; roll back. + if (controller.signal.aborted) { + rollbackUploadedAttachments = true; + throw controller.signal.reason; + } + + let result: { stopReason?: string; [key: string]: unknown }; + try { + result = await session.prompt( + { + prompt, + _meta: { + [CHANNEL_PROMPT_META_KEY]: true, + ...(promptAuthorization + ? { + [CHANNEL_PROMPT_AUTHORIZATION_META_KEY]: + promptAuthorization, + } + : {}), + ...(options?.displayText !== undefined + ? { + [CHANNEL_PROMPT_DISPLAY_TEXT_META_KEY]: options.displayText, + } + : {}), + }, }, - }, - controller.signal, - ); + controller.signal, + ); + } catch (error) { + // Roll back only when the turn was never admitted; once admitted the + // daemon may already have resolved the uploads. + if (isDefinitePromptAdmissionRejection(error)) { + rollbackUploadedAttachments = true; + } + throw error; + } // Prefer turn_complete for deterministic chunk collection (SSE path). // Fall back to one event-loop tick for non-SSE prompt paths (blocking // HTTP, non-202 responses) where turn_complete never arrives. @@ -470,6 +671,24 @@ export class DaemonChannelBridge ) { this.activePromptControllers.delete(sessionId); } + if (rollbackUploadedAttachments && removeAttachment) { + const removals = await Promise.allSettled( + uploadedAttachmentIds.map((attachmentId) => + removeAttachment(attachmentId), + ), + ); + removals.forEach((removal, index) => { + if (removal.status === 'rejected') { + const reason = + removal.reason instanceof Error + ? removal.reason.message + : String(removal.reason); + process.stderr.write( + `[DaemonChannelBridge] failed to remove channel image ${sanitizeLogText(uploadedAttachmentIds[index] ?? '', 128)} for session ${sanitizeLogText(sessionId, 128)} during rollback: ${sanitizeLogText(reason, 256)}\n`, + ); + } + }); + } } } diff --git a/packages/channels/base/src/SessionRouter.test.ts b/packages/channels/base/src/SessionRouter.test.ts index 4900bc864e..82365ba053 100644 --- a/packages/channels/base/src/SessionRouter.test.ts +++ b/packages/channels/base/src/SessionRouter.test.ts @@ -77,6 +77,8 @@ function daemonSession( sessionId, workspaceCwd: '/tmp', prompt: vi.fn().mockResolvedValue({}), + uploadAttachment: vi.fn(), + removeAttachment: vi.fn().mockResolvedValue(true), events: vi.fn(async function* (options?: { signal?: AbortSignal }) { await new Promise((resolve) => { if (options?.signal?.aborted) { diff --git a/packages/channels/dingtalk/src/DingtalkAdapter.test.ts b/packages/channels/dingtalk/src/DingtalkAdapter.test.ts index 8ac555fe7a..8d17b929df 100644 --- a/packages/channels/dingtalk/src/DingtalkAdapter.test.ts +++ b/packages/channels/dingtalk/src/DingtalkAdapter.test.ts @@ -3790,6 +3790,74 @@ describe('DingtalkChannel quoted media', () => { ).onMessage(downstream); } + it('downloads every picture in one richText callback', async () => { + const downloadCodes: string[] = []; + vi.spyOn(globalThis, 'fetch').mockImplementation( + (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + if (url.startsWith('https://oapi.dingtalk.com/gettoken')) { + return Promise.resolve( + new Response( + JSON.stringify({ errcode: 0, access_token: 'app-token' }), + { status: 200 }, + ), + ); + } + if ( + url === 'https://api.dingtalk.com/v1.0/robot/messageFiles/download' + ) { + const request = JSON.parse(String(init?.body)) as { + downloadCode: string; + }; + downloadCodes.push(request.downloadCode); + return Promise.resolve( + new Response( + JSON.stringify({ + downloadUrl: `https://example.com/${request.downloadCode}`, + }), + { status: 200 }, + ), + ); + } + const bytes = url.endsWith('/picture-1') + ? new Uint8Array([1]) + : new Uint8Array([2]); + return Promise.resolve( + new Response(bytes, { + status: 200, + headers: { 'content-type': 'image/png' }, + }), + ); + }, + ); + const channel = createChannel(); + + sendDirectMedia(channel, 'richText', { + richText: [ + { type: 'picture', downloadCode: 'picture-1' }, + { type: 'picture', downloadCode: 'picture-2' }, + ], + }); + + await vi.waitFor(() => { + expect(channel.handleInbound).toHaveBeenCalledOnce(); + }); + const envelope = vi.mocked(channel.handleInbound).mock.calls[0]![0]; + expect(downloadCodes).toEqual(['picture-1', 'picture-2']); + expect(envelope.attachments).toEqual([ + { + type: 'image', + data: Buffer.from([1]).toString('base64'), + mimeType: 'image/png', + }, + { + type: 'image', + data: Buffer.from([2]).toString('base64'), + mimeType: 'image/png', + }, + ]); + }); + it('downloads a replied picture and attaches it to the prompt', async () => { const downloadCodes = mockMediaDownload( 'image/png', @@ -4000,11 +4068,7 @@ describe('DingtalkChannel quoted media', () => { if (filePath) tempDirs.add(dirname(filePath)); }); - // R4-1: ChannelBase resolves a single inline image per envelope (the first - // data-only image attachment fills imageBase64) and silently drops every - // later data-only attachment, so the quoted image must be file-backed when - // the message's own image already occupies the slot. - it('file-backs a quoted image when the message already carries its own image', async () => { + it('keeps a quoted image data-backed when the message carries its own image', async () => { const downloadCodes = mockMediaDownload( 'image/png', new Uint8Array([1, 2, 3]), @@ -4054,26 +4118,18 @@ describe('DingtalkChannel quoted media', () => { referencedText: '[image]', }); expect(envelope.attachments).toHaveLength(2); - // The own image keeps the single inline slot ChannelBase resolves. - expect(envelope.attachments?.[0]).toEqual({ - type: 'image', - data: Buffer.from([1, 2, 3]).toString('base64'), - mimeType: 'image/png', - }); - // The quoted image must not be a second data-only attachment — that shape - // is silently dropped by ChannelBase's single-image resolution. - const quotedAttachment = envelope.attachments?.[1]; - expect(quotedAttachment).toMatchObject({ - type: 'image', - mimeType: 'image/png', - }); - expect(quotedAttachment).not.toHaveProperty('data'); - const filePath = quotedAttachment?.filePath; - if (filePath) tempDirs.add(dirname(filePath)); - expect(filePath).toBeTruthy(); - expect(existsSync(filePath!)).toBe(true); - expect(readFileSync(filePath!)).toEqual(Buffer.from([1, 2, 3])); - expect(quotedAttachment?.fileName).toMatch(/^dingtalk_image_\d+\.png$/); + expect(envelope.attachments).toEqual([ + { + type: 'image', + data: Buffer.from([1, 2, 3]).toString('base64'), + mimeType: 'image/png', + }, + { + type: 'image', + data: Buffer.from([1, 2, 3]).toString('base64'), + mimeType: 'image/png', + }, + ]); }); it('cleans the generated placeholder for a direct file message', async () => { diff --git a/packages/channels/dingtalk/src/DingtalkAdapter.ts b/packages/channels/dingtalk/src/DingtalkAdapter.ts index 9204dff500..134519388d 100644 --- a/packages/channels/dingtalk/src/DingtalkAdapter.ts +++ b/packages/channels/dingtalk/src/DingtalkAdapter.ts @@ -2116,16 +2116,7 @@ export class DingtalkChannel extends ChannelBase { const media = await downloadMedia(downloadCode, robotCode, token); if (!media) return; - // ChannelBase fills a single imageBase64 slot from the FIRST data-only - // image attachment and silently drops every later one, so an image - // arriving after the slot is taken (e.g. a quoted picture alongside the - // message's own picture) falls through to the file-backed path — the - // `saved to:` prompt line is what keeps it reachable for the agent. - const inlineImageSlotFree = !(envelope.attachments || []).some( - (attachment) => attachment.type === 'image' && attachment.data, - ); - - if (mediaType === 'image' && inlineImageSlotFree) { + if (mediaType === 'image') { const mimeType = media.mimeType.startsWith('image/') ? media.mimeType : 'image/jpeg'; @@ -2341,15 +2332,17 @@ export class DingtalkChannel extends ChannelBase { } const processMessage = async () => { - // Download media if present (first downloadCode only for images) + // Download media in callback order. if (content.downloadCodes.length > 0 && content.mediaType) { - await this.attachMedia( - envelope, - content.downloadCodes[0]!, - content.mediaType, - content.fileName, - content.placeholder, - ); + for (const downloadCode of content.downloadCodes) { + await this.attachMedia( + envelope, + downloadCode, + content.mediaType, + content.fileName, + content.placeholder, + ); + } } if (quoted.media) { await this.attachMedia( diff --git a/packages/cli/src/commands/channel/daemon-worker.test.ts b/packages/cli/src/commands/channel/daemon-worker.test.ts index 16056fd479..2d5465f812 100644 --- a/packages/cli/src/commands/channel/daemon-worker.test.ts +++ b/packages/cli/src/commands/channel/daemon-worker.test.ts @@ -1319,6 +1319,43 @@ describe('runChannelDaemonWorker', () => { expect(bridgeFacade.shellCommand).toBeTypeOf('function'); }); + it('enables attachment uploads only when capabilities include session_attachments', async () => { + const sdk = createSdk(); + sdk.client.capabilities.mockResolvedValueOnce({ + v: 1, + mode: 'http-bridge', + features: ['session_attachments'], + modelServices: [], + workspaceCwd: '/workspace', + }); + + await runChannelDaemonWorker({ + daemonUrl: 'http://127.0.0.1:4170', + workspace: '/workspace', + selection: { mode: 'names', names: ['telegram'] }, + loadDaemonSdk: async () => sdk, + }); + + expect(mockDaemonChannelBridge).toHaveBeenCalledWith( + expect.objectContaining({ sessionAttachments: true }), + ); + }); + + it('keeps attachment uploads off for daemons without session_attachments', async () => { + const sdk = createSdk(); + + await runChannelDaemonWorker({ + daemonUrl: 'http://127.0.0.1:4170', + workspace: '/workspace', + selection: { mode: 'names', names: ['telegram'] }, + loadDaemonSdk: async () => sdk, + }); + + expect(mockDaemonChannelBridge).toHaveBeenCalledWith( + expect.objectContaining({ sessionAttachments: false }), + ); + }); + it('fails fast for unknown selected channel names', async () => { const sdk = createSdk(); diff --git a/packages/cli/src/commands/channel/daemon-worker.ts b/packages/cli/src/commands/channel/daemon-worker.ts index 1d3fc84494..cf03aa421c 100644 --- a/packages/cli/src/commands/channel/daemon-worker.ts +++ b/packages/cli/src/commands/channel/daemon-worker.ts @@ -95,6 +95,7 @@ import { } from './loop-runtime.js'; const SESSION_SHELL_COMMAND_FEATURE = 'session_shell_command'; +const SESSION_ATTACHMENTS_FEATURE = 'session_attachments'; const MAX_ACTIVE_WEBHOOK_TASKS = 16; const WORKER_SHUTDOWN_DRAIN_MS = 10_000; @@ -492,6 +493,9 @@ export async function runChannelDaemonWorker( DaemonSessionClient: sdk.DaemonSessionClient, clientId: `qwen-channel-worker:${process.pid}`, }), + sessionAttachments: capabilities.features.includes( + SESSION_ATTACHMENTS_FEATURE, + ), ...(opts.promptAuthorization ? { promptAuthorization: opts.promptAuthorization } : {}),