diff --git a/docs/design/attachment-path-context.md b/docs/design/attachment-path-context.md new file mode 100644 index 0000000000..d9c861eada --- /dev/null +++ b/docs/design/attachment-path-context.md @@ -0,0 +1,81 @@ +# Attachment paths in model context + +[English](attachment-path-context.md) | [简体中文](attachment-path-context.zh-CN.md) + +## Problem and scope + +Uploaded files are stored on disk, but dispatch only supplies their contents and +an `attachment:///` identifier (images only carry bytes and MIME type). A model +that needs the original file for a script must discover its location, sometimes +using memory. This fix supplies the location independently of memory. It does +not add Excel parsing, change browser previews, or add a new tool. + +## Design + +The session attachment store adds a model-context string to the existing +content block's `_meta[DAEMON_ATTACHMENT_CONTEXT_META_KEY]`. It contains the +stored name, attachment URI, MIME type, and absolute path as JSON, plus a short +instruction to use that path with tools running on the daemon host. Names and +paths are JSON encoded; attachment contents remain user data. Images retain +their inline bytes and text files retain their contents. + +`Session` adds this context to the model's reference parts, including the +image-only path, without merging it into the user's text. There remains exactly +one content block per attachment reference. User display text, title input, and +mid-turn attachment-reference persistence keep their existing behavior. + +Vision Bridge derives its focus hint from the resolved text with attachment +path metadata excluded. This keeps its 2,000-character hint budget available +for the user's question, expanded commands, and audio transcription. The main +model still receives the path context. Normal, custom-command, and mid-turn +delivery use the same filtering. + +Obtain the path from the same successful read that supplies the bytes. Do not +reconstruct the default directory: a configured root may fall back to legacy +storage. Keep public attachment download responses unchanged. Generate metadata +when a stored reference is dispatched, including references after a session +restore, instead of persisting a separate path mapping. + +Normal prompt dispatch and mid-turn queue draining already share the resolver. +Both retain the new metadata on surviving attachment blocks during degradation. +Duplicate references share one read and encoding, and failed reads are still +evicted from the dispatch memo. + +## Constraints + +Paths belong to the daemon/ACP execution filesystem, not the browser's machine. +The daemon launches ACP children locally, but SSH or Managed sessions may +delegate tools to another filesystem. Metadata explicitly identifies the daemon +host and does not claim remote tools can access its files. ACP currently rejects +the separate tool execution sandbox. This change does not add remote filesystem +transport or sandbox mounts. Existing tool permissions still apply. A missing +attachment keeps the existing failure/degradation behavior; never search another +session for a replacement. Raw inline content without a stored attachment +reference has no new path metadata. + +Absolute paths are sent to the configured model provider as prompt content. +The default path can reveal the daemon user's home-directory name; a custom +root can reveal its directory hierarchy. Paths are not anonymized. A private +attachment alias resolved by tools would require a separate design. + +Providing a path does not add a parser for unsupported binary formats. A model +can use an available parser through shell tools. File removal after dispatch can +still invalidate a previously supplied path. + +## Validation and acceptance + +- In a fresh session with memory disabled, image, text, and binary attachments + include absolute paths in the final model request. Reading those paths returns + the original bytes; image content remains available to a vision model. +- Configured roots, fallback roots, restored stores, duplicate filenames, and + names containing spaces or Unicode report the correct stored file. +- Unavailable attachments report failure or degrade as before; surviving + attachments keep both metadata and content during mid-turn delivery. +- Existing content, memoization, download, and session isolation tests pass. +- Normal and image-only mid-turn messages retain their original display text + and attachment references; paths appear only in model context. +- Long attachment metadata does not consume the Vision Bridge focus-hint + budget or displace the user's question; the main model retains the paths. +- Build, typecheck, targeted unit tests, and an isolated Store → Session → OpenAI + request-conversion probe pass. The probe does not contact a model or load + personal memory; it verifies request construction and file access. diff --git a/docs/design/attachment-path-context.zh-CN.md b/docs/design/attachment-path-context.zh-CN.md new file mode 100644 index 0000000000..418db56bd4 --- /dev/null +++ b/docs/design/attachment-path-context.zh-CN.md @@ -0,0 +1,66 @@ +# 模型上下文中的附件路径 + +[English](attachment-path-context.md) | [简体中文](attachment-path-context.zh-CN.md) + +## 问题与范围 + +上传文件已保存在磁盘上,但分发时只提供内容和 `attachment:///` 标识 +(图片仅携带字节和 MIME 类型)。模型需要通过脚本处理原文件时,必须自行 +寻找位置,有时依赖记忆。本修复直接提供位置,不依赖记忆;不增加 Excel +解析、不修改浏览器预览,也不新增工具。 + +## 设计 + +会话附件存储在原内容块的 `_meta[DAEMON_ATTACHMENT_CONTEXT_META_KEY]` 中 +添加模型上下文字符串,包含以 JSON 编码的存储名称、附件 URI、MIME 类型和 +绝对路径,并说明使用该路径调用 daemon 主机上的工具。名称和路径使用 JSON +编码;附件内容仍属于用户数据。图片保留内联字节,文本文件保留原有内容。 + +`Session` 将该上下文加入模型的引用内容,包括仅有图片的消息,不合并到用户 +正文中。每个附件引用仍对应一个内容块,用户展示文本、标题输入以及回合中 +附件引用的持久化行为保持不变。 + +Vision Bridge 从解析后的文本生成解读提示时,排除附件路径元数据,避免其 +占用 2,000 字符预算而挤掉用户问题、展开后的命令或语音转录。主模型仍然 +接收路径上下文。普通发送、自定义命令和回合中投递共用这一过滤逻辑。 + +路径取自提供文件字节的同一次成功读取。不推算默认目录:自定义根目录可能 +回退到旧存储。对外附件下载响应保持不变。存储引用分发时生成元数据,包括 +恢复会话后分发的引用,不单独持久化一份路径映射。 + +正常提示分发和回合中消息队列消费已经共用解析器,降级时两者都保留有效 +附件内容块中的元数据。重复引用继续共用一次读取和编码,读取失败后仍从 +分发缓存中移除。 + +## 约束 + +路径属于 daemon/ACP 执行文件系统,不属于浏览器所在机器。受支持的 daemon +在本地启动 ACP 子进程,但 SSH 或 Managed 会话可能把工具委派到另一个文件 +系统。元数据明确标注 daemon 主机,不声称远程工具可以访问该文件。ACP 当前 +拒绝独立的工具执行沙箱。本修改不增加远程文件系统传输或沙箱挂载。现有 +工具权限继续生效。附件缺失时保留现有失败或降级行为,不能到其他会话寻找 +替代文件。没有存储附件引用的原始内联内容不 +添加路径元数据。 + +绝对路径会作为提示内容发送给配置的模型服务商。默认路径可能暴露 daemon +用户的主目录名称,自定义根目录可能暴露其目录层级;当前方案不做路径匿名化。 +若要使用由工具解析的私有附件别名,需要另行设计。 + +提供路径不会增加对不支持的二进制格式的解析器。模型可以通过 shell 工具 +调用可用的解析器。分发后删除文件仍可能使此前提供的路径失效。 + +## 验证与验收 + +- 在禁用记忆的新会话中,图片、文本和二进制附件的绝对路径出现在最终模型 + 请求中;通过这些路径读取到的字节与原文件一致;视觉模型仍可接收图片内容。 +- 自定义根目录、备用根目录、恢复后的存储、重名文件以及包含空格或 Unicode + 的名称都指向正确的存储文件。 +- 不可用附件继续按原有语义报错或降级;回合中投递时,有效附件同时保留 + 元数据和内容。 +- 现有内容、读取缓存、下载和会话隔离测试通过。 +- 正常消息和回合中仅含图片的消息保留原展示文本及附件引用;路径仅进入模型 + 上下文。 +- 较长的附件元数据不会占用 Vision Bridge 的解读提示预算或挤掉用户问题; + 主模型仍然保留路径。 +- 构建、类型检查、定向单元测试以及隔离的 Store → Session → OpenAI 请求 + 转换探针通过。探针不调用模型、不加载个人记忆,验证请求构造和文件访问。 diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index 7c07677dfe..d6ab95a27d 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -17642,7 +17642,15 @@ describe('createAcpSessionBridge', () => { ); expect(prompts[0]?.prompt).toEqual([ - { type: 'image', data: 'AQID', mimeType: 'image/png' }, + { + type: 'image', + data: 'AQID', + mimeType: 'image/png', + _meta: { + 'qwen.daemon.attachmentContext': + expect.stringContaining('"absolutePath":'), + }, + }, ]); expect(prompts[0]?._meta?.['qwen.daemon.attachmentReferences']).toEqual([ reference, @@ -17772,6 +17780,10 @@ describe('createAcpSessionBridge', () => { expect(prompts[0]?.prompt).toEqual([ { type: 'resource', + _meta: { + 'qwen.daemon.attachmentContext': + expect.stringContaining('"absolutePath":'), + }, resource: { uri: 'attachment:///notes.txt', mimeType: 'text/plain', @@ -17780,6 +17792,10 @@ describe('createAcpSessionBridge', () => { }, { type: 'resource', + _meta: { + 'qwen.daemon.attachmentContext': + expect.stringContaining('"absolutePath":'), + }, resource: { uri: 'attachment:///report.pdf', mimeType: 'application/pdf', @@ -39094,7 +39110,15 @@ describe('createAcpSessionBridge — mid-turn message queue (enqueueMidTurnMessa type: 'text', text: 'two images\n[Attachment is no longer available]', }, - { type: 'image', data: 'AQI=', mimeType: 'image/png' }, + { + type: 'image', + data: 'AQI=', + mimeType: 'image/png', + _meta: { + 'qwen.daemon.attachmentContext': + expect.stringContaining('"absolutePath":'), + }, + }, ]); releases[1]!(); await vi.waitFor(() => @@ -39174,7 +39198,15 @@ describe('createAcpSessionBridge — mid-turn message queue (enqueueMidTurnMessa type: 'text', text: 'm2\n[Attachment is no longer available]', }, - { type: 'image', data: 'AQI=', mimeType: 'image/png' }, + { + type: 'image', + data: 'AQI=', + mimeType: 'image/png', + _meta: { + 'qwen.daemon.attachmentContext': + expect.stringContaining('"absolutePath":'), + }, + }, ]); releases[2]!(); await vi.waitFor(() => diff --git a/packages/acp-bridge/src/bridgeClient.test.ts b/packages/acp-bridge/src/bridgeClient.test.ts index 6669ac3045..ba4e07e123 100644 --- a/packages/acp-bridge/src/bridgeClient.test.ts +++ b/packages/acp-bridge/src/bridgeClient.test.ts @@ -4155,12 +4155,13 @@ describe('BridgeClient — mid-turn queue drain (craft/drainMidTurnQueue)', () = it('degrades a attachmentId reused across drained messages after its first use', async () => { const publish = vi.fn().mockReturnValue(true); const media = new SessionAttachmentStore(); + const read = vi.spyOn(fsp, 'readFile'); try { const reference = await media.putAttachment( Uint8Array.of(1, 2, 3), 'image/png', ); - const read = vi.spyOn(media, 'read'); + read.mockClear(); const entry = { sessionId: 'sess:shared-media', midTurnMessageQueue: [ @@ -4217,6 +4218,7 @@ describe('BridgeClient — mid-turn queue drain (craft/drainMidTurnQueue)', () = // serialized, so one stored blob cannot amplify the drain response. expect(read).toHaveBeenCalledTimes(1); } finally { + read.mockRestore(); await media.close(); } }); diff --git a/packages/acp-bridge/src/bridgeTypes.ts b/packages/acp-bridge/src/bridgeTypes.ts index e68d2a0323..ecd892edbf 100644 --- a/packages/acp-bridge/src/bridgeTypes.ts +++ b/packages/acp-bridge/src/bridgeTypes.ts @@ -1418,6 +1418,8 @@ export const DAEMON_PASSIVE_MANAGED_RUNTIME_RECOVERY_META_KEY = 'qwen.daemon.passiveManagedRuntimeRecovery'; export const DAEMON_ATTACHMENT_REFERENCES_META_KEY = 'qwen.daemon.attachmentReferences'; +export const DAEMON_ATTACHMENT_CONTEXT_META_KEY = + 'qwen.daemon.attachmentContext'; export const MAX_TRUSTED_MODEL_PROMPT_CHARS = 64 * 1024; export function isValidTrustedModelPrompt(value: unknown): value is string { diff --git a/packages/acp-bridge/src/sessionAttachments.test.ts b/packages/acp-bridge/src/sessionAttachments.test.ts index 7ebefe2382..66580c036e 100644 --- a/packages/acp-bridge/src/sessionAttachments.test.ts +++ b/packages/acp-bridge/src/sessionAttachments.test.ts @@ -8,6 +8,7 @@ import { promises as fs, statSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import * as path from 'node:path'; import type { ContentBlock } from '@agentclientprotocol/sdk'; +import { DAEMON_ATTACHMENT_CONTEXT_META_KEY } from './bridgeTypes.js'; import { describe, expect, it, vi } from 'vitest'; import { SESSION_ATTACHMENT_UNAVAILABLE_TEXT, @@ -28,7 +29,102 @@ vi.mock('node:fs', async (importOriginal) => { }; }); +const attachmentMeta = { + [DAEMON_ATTACHMENT_CONTEXT_META_KEY]: + expect.stringContaining('"absolutePath":'), +}; + describe('SessionAttachmentStore', () => { + it.each([ + ['notes 空格.txt', 'text/plain', Buffer.from('attachment text')], + ['table.xlsx', 'application/octet-stream', Buffer.from([0, 255, 1])], + ['image.png', 'image/png', Buffer.from([1, 2, 3])], + ])( + 'supplies the actual stored path for %s after restore', + async (name, mimeType, data) => { + const root = await fs.mkdtemp(path.join(tmpdir(), 'qwen-path-context-')); + const sessionId = 'restored'; + const store = new SessionAttachmentStore(root, sessionId); + const restored = new SessionAttachmentStore(root, sessionId); + try { + const first = await store.putAttachment(data, mimeType, name); + const duplicate = await store.putAttachment(data, mimeType, name); + expect(duplicate.attachmentId).not.toBe(first.attachmentId); + await store.close(); + for (const reference of [first, duplicate]) { + const [content] = await restored.resolveContent([reference]); + const context = content?._meta?.[DAEMON_ATTACHMENT_CONTEXT_META_KEY]; + if (typeof context !== 'string') + throw new Error('Missing attachment context'); + const absolutePath = path.join( + root, + `session-${sessionId}`, + reference.attachmentId, + ); + expect(JSON.parse(context.split('\n')[1]!)).toEqual({ + name: reference.attachmentId, + uri: `attachment:///${encodeURIComponent(reference.attachmentId)}`, + mimeType, + absolutePath, + }); + expect(path.isAbsolute(absolutePath)).toBe(true); + expect(await fs.readFile(absolutePath)).toEqual(data); + expect(content?.type).toBe(reference.type); + } + const otherSession = new SessionAttachmentStore(root, 'other'); + await expect(otherSession.resolveContent([first])).rejects.toThrow( + 'Unknown or unavailable', + ); + await otherSession.close(); + } finally { + await store.close(); + await restored.close(); + await fs.rm(root, { recursive: true, force: true }); + } + }, + ); + + it.each(['missing', 'unreadable'] as const)( + 'reports the fallback path when the primary is %s', + async (primaryState) => { + const root = await fs.mkdtemp( + path.join(tmpdir(), 'qwen-fallback-context-'), + ); + const primary = path.join(root, 'primary'); + const fallback = path.join(root, 'legacy'); + const oldStore = new SessionAttachmentStore(fallback, 'test'); + const store = new SessionAttachmentStore(primary, 'test', fallback); + const readFile = vi.spyOn(fs, 'readFile'); + try { + const reference = await oldStore.putAttachment( + Buffer.from('legacy'), + 'text/plain', + 'notes.txt', + ); + await oldStore.close(); + if (primaryState === 'unreadable') { + readFile.mockRejectedValueOnce( + Object.assign(new Error('unreadable'), { code: 'EACCES' }), + ); + } + const [content] = await store.resolveContent([reference]); + const context = content?._meta?.[DAEMON_ATTACHMENT_CONTEXT_META_KEY]; + if (typeof context !== 'string') + throw new Error('Missing attachment context'); + const { absolutePath } = JSON.parse(context.split('\n')[1]!); + expect(absolutePath).toBe( + path.join(fallback, 'session-test', reference.attachmentId), + ); + expect(await fs.readFile(absolutePath, 'utf8')).toBe('legacy'); + } finally { + readFile.mockRestore(); + await oldStore.close(); + await store.close(); + await fs.rm(root, { recursive: true, force: true }); + } + }, + ); + it.each(['close', 'delete'] as const)( 'invalidates staged uploads on %s', async (operation) => { @@ -405,7 +501,12 @@ describe('SessionAttachmentStore', () => { size: 3, }); expect(await store.resolveContent([reference])).toEqual([ - { type: 'image', data: 'AQID', mimeType: 'image/png' }, + { + type: 'image', + data: 'AQID', + mimeType: 'image/png', + _meta: attachmentMeta, + }, ]); expect(await store.read(reference.attachmentId)).toEqual({ data: Buffer.from([1, 2, 3]), @@ -436,6 +537,7 @@ describe('SessionAttachmentStore', () => { expect(await store.resolveContent([reference])).toEqual([ { type: 'resource', + _meta: attachmentMeta, resource: { uri: 'attachment:///notes.txt', mimeType: 'text/plain', @@ -467,6 +569,7 @@ describe('SessionAttachmentStore', () => { expect(await store.resolveContent([reference])).toEqual([ { type: 'resource', + _meta: attachmentMeta, resource: { uri: 'attachment:///report.pdf', mimeType: 'application/pdf', @@ -495,6 +598,7 @@ describe('SessionAttachmentStore', () => { expect(await store.resolveContent([reference])).toEqual([ { type: 'resource', + _meta: attachmentMeta, resource: { uri: `attachment:///${name}`, mimeType, @@ -519,6 +623,7 @@ describe('SessionAttachmentStore', () => { expect(await store.resolveContent([reference])).toEqual([ { type: 'resource', + _meta: attachmentMeta, resource: { uri: 'attachment:///payload.unknown', mimeType: 'application/octet-stream', @@ -548,6 +653,7 @@ describe('SessionAttachmentStore', () => { expect(await store.resolveContent([reference])).toEqual([ { type: 'resource', + _meta: attachmentMeta, resource: { uri: 'attachment:///diagram.svg', mimeType: 'image/svg+xml', @@ -580,9 +686,24 @@ describe('SessionAttachmentStore', () => { ]); expect(resolved).toEqual([ - { type: 'image', data: 'AQID', mimeType: 'image/png' }, - { type: 'image', data: 'AQID', mimeType: 'image/png' }, - { type: 'image', data: 'AQID', mimeType: 'image/png' }, + { + type: 'image', + data: 'AQID', + mimeType: 'image/png', + _meta: attachmentMeta, + }, + { + type: 'image', + data: 'AQID', + mimeType: 'image/png', + _meta: attachmentMeta, + }, + { + type: 'image', + data: 'AQID', + mimeType: 'image/png', + _meta: attachmentMeta, + }, ]); expect(readFile).toHaveBeenCalledTimes(1); } finally { @@ -603,6 +724,7 @@ describe('SessionAttachmentStore', () => { const memo = new Map>(); const block = { + _meta: attachmentMeta, type: 'image', data: 'AQID', mimeType: 'image/png', @@ -1515,6 +1637,7 @@ describe('SessionAttachmentStore', () => { await expect(restored.resolveContent([typescript])).resolves.toEqual([ { type: 'resource', + _meta: attachmentMeta, resource: { uri: 'attachment:///example.ts', mimeType: 'text/plain', @@ -1666,7 +1789,12 @@ describe('SessionAttachmentStore', () => { // The failed entry must not stay cached: the next resolution re-reads // from disk and succeeds. await expect(store.resolveContent([reference], memo)).resolves.toEqual([ - { type: 'image', data: 'CQk=', mimeType: 'image/png' }, + { + type: 'image', + data: 'CQk=', + mimeType: 'image/png', + _meta: attachmentMeta, + }, ]); expect(readFile).toHaveBeenCalledTimes(2); } finally { @@ -1689,7 +1817,12 @@ describe('SessionAttachmentStore', () => { expect(result.retainedBlocks).toEqual([text, live]); expect(result.resolvedBlocks).toEqual([ text, - { type: 'image', data: 'AQI=', mimeType: 'image/png' }, + { + type: 'image', + data: 'AQI=', + mimeType: 'image/png', + _meta: attachmentMeta, + }, ]); } finally { await store.close(); diff --git a/packages/acp-bridge/src/sessionAttachments.ts b/packages/acp-bridge/src/sessionAttachments.ts index 2ec49e9474..b200781ba9 100644 --- a/packages/acp-bridge/src/sessionAttachments.ts +++ b/packages/acp-bridge/src/sessionAttachments.ts @@ -8,6 +8,7 @@ import { constants as fsConstants, promises as fs, statSync } from 'node:fs'; import { tmpdir } from 'node:os'; import * as path from 'node:path'; import type { ContentBlock } from '@agentclientprotocol/sdk'; +import { DAEMON_ATTACHMENT_CONTEXT_META_KEY } from './bridgeTypes.js'; import { getSpecificMimeType } from '@qwen-code/qwen-code-core'; import { SessionAttachmentUploads, @@ -38,6 +39,12 @@ interface DurableAttachmentDirectory { inodeVerifiable: boolean; } +interface StoredAttachment { + data: Buffer; + mimeType: string; + absolutePath: string; +} + function hasVerifiableInode(ino: number): boolean { return Number.isSafeInteger(ino) && ino > 0; } @@ -742,6 +749,14 @@ export class SessionAttachmentStore { async read( attachmentId: string, ): Promise<{ data: Buffer; mimeType: string } | undefined> { + const attachment = await this.readStored(attachmentId); + if (!attachment) return undefined; + return { data: attachment.data, mimeType: attachment.mimeType }; + } + + private async readStored( + attachmentId: string, + ): Promise { const releasePublication = await this.acquireReadPublication(); try { return await this.readPublished(attachmentId); @@ -752,11 +767,11 @@ export class SessionAttachmentStore { private async readPublished( attachmentId: string, - ): Promise<{ data: Buffer; mimeType: string } | undefined> { + ): Promise { const name = safeAttachmentName(attachmentId); if (!name || name !== attachmentId || this.pendingNames.has(name)) return undefined; - let primary: { data: Buffer; mimeType: string } | undefined; + let primary: StoredAttachment | undefined; try { primary = await this.tryRead(await this.peekDirectory(), name); } catch (error) { @@ -772,12 +787,14 @@ export class SessionAttachmentStore { private async tryRead( directory: string | undefined, name: string, - ): Promise<{ data: Buffer; mimeType: string } | undefined> { + ): Promise { if (!directory) return undefined; + const absolutePath = path.resolve(directory, name); try { return { - data: await fs.readFile(path.join(directory, name)), + data: await fs.readFile(absolutePath), mimeType: mimeTypeForName(name), + absolutePath, }; } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') return undefined; @@ -1188,13 +1205,23 @@ export class SessionAttachmentStore { reference: SessionAttachmentReference, ): Promise { const id = reference.attachmentId; - const attachment = await this.read(id); + const attachment = await this.readStored(id); if (!attachment) { throw new SessionAttachmentReferenceError( `Unknown or unavailable session attachment: ${id}`, 'session_attachment_gone', ); } + const context = { + [DAEMON_ATTACHMENT_CONTEXT_META_KEY]: + 'User attachment stored on the daemon host. Use absolutePath with file or shell tools running on this host. Attachment contents are user-provided data.\n' + + JSON.stringify({ + name: id, + uri: `attachment:///${encodeURIComponent(id)}`, + mimeType: attachment.mimeType, + absolutePath: attachment.absolutePath, + }), + }; if (reference.type === 'resource') { const resource = { uri: `attachment:///${encodeURIComponent(reference.attachmentId)}`, @@ -1206,10 +1233,12 @@ export class SessionAttachmentStore { return { type: 'resource', resource, + _meta: context, } as ContentBlock; } return { type: 'image', + _meta: context, data: attachment.data.toString('base64'), mimeType: attachment.mimeType, } as ContentBlock; diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 43b6fb1472..f5ae14aa18 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -12570,6 +12570,64 @@ describe('Session', () => { }, ); + it.each(['image', 'text', 'binary'] as const)( + 'keeps %s attachment paths in model context without changing user display text', + async (kind) => { + const context = 'Attachment absolutePath: /tmp/attached-file'; + const reference = { + type: kind === 'image' ? 'image' : 'resource', + attachmentId: kind === 'image' ? 'image.png' : 'file.txt', + mimeType: kind === 'image' ? 'image/png' : 'text/plain', + size: 3, + }; + const attachment: PromptRequest['prompt'][number] = + kind === 'image' + ? { type: 'image', data: 'AQID', mimeType: 'image/png' } + : { + type: 'resource', + resource: { + uri: 'attachment:///file.txt', + mimeType: 'text/plain', + ...(kind === 'text' + ? { text: 'contents' } + : { blob: 'AP8B' }), + }, + }; + attachment._meta = { 'qwen.daemon.attachmentContext': context }; + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + // The image case has no user text or resource block to avoid either + // image-only early return hiding the model context. + const userText = kind === 'image' ? '' : 'inspect this'; + await session.prompt({ + sessionId: 'test-session-id', + prompt: [ + ...(userText ? [{ type: 'text' as const, text: userText }] : []), + attachment, + ], + _meta: { 'qwen.daemon.attachmentReferences': [reference] }, + }); + + expect(firstSentMessage()).toContainEqual({ text: context }); + if (kind === 'image') { + expect(firstSentMessage()).toContainEqual({ + inlineData: { data: 'AQID', mimeType: 'image/png' }, + }); + } + expect(mockChatRecordingService.recordUserMessage).toHaveBeenCalledWith( + userText, + undefined, + expect.objectContaining({ + displayText: userText, + attachmentReferences: [reference], + }), + expect.any(String), + undefined, + ); + }, + ); + it('records daemon attachment references for transcript replay', async () => { const imageReference = { type: 'image' as const, @@ -17965,6 +18023,65 @@ describe('Session', () => { expect(sent.some((part) => 'inlineData' in part)).toBe(false); }); + it.each(['normal', 'custom command', 'image-only'] as const)( + 'keeps attachment paths outside the vision bridge focus for %s prompts', + async (kind) => { + const question = kind === 'image-only' ? '' : 'Read the serial number.'; + const context = `Attachment absolutePath: /tmp/${'a'.repeat(2100)}`; + mockConfig.getEffectiveInputModalities = vi.fn().mockReturnValue({}); + mockConfig.getDefaultVisionBridgeModel = vi.fn().mockReturnValue({ + id: 'qwen3.7-plus', + }); + runVisionBridgeSpy.mockResolvedValue({ + applied: false, + status: 'skipped', + convertedCount: 0, + omittedCount: 0, + }); + mockChat.sendMessageStream = vi + .fn() + .mockResolvedValue(createEmptyStream()); + if (kind === 'custom command') { + vi.mocked( + nonInteractiveCliCommands.handleSlashCommand, + ).mockResolvedValueOnce({ + type: 'submit_prompt', + content: [{ text: question }], + resolvedCommand: { name: 'inspect', kind: CommandKind.FILE }, + }); + } + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [ + { + type: 'text', + text: kind === 'custom command' ? '/inspect' : question, + }, + { + type: 'image', + mimeType: 'image/png', + data: 'iVBORw0KGgo=', + _meta: { 'qwen.daemon.attachmentContext': context }, + }, + ], + }); + + expect(runVisionBridgeSpy).toHaveBeenCalledOnce(); + expect(runVisionBridgeSpy).toHaveBeenCalledWith( + expect.objectContaining({ + intentText: question, + parts: expect.arrayContaining([ + { text: context }, + { inlineData: { mimeType: 'image/png', data: 'iVBORw0KGgo=' } }, + ]), + }), + ); + expect(textParts(firstSentMessage())).toContain(context); + if (question) expect(textParts(firstSentMessage())).toContain(question); + }, + ); + it('routes an agent-capable image prompt for that ACP prompt only', async () => { const runtimeView = { contentGenerator: {}, @@ -23272,6 +23389,10 @@ describe('Session', () => { type: 'image', mimeType: 'image/png', data: 'cHVyZS1pbWFnZQ==', + _meta: { + 'qwen.daemon.attachmentContext': + 'Attachment absolutePath: /tmp/image-2.png', + }, }, ], displayText: '', @@ -23375,6 +23496,9 @@ describe('Session', () => { expect(secondCall?.[1].message).toEqual( expect.arrayContaining(midTurnParts), ); + expect(secondCall?.[1].message).toContainEqual({ + text: 'Attachment absolutePath: /tmp/image-2.png', + }); expect(runVisionBridgeSpy).not.toHaveBeenCalled(); expect(secondCall?.[1].message).not.toEqual( expect.arrayContaining([ @@ -23482,7 +23606,10 @@ describe('Session', () => { expect( mockChatRecordingService.recordMidTurnUserMessage, ).toHaveBeenCalledWith( - [{ text: '\n[User message received during tool execution]: ' }], + [ + { text: '\n[User message received during tool execution]: ' }, + { text: 'Attachment absolutePath: /tmp/image-2.png' }, + ], '', undefined, [ diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index eafc530b7b..d240c70280 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -6,6 +6,7 @@ import { createAgentHostToolInvocationGuard } from '@qwen-code/qwen-code-core/agents/workspace-agents/capability.js'; import { isToolCallConcurrencySafe } from '@qwen-code/qwen-code-core/core/coreToolScheduler.js'; +import { collectText } from '@qwen-code/qwen-code-core/services/visionBridge/image-part-utils.js'; import { captureHookExecutionOwner, runWithHookExecutionOwner, @@ -303,6 +304,7 @@ import { DAEMON_INPUT_ANNOTATIONS_META_KEY, DAEMON_PERMISSION_CANCEL_REASON_META_KEY, DAEMON_PROMPT_DISPLAY_TEXT_META_KEY, + DAEMON_ATTACHMENT_CONTEXT_META_KEY, IMAGE_ONLY_PROMPT_TEXT, DAEMON_SUBMITTED_PROMPT_META_KEY, DAEMON_RESTORE_ASK_USER_QUESTION_META_KEY, @@ -9876,6 +9878,7 @@ export class Session implements SessionContext { // longer timeouts while remaining cancellable by the real turn. rawParts = await this.#applyBridgeConversionsIfNeeded( rawParts, + message.content, abortSignal, options.onFullTurnModel, ); @@ -16620,6 +16623,7 @@ export class Session implements SessionContext { }); return this.#applyBridgeConversionsIfNeeded( [...attachmentParts, ...expandedPrompt], + attachmentBlocks, abortSignal, onFullTurnModel, ); @@ -16807,6 +16811,7 @@ export class Session implements SessionContext { const sshWorkspace = Boolean(this.config.getExecutionEnvironment?.()); const embeddedContext: EmbeddedResourceResource[] = []; + const attachmentParts: Part[] = []; const extensionMentions = new Map(); const mcpServerMentions = new Map(); const textPathSpecsToRead = new Map(); @@ -16818,11 +16823,20 @@ export class Session implements SessionContext { ? parts : this.#applyBridgeConversionsIfNeeded( parts, + message, abortSignal, options.onFullTurnModel, ); const parts = message.map((part) => { + const attachmentContext = + part._meta?.[DAEMON_ATTACHMENT_CONTEXT_META_KEY]; + if ( + (part.type === 'image' || part.type === 'resource') && + typeof attachmentContext === 'string' + ) { + attachmentParts.push({ text: attachmentContext }); + } switch (part.type) { case 'text': if (sshWorkspace) return { text: part.text }; @@ -16970,13 +16984,18 @@ export class Session implements SessionContext { if ( pathSpecsToRead.length === 0 && embeddedContext.length === 0 && + attachmentParts.length === 0 && extensionParts.length === 0 && mcpServerParts.length === 0 ) { return finish(partsToSend); } - if (pathSpecsToRead.length === 0 && embeddedContext.length === 0) { + if ( + pathSpecsToRead.length === 0 && + embeddedContext.length === 0 && + attachmentParts.length === 0 + ) { return finish([...partsToSend, ...extensionParts, ...mcpServerParts]); } @@ -17012,6 +17031,7 @@ export class Session implements SessionContext { // the "--- Content from ... ---" delimiter labels, not by position, so // leading with the content is safe. const referenceParts: Part[] = [ + ...attachmentParts, ...partsToSend.filter((part) => 'inlineData' in part), ...extensionParts, ...mcpServerParts, @@ -17089,6 +17109,7 @@ export class Session implements SessionContext { async #applyBridgeConversionsIfNeeded( originalParts: Part[], + sourceBlocks: ContentBlock[], abortSignal: AbortSignal, onFullTurnModel?: (model: string) => boolean, ): Promise { @@ -17127,11 +17148,23 @@ export class Session implements SessionContext { let bridgeResult: VisionBridgeResult; try { + const attachmentContexts = new Set( + sourceBlocks + .filter( + (block) => block.type === 'image' || block.type === 'resource', + ) + .map((block) => block._meta?.[DAEMON_ATTACHMENT_CONTEXT_META_KEY]) + .filter((context) => typeof context === 'string'), + ); debugLogger.debug('vision bridge: gate matched, running conversion'); bridgeResult = await runVisionBridge({ config: this.config, parts, signal: abortSignal, + // Stored paths stay in model context, outside the bridge's focus budget. + intentText: collectText( + parts.filter((part) => !attachmentContexts.has(part.text ?? '')), + ), }); } catch (error) { debugLogger.debug(