From f57a4a044ad036b6b46718af12e163f655e3391c Mon Sep 17 00:00:00 2001 From: creatixchu Date: Fri, 24 Jul 2026 20:55:25 +0800 Subject: [PATCH] fix(gui): address ds-review-bot findings on image attachments - dsh web gains --provider/--model: a non-deepseek provider mounts the matching pi-ai catalog route (ambient credentials), making image input reachable from the shipped Web assembly; requires an explicit --model - attachment-local syncs the publication directories after the hard-link publish so a reported durable reference survives a crash (POSIX; Windows relies on filesystem metadata journaling) - the attachment seam gains storage-free validateImage; the host validates a complete multi-image prompt before persisting any member, so one malformed image cannot strand valid members as unreferenced objects - startSession sends before navigating: a rejected first send keeps the empty state, its error strip, and the complete draft mounted - the webserver rejects an undeclared-length body the moment it crosses the configured limit instead of draining a potentially endless stream to EOF --- ...ge-input-and-durable-attachments.i18n.yaml | 4 +-- ...dal-image-input-and-durable-attachments.md | 10 +++--- ...-image-input-and-durable-attachments.zh.md | 10 +++--- apps/cli/README.md | 2 +- apps/cli/src/web.ts | 20 ++++++++++- docs/cordis-catalog/services.md | 8 +++++ .../attachment/attachment-local/README.md | 2 +- .../attachment/attachment-local/src/index.ts | 8 +++-- .../attachment/attachment-local/src/store.ts | 33 +++++++++++++++++++ .../attachment-local/tests/index.spec.ts | 19 +++++++++++ packages/attachment/attachment/README.md | 2 +- packages/attachment/attachment/src/index.ts | 8 +++++ .../src/client/contract/slots.ts | 6 +++- .../ui-conversation/src/client/service.ts | 18 +++++----- .../tests/service-orchestration.spec.ts | 18 +++++++--- .../cordis/tool-cordis/src/api-catalog.ts | 4 +++ packages/host/runtime/src/api-proxy.ts | 10 ++++++ .../host/runtime/tests/host-runtime.spec.ts | 25 ++++++++++++++ packages/host/webserver/src/index.ts | 18 +++++----- .../host/webserver/tests/webserver.spec.ts | 21 ++++++++++++ packages/llm/llm-pi-ai/tests/adapter.spec.ts | 4 +++ .../llm/llm-pi-ai/tests/provider-apis.e2e.ts | 4 +++ scripts/test-invariants.ts | 4 +++ 23 files changed, 217 insertions(+), 41 deletions(-) diff --git a/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.i18n.yaml b/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.i18n.yaml index 5d438dfc9d..19d5dbc04a 100644 --- a/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write -2026-07-22-web-multimodal-image-input-and-durable-attachments.md: 156bec4a4f9bb05490847cbe775368bc472a33e9 -2026-07-22-web-multimodal-image-input-and-durable-attachments.zh.md: fb0f8d3031bb2d0e6af97136892cd07c22d575b3 +2026-07-22-web-multimodal-image-input-and-durable-attachments.md: a88cc0fd460bcc2b7f29c8cf5a3b9a91c5fc10e6 +2026-07-22-web-multimodal-image-input-and-durable-attachments.zh.md: ad2b8724e677c61121356921382d99d6e8741a0b diff --git a/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.md b/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.md index 156bec4a4f..a88cc0fd46 100644 --- a/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.md +++ b/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.md @@ -26,7 +26,7 @@ Version one supports PNG, JPEG, WebP, and GIF paste and drag-and-drop, image-onl - The rail is shared by the empty-state and resident composers, is hidden when empty, and scrolls horizontally instead of widening the composer. - Each approximately 72-by-72-pixel thumbnail has a remove action and opens its original draft image on double-click. - A prompt may contain text and images or images only. Pure text paste remains native browser behavior; mixed clipboard content inserts its text normally while adding its files to the rail, and file-only paste prevents default browser handling. File drops on the composer always prevent browser navigation and report unsupported files locally. -- A failed send restores the complete text and image draft. Removal, successful send, empty-state disposal, rendered-session disposal, and application disposal revoke the object URLs they own. +- A failed send restores the complete text and image draft. The empty state navigates to the new session only after its first send is accepted, so a rejected send keeps the draft and its error surface mounted. Removal, successful send, empty-state disposal, rendered-session disposal, and application disposal revoke the object URLs they own. - Historical user and assistant images use one `MessageImage` control. Inline images preserve intrinsic aspect ratio, do not upscale, and stay within a 240-by-240-pixel box. - Double-clicking a message image opens the stored original in a viewport-bounded modal. Escape, the close control, and backdrop activation close it and restore focus. - Version one does not override the browser context menu and provides no explicit image-copy action. @@ -63,7 +63,7 @@ interface ComposerAttachment { This split uses the slots framework's store seat and bound actions as the single subscription path for UI state while keeping non-serializable browser objects out of persisted JSON. Draft text and ordered image identifiers continue to use `localStorage`; after a reload, `ConversationRoot` prunes identifiers whose runtime objects no longer exist. Unsent images therefore do not survive reload because browser `File` and object URLs are not durable. A native client may stage input in an OS temporary directory, but it must treat that path exactly like the browser object URL: delete it when no longer needed and copy the bytes into the durable store before message acceptance. -The local attachment backend resolves an explicit `dshHome`, then `$DSH_HOME`, then `~/.dsh`. It stores content-addressed objects below `$DSH_HOME/attachments/v1/objects//` with owner-only directory and file permissions. A temporary file is written, synchronized, and atomically published before the service returns a reference. The content digest is encoded in the opaque `sha256:` identifier, and every read verifies the digest, media type, byte length, width, and height. +The local attachment backend resolves an explicit `dshHome`, then `$DSH_HOME`, then `~/.dsh`. It stores content-addressed objects below `$DSH_HOME/attachments/v1/objects//` with owner-only directory and file permissions. A temporary file is written, synchronized, atomically published, and made durable with a directory sync on the publication directories (POSIX; Windows relies on filesystem metadata journaling) before the service returns a reference. The content digest is encoded in the opaque `sha256:` identifier, and every read verifies the digest, media type, byte length, width, and height. The store performs no automatic deletion in version one. Sent user images and model-generated images remain reachable for history, resume, and fork. Reference-aware garbage collection needs a separate design because an age-only rule can delete data still referenced by a durable session. Deployment byte and pixel limits are admission policy on writes; reads verify the digest and recorded metadata without reapplying current admission limits, so lowering policy does not invalidate older history. @@ -108,7 +108,7 @@ type PromptInputPart = } ``` -Base64 crosses JSON-RPC once and is discarded after persistence. The host validates canonical base64, image count, aggregate bytes, individual bytes, magic-byte MIME, intrinsic dimensions, and decoded-pixel count. Only after every image succeeds does it call the agent with normalized text and durable image blocks. A failure appends no user event and exposes no attachment path or raw bytes. +Base64 crosses JSON-RPC once and is discarded after persistence. The host validates canonical base64, image count, aggregate bytes, individual bytes, magic-byte MIME, intrinsic dimensions, and decoded-pixel count — the complete batch, through the seam's storage-free `validateImage`, before persisting any member, so one malformed image cannot strand the batch's valid members as unreferenced objects. Only after every image succeeds does it call the agent with normalized text and durable image blocks. A failure appends no user event and exposes no attachment path or raw bytes. `session.attachment` is a read-only, session-scoped endpoint. The host serves bytes only when a durable event in that session references the requested attachment identifier. The client deduplicates loads by session and attachment identifier while that session is rendered, revokes resolved URLs on rendered-session disposal, and invalidates late loads so an unmounted session cannot repopulate the cache. @@ -118,7 +118,7 @@ Model catalog entries gain optional merge-extensible input and output modality d The host is the authoritative preflight boundary. It resolves the session's latest routed provider/model, falling back through agent options to host defaults; if that model explicitly excludes image input, it rejects the prompt before writing any attachment or event, and the client restores the draft. Unknown capability proceeds to the adapter guard so uncatalogued model identifiers remain usable. `host.describe` projects the default model and image limits into `SessionsService`; both composers use the limits before allocating object URLs or base64, while only the no-session composer uses the default model for early explicit text-only feedback. Decoded-pixel validation and every resident session's actual route remain authoritative on the host. -The Pi-AI adapter is the first visual-input route: it resolves `ctx.attachments` at request time, then resolves each durable reference and emits native image content only for models that declare image input. Request-time service resolution keeps Cordis load order from freezing optional attachment availability. The hand-written DeepSeek adapter throws typed `UNSUPPORTED_CONTENT` for an image anywhere in the request, including nested tool results. No adapter may flatten or skip an image. +The Pi-AI adapter is the first visual-input route: it resolves `ctx.attachments` at request time, then resolves each durable reference and emits native image content only for models that declare image input. The shipped Web assembly reaches it through `dsh web --provider --model `, which mounts that pi-ai catalog route with the provider's ambient credentials; the DeepSeek-only default remains text-only. Request-time service resolution keeps Cordis load order from freezing optional attachment availability. The hand-written DeepSeek adapter throws typed `UNSUPPORTED_CONTENT` for an image anywhere in the request, including nested tool results. No adapter may flatten or skip an image. Core supports structured assistant image blocks, but no current production provider route is certified for image output. Any future output-capable adapter must retrieve provider bytes under bounded size and time policy, validate them through the same attachment service, persist them, and only then publish the atomic `ImageBlock`. A URL in assistant Markdown remains text and is never downloaded automatically. @@ -134,7 +134,7 @@ Composer thumbnails and each `MessageImage` own ephemeral original-preview state ### Limits and trust boundaries -Version one accepts PNG, JPEG, WebP, and GIF only. SVG and remote URLs are excluded. Default limits are 5 MiB per image, 10 images and 20 MiB aggregate image bytes per message, and 40 million intrinsic pixels per image. These deployment-varying limits are validated backend configuration and are projected to the client for fast-path guidance; host validation remains authoritative. The Web carrier independently caps buffered API request bodies, with `dsh web` deriving its default from the aggregate image limit plus base64/envelope expansion and allowing an explicit override. +Version one accepts PNG, JPEG, WebP, and GIF only. SVG and remote URLs are excluded. Default limits are 5 MiB per image, 10 images and 20 MiB aggregate image bytes per message, and 40 million intrinsic pixels per image. These deployment-varying limits are validated backend configuration and are projected to the client for fast-path guidance; host validation remains authoritative. The Web carrier independently caps buffered API request bodies, with `dsh web` deriving its default from the aggregate image limit plus base64/envelope expansion and allowing an explicit override; a body without a declared length is rejected the moment it crosses the cap rather than drained to its end. Malformed base64, unsupported or mismatched media, truncated headers, excess bytes, excess image count, excess pixels, missing objects, and integrity mismatches return stable structured failures. Original filenames are reduced to a display basename, control characters are removed, and no local path is logged or returned to the browser. diff --git a/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.zh.md b/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.zh.md index fb0f8d3031..ad2b8724e6 100644 --- a/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.zh.md +++ b/.agents/notes/implemented/feature/2026-07-22-web-multimodal-image-input-and-durable-attachments.zh.md @@ -26,7 +26,7 @@ Status: implemented - 空状态输入区与常驻输入区共用附件栏;附件栏为空时隐藏,通过横向滚动避免撑宽输入区。 - 每个缩略图约为 72 × 72 像素,带有移除操作;双击时打开草稿原图。 - 提示词可同时包含文本与图片,也可仅包含图片。粘贴纯文本时保持浏览器原生行为;粘贴混合的剪贴板内容时,文本会正常插入,文件则同时添加到附件栏;仅粘贴文件时才阻止浏览器的默认处理。在输入区放置文件时总会阻止浏览器导航,并在本地报告不受支持的文件。 -- 发送失败时恢复完整的文本与图片草稿。移除、发送成功、空状态释放、已渲染会话释放和应用释放都会撤销各自持有的对象 URL。 +- 发送失败时恢复完整的文本与图片草稿。空状态只有在首次发送被接受后才导航到新会话,因此发送被拒绝时草稿及其错误提示仍保持挂载。移除、发送成功、空状态释放、已渲染会话释放和应用释放都会撤销各自持有的对象 URL。 - 历史用户图片与助手图片共用一个 `MessageImage` 控件。行内图片保持固有宽高比、不放大,并限制在 240 × 240 像素的边界框内。 - 双击消息图片会在不超出视口的模态框中打开存储的原图。按 Escape、激活关闭控件或激活背景区域都会关闭模态框并恢复焦点。 - 第一版不覆盖浏览器上下文菜单,也不提供明确的图片复制操作。 @@ -63,7 +63,7 @@ interface ComposerAttachment { 这一拆分让 UI 状态通过 slots 框架的 store 席位和绑定 actions 使用唯一的订阅路径,同时避免把不可序列化的浏览器对象写进持久 JSON。草稿文本和有序图片标识符继续使用 `localStorage`;重载后,`ConversationRoot` 会清理缺少对应运行时对象的标识符。未发送图片因此无法跨重载保留,因为浏览器 `File` 与对象 URL 不具备持久性。原生客户端可以在操作系统临时目录中暂存输入,但必须像对待浏览器对象 URL 一样对待该路径:不再需要时删除,并在消息被接受前把字节复制进持久存储。 -本地附件后端依次解析显式 `dshHome`、`$DSH_HOME` 和 `~/.dsh`。它把内容寻址对象存储在 `$DSH_HOME/attachments/v1/objects//` 下,并为目录和文件设置仅所有者可访问的权限。服务先写入并同步临时文件,再以原子方式发布,之后才返回引用。内容摘要编码在不透明的 `sha256:` 标识符中,每次读取都会校验摘要、媒体类型、字节长度、宽度和高度。 +本地附件后端依次解析显式 `dshHome`、`$DSH_HOME` 和 `~/.dsh`。它把内容寻址对象存储在 `$DSH_HOME/attachments/v1/objects//` 下,并为目录和文件设置仅所有者可访问的权限。服务先写入并同步临时文件,再以原子方式发布,并对发布目录执行目录同步使其持久(POSIX;Windows 依赖文件系统元数据日志),之后才返回引用。内容摘要编码在不透明的 `sha256:` 标识符中,每次读取都会校验摘要、媒体类型、字节长度、宽度和高度。 第一版不对存储执行自动删除。已发送的用户图片和模型生成图片会一直保留,以供历史记录、恢复和 fork 使用。按引用感知的垃圾回收需要单独设计,因为仅按时间清理可能删除仍被持久会话引用的数据。部署的字节和像素限制是写入时的准入策略;读取时会校验摘要和已记录的元数据,但不重新应用当前准入限制,因此收紧策略不会导致旧历史记录失效。 @@ -108,7 +108,7 @@ type PromptInputPart = } ``` -Base64 只跨越一次 JSON-RPC,并在持久化后丢弃。宿主会校验规范 base64、图片数量、总字节数、单张图片字节数、与魔数字节一致的 MIME、固有尺寸和解码像素数。只有每张图片都成功后,宿主才会用规范化文本和持久图片块调用 agent。失败时不追加用户事件,也不公开任何附件路径或原始字节。 +Base64 只跨越一次 JSON-RPC,并在持久化后丢弃。宿主会校验规范 base64、图片数量、总字节数、单张图片字节数、与魔数字节一致的 MIME、固有尺寸和解码像素数——并在持久化任何成员之前,通过服务边界上不触碰存储的 `validateImage` 校验完整批次,因此一张畸形图片不会把批次中的有效成员留成无引用的孤儿对象。只有每张图片都成功后,宿主才会用规范化文本和持久图片块调用 agent。失败时不追加用户事件,也不公开任何附件路径或原始字节。 `session.attachment` 是只读且限定于会话作用域的端点。只有该会话中的持久事件引用了所请求的附件标识符,宿主才提供字节。会话处于渲染状态时,客户端会按会话和附件标识符对加载操作去重;已渲染会话释放时会撤销已解析的 URL,并使延迟完成的加载失效,以免已卸载的会话重新写入缓存。 @@ -118,7 +118,7 @@ Base64 只跨越一次 JSON-RPC,并在持久化后丢弃。宿主会校验规 宿主是权威的前置检查边界。它会解析会话最新路由到的提供方和模型,并在缺失时依次回退到 agent 选项和宿主默认值;如果该模型明确排除图片输入,宿主会在写入任何附件或事件前拒绝提示词,客户端则恢复草稿。能力未知时继续进入适配器强制检查,使未收录的模型标识符仍然可用。`host.describe` 把默认模型和图片限制投影到 `SessionsService`;两种输入区都会在分配对象 URL 或 base64 前使用这些限制,只有无会话输入区会使用默认模型,针对明确仅支持文本的情况提前反馈。解码像素校验与每个常驻会话的实际路由均由宿主作出权威判定。 -Pi-AI 适配器是首条视觉输入路径:它在请求时解析 `ctx.attachments`,再解析每个持久引用,并且仅为声明支持图片输入的模型生成提供方原生图片内容。在请求时解析服务,可避免 Cordis 加载顺序将可选附件服务的可用性固化。手写 DeepSeek 适配器遇到请求中任何位置的图片时都会抛出类型化的 `UNSUPPORTED_CONTENT` 错误,包括嵌套工具结果中的图片。任何适配器都不得将图片展平或跳过。 +Pi-AI 适配器是首条视觉输入路径:它在请求时解析 `ctx.attachments`,再解析每个持久引用,并且仅为声明支持图片输入的模型生成提供方原生图片内容。交付的 Web 组装通过 `dsh web --provider --model ` 到达这条路径——该命令用提供方的环境凭据挂载对应的 pi-ai 目录路由;仅含 DeepSeek 的默认组装仍是纯文本。在请求时解析服务,可避免 Cordis 加载顺序将可选附件服务的可用性固化。手写 DeepSeek 适配器遇到请求中任何位置的图片时都会抛出类型化的 `UNSUPPORTED_CONTENT` 错误,包括嵌套工具结果中的图片。任何适配器都不得将图片展平或跳过。 核心层支持结构化助手图片块,但当前没有任何生产提供方路径通过图片输出认证。未来任何支持输出的适配器都必须在有界的大小和时间策略下获取提供方字节,通过同一个附件服务校验并持久化字节,之后才能以原子方式发布 `ImageBlock`。助手 Markdown 中的 URL 仍是文本,绝不自动下载。 @@ -134,7 +134,7 @@ token 估算根据图片尺寸计量,但不把 base64 或附件定位符作为 ### 限制与信任边界 -第一版仅接受 PNG、JPEG、WebP 和 GIF。不接受 SVG 和远程 URL。默认限制为每张图片 5 MiB、每条消息 10 张图片和 20 MiB 图片总字节数,以及每张图片 4,000 万个固有像素。这些随部署变化的限制属于经过校验的后端配置,并会投影给客户端以提供快速路径指引;宿主校验仍是权威结果。Web 载体会独立限制 API 请求体的缓冲大小;`dsh web` 根据图片总量限制加上 base64 和请求封装的膨胀量推导默认值,并允许显式覆盖。 +第一版仅接受 PNG、JPEG、WebP 和 GIF。不接受 SVG 和远程 URL。默认限制为每张图片 5 MiB、每条消息 10 张图片和 20 MiB 图片总字节数,以及每张图片 4,000 万个固有像素。这些随部署变化的限制属于经过校验的后端配置,并会投影给客户端以提供快速路径指引;宿主校验仍是权威结果。Web 载体会独立限制 API 请求体的缓冲大小;`dsh web` 根据图片总量限制加上 base64 和请求封装的膨胀量推导默认值,并允许显式覆盖;未声明长度的请求体在越过上限的瞬间即被拒绝,而不是先读完再拒。 格式错误的 base64、不支持或不匹配的媒体、截断的文件头、超出字节限制、超出图片数量、超出像素限制、对象缺失和完整性不匹配都会返回稳定的结构化错误。原始文件名只保留用于显示的末段,控制字符会被移除,并且任何本地路径都不会写入日志或返回浏览器。 diff --git a/apps/cli/README.md b/apps/cli/README.md index e6a33247ca..89390a6dff 100644 --- a/apps/cli/README.md +++ b/apps/cli/README.md @@ -10,7 +10,7 @@ The TUI surface: - tells the agent where its own source lives: after boot it adds a prompt section naming this harness checkout, resolved from the launcher's real path so it holds under a PATH symlink and an arbitrary cwd, so the self-referential `cordis` toolset can read and modify it; - applies the personal overlay from `~/.dsh` (see [app-boot's Personal config](../../packages/ui/app-boot/README.md#personal-config)): `.env` fills environment gaps (ambient > project `.env` > personal `.env`), `config.yaml` patches the booted tree. -The Web surface treats its invoking directory as the default project, loads applicable `AGENTS.md`/`CLAUDE.md` instructions into each agent-loop request prefix with a 65,536-byte render budget, and opts into first-message model titles. The headless surface retains deterministic fallback titles without making the auxiliary title-model request. +The Web surface treats its invoking directory as the default project, loads applicable `AGENTS.md`/`CLAUDE.md` instructions into each agent-loop request prefix with a 65,536-byte render budget, and opts into first-message model titles. `dsh web --provider --model ` selects the default agent route; a non-`deepseek` provider additionally mounts that pi-ai catalog route with the provider's ambient credentials (e.g. `ANTHROPIC_API_KEY`), which is how image input reaches a visual-capable model, and requires an explicit `--model`. The headless surface retains deterministic fallback titles without making the auxiliary title-model request. ## Install (developer machine) diff --git a/apps/cli/src/web.ts b/apps/cli/src/web.ts index ea7ed1f2eb..97da4734c1 100644 --- a/apps/cli/src/web.ts +++ b/apps/cli/src/web.ts @@ -49,6 +49,8 @@ export async function runWeb(argv: string[]): Promise { host: { type: 'string', default: LOOPBACK_HOST }, port: { type: 'string', default: '3080' }, 'max-request-body-bytes': { type: 'string' }, + provider: { type: 'string' }, + model: { type: 'string' }, dev: { type: 'boolean', default: false }, }, allowPositionals: false, @@ -77,12 +79,28 @@ export async function runWeb(argv: string[]): Promise { process.exit(1) } - // A missing DEEPSEEK_API_KEY throws here (plugin load is fail-loud, uncaught by design). + // Default routing: --provider deepseek (implicit) keeps the DeepSeek-only + // assembly; any other --provider additionally mounts that pi-ai catalog + // route (credentials via the provider's ambient discovery, e.g. + // ANTHROPIC_API_KEY) so visual-capable models are reachable from the shipped + // Web app. A non-default provider requires an explicit --model — this shell + // has no evidence for inventing another provider's default. + const provider = values.provider ?? 'deepseek' + if (provider !== 'deepseek' && values.model === undefined) { + process.stderr.write(`dsh web: --provider ${provider} requires an explicit --model\n`) + process.exit(1) + } + + // A missing DEEPSEEK_API_KEY throws here (plugin load is fail-loud, uncaught + // by design); an unknown --provider fails the pi-ai catalog check the same way. const host = await startHost({ boot: { persistenceRoot: './.sessions', workspaceContext: { maxBytes: 65_536 }, sessionTitleLlm: true, + ...provider === 'deepseek' ? {} : { piAiProviders: [{ provider }] }, + ...values.provider === undefined ? {} : { provider: values.provider }, + ...values.model === undefined ? {} : { model: values.model }, }, }) const attachments = host.ctx.get('attachments') diff --git a/docs/cordis-catalog/services.md b/docs/cordis-catalog/services.md index 0c6e930a44..51297f8a45 100644 --- a/docs/cordis-catalog/services.md +++ b/docs/cordis-catalog/services.md @@ -253,6 +253,14 @@ Source: [`packages/ui/user-approval/src/index.ts:213`](../../packages/ui/user-ap Immutable binary attachment service. Implementations validate bytes before publishing a reference. ```ts cordis-catalog +/** + * Validate one image against the deployment policy without persisting anything. + * Callers persisting a multi-image batch validate every member first so a + * malformed member cannot leave earlier members as unreferenced objects. + * @param input - encoded bytes, declared media type, and optional display name. + */ +abstract validateImage(input: SaveImageAttachment): void + /** * Validate and durably commit one image before its owning session event is appended. * @param input - encoded bytes, declared media type, and optional display name. diff --git a/packages/attachment/attachment-local/README.md b/packages/attachment/attachment-local/README.md index 3e867695d6..d1c4327203 100644 --- a/packages/attachment/attachment-local/README.md +++ b/packages/attachment/attachment-local/README.md @@ -1,6 +1,6 @@ # @deepseek-ai/dsh-attachment-local -The private local implementation of [`@deepseek-ai/dsh-attachment`](../attachment). Objects land at `/attachments/v1/objects//` and are addressed by an opaque `sha256:` id. Writes use a private staging directory, owner-only files, a synced temporary file, and an atomic exclusive hard-link publish; reads re-check the digest, media signature, dimensions, and logged metadata. Byte and pixel limits are write-time admission policy, so a later policy reduction does not make already-admitted history unreadable. +The private local implementation of [`@deepseek-ai/dsh-attachment`](../attachment). Objects land at `/attachments/v1/objects//` and are addressed by an opaque `sha256:` id. Writes use a private staging directory, owner-only files, a synced temporary file, an atomic exclusive hard-link publish, and a directory sync on the publication directories (POSIX; Windows relies on filesystem metadata journaling) so the reported reference survives a crash; reads re-check the digest, media signature, dimensions, and logged metadata. Byte and pixel limits are write-time admission policy, so a later policy reduction does not make already-admitted history unreadable. `DSH_HOME` resolves through the shared path policy: explicit config, `$DSH_HOME`, then `~/.dsh`. Session logs contain only the reference and verified metadata, never this host path. diff --git a/packages/attachment/attachment-local/src/index.ts b/packages/attachment/attachment-local/src/index.ts index acb06f16c9..18ce810f7b 100644 --- a/packages/attachment/attachment-local/src/index.ts +++ b/packages/attachment/attachment-local/src/index.ts @@ -6,10 +6,10 @@ import z from 'schemastery' import { AttachmentStore } from '@deepseek-ai/dsh-attachment' import type { ImageAttachmentLimits, ImageAttachmentRef, SaveImageAttachment, StoredImageAttachment } from '@deepseek-ai/dsh-attachment' import { resolveDshHome } from '@deepseek-ai/dsh-paths' -import { readImageFile, saveImageFile } from './store.ts' +import { readImageFile, saveImageFile, validateImageFile } from './store.ts' export { detectImage } from './image.ts' -export { readImageFile, saveImageFile } from './store.ts' +export { readImageFile, saveImageFile, validateImageFile } from './store.ts' export { AttachmentError } from '@deepseek-ai/dsh-attachment' export type { ImageAttachmentRef } from '@deepseek-ai/dsh-attachment' @@ -62,6 +62,10 @@ export class LocalAttachmentStore extends AttachmentStore { }) } + validateImage(input: SaveImageAttachment): void { + validateImageFile(input, this.imageLimits) + } + async saveImage(input: SaveImageAttachment): Promise { return saveImageFile(this.root, input, this.imageLimits) } diff --git a/packages/attachment/attachment-local/src/store.ts b/packages/attachment/attachment-local/src/store.ts index 22e2ca2309..34431f5a2b 100644 --- a/packages/attachment/attachment-local/src/store.ts +++ b/packages/attachment/attachment-local/src/store.ts @@ -52,6 +52,32 @@ function validateAdmission(metadata: Omit { + /* v8 ignore next -- Windows cannot open directory handles; NTFS metadata journaling owns entry durability there. */ + if (process.platform === 'win32') return + const handle = await open(path, constants.O_RDONLY) + try { + await handle.sync() + } finally { + await handle.close() + } +} + /** * Save and verify immutable image bytes below a versioned attachment root. * @param root - absolute `DSH_HOME/attachments/v1` root. @@ -86,6 +112,13 @@ export async function saveImageFile(root: string, input: SaveImageAttachment, li const existing = new Uint8Array(await readFile(target)) if (digest(existing) !== sha256) throw new AttachmentError('Stored attachment failed integrity verification.', 'ATTACHMENT_CORRUPT') } + // The synced file becomes durable only once its directory entries are: sync + // the bucket (the new object entry) and its parent (the possibly new bucket + // entry) before this reference can reach a session checkpoint. The dedup + // path syncs too — the earlier save that created the entry may have crashed + // before its own directory sync. + await syncDirectory(bucket) + await syncDirectory(join(root, 'objects')) await unlink(temporary) } catch (error) { /* v8 ignore next -- A descriptor can remain open only when the underlying write/sync/close operation fails. */ diff --git a/packages/attachment/attachment-local/tests/index.spec.ts b/packages/attachment/attachment-local/tests/index.spec.ts index b21b0d544d..b99df8b179 100644 --- a/packages/attachment/attachment-local/tests/index.spec.ts +++ b/packages/attachment/attachment-local/tests/index.spec.ts @@ -1,4 +1,5 @@ import { Context } from 'cordis' +import { existsSync } from 'node:fs' import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -36,4 +37,22 @@ describe('local attachment service', () => { await rm(dshHome, { recursive: true, force: true }) } }) + + it('validates without persisting: a rejected image leaves no storage root behind', async () => { + const dshHome = await mkdtemp(join(tmpdir(), 'dsh-attachment-validate-')) + try { + const service = new LocalAttachmentStore(new Context(), { dshHome }) + expect(() => { service.validateImage({ data: Uint8Array.of(1, 2, 3), mediaType: 'image/png' }) }) + .toThrow(/Unsupported or malformed image data/) + const valid = Uint8Array.from(Buffer.from( + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=', + 'base64', + )) + expect(() => { service.validateImage({ data: valid, mediaType: 'image/png' }) }).not.toThrow() + // Validation is storage-free: nothing below the root may exist yet. + expect(existsSync(service.root)).toBe(false) + } finally { + await rm(dshHome, { recursive: true, force: true }) + } + }) }) diff --git a/packages/attachment/attachment/README.md b/packages/attachment/attachment/README.md index 2fcc99c477..11e2d4035f 100644 --- a/packages/attachment/attachment/README.md +++ b/packages/attachment/attachment/README.md @@ -2,7 +2,7 @@ The durable attachment seam. `ctx.attachments` validates and atomically commits immutable image bytes, then returns a serializable `ImageAttachmentRef`; consumers never persist browser paths, object URLs, provider URLs, or base64 in session events. -Unsent composer images remain browser-owned temporary drafts. `saveImage` is called only at message submission or while committing structured provider output, before any model-visible session event is published. `readImage` verifies the content-addressed object against its logged metadata. +Unsent composer images remain browser-owned temporary drafts. `saveImage` is called only at message submission or while committing structured provider output, before any model-visible session event is published. `validateImage` runs the same admission policy without persisting; batch writers validate every member first so one malformed member cannot strand earlier members as unreferenced objects (there is no garbage collection). `readImage` verifies the content-addressed object against its logged metadata. ## Model Experience diff --git a/packages/attachment/attachment/src/index.ts b/packages/attachment/attachment/src/index.ts index 000856be78..8ddf7dd1d1 100644 --- a/packages/attachment/attachment/src/index.ts +++ b/packages/attachment/attachment/src/index.ts @@ -33,6 +33,14 @@ export abstract class AttachmentStore extends Service { /** Deployment-resolved image policy used by authoritative and fast-path validation. */ abstract readonly imageLimits: ImageAttachmentLimits + /** + * Validate one image against the deployment policy without persisting anything. + * Callers persisting a multi-image batch validate every member first so a + * malformed member cannot leave earlier members as unreferenced objects. + * @param input - encoded bytes, declared media type, and optional display name. + */ + abstract validateImage(input: SaveImageAttachment): void + /** * Validate and durably commit one image before its owning session event is appended. * @param input - encoded bytes, declared media type, and optional display name. diff --git a/packages/client/ui-conversation/src/client/contract/slots.ts b/packages/client/ui-conversation/src/client/contract/slots.ts index 6b46086be3..7bea1c7e33 100644 --- a/packages/client/ui-conversation/src/client/contract/slots.ts +++ b/packages/client/ui-conversation/src/client/contract/slots.ts @@ -187,7 +187,11 @@ export interface EmptyStateInjected { releaseDraftImage(id: string): void /** Release all service-owned image previews held by the empty state. */ releaseDraftImages(attachments: readonly ComposerAttachment[]): void - /** The create → navigate → first-send chain, in one service call. */ + /** + * The create → first-send → navigate chain, in one service call. Navigation + * happens only after the send is accepted, so a failure leaves the empty + * state and its draft mounted. + */ startSession(opts: { cwd?: string text: string diff --git a/packages/client/ui-conversation/src/client/service.ts b/packages/client/ui-conversation/src/client/service.ts index 0336809191..f6b0582d85 100644 --- a/packages/client/ui-conversation/src/client/service.ts +++ b/packages/client/ui-conversation/src/client/service.ts @@ -209,12 +209,12 @@ export class ConversationService extends Service { /** * Empty-state first-send chain (root-context method; does not read scope): - * create the session, navigate to it, then send through the new scope. - * The create → open ordering is safe: the manager merges the new summary - * synchronously before create() resolves, so the list store is projected by - * the time open() validates against it (manager notification batching is - * microtask-based; SessionsService projects on the same flush that create - * awaited through the RPC round trip). + * create the session, send through the new scope, and navigate only after + * the send is accepted. Navigation is the publication point — opening + * earlier would unmount the empty state (releasing its draft previews) + * while the send can still fail, leaving the failure with no surface and + * the user with a lost draft; on rejection here the still-mounted empty + * state keeps the draft and shows the error locally. * @param opts - project directory, prompt text, images, and send mode. */ async startSession(opts: { @@ -226,9 +226,10 @@ export class ConversationService extends Service { const sessions = this.requireSessions() const id = await sessions.create(opts.cwd === undefined ? {} : { cwd: opts.cwd }) // The manager notifier flushes per microtask; one await guarantees the - // list-store projection landed before sessions.open validates against it. + // list-store projection landed before sessions.open validates against it + // (the manager merges the new summary synchronously before create() + // resolves; batching is microtask-based). await Promise.resolve() - sessions.open(id) const scoped = sessions.scope(id) if (scoped === undefined) throw new Error(`conversation.startSession: created session "${id}" resolved no scope`) // ctx.get, not scoped.conversation: property access walks the fiber @@ -237,6 +238,7 @@ export class ConversationService extends Service { const scopedConversation = scoped.get('conversation') if (scopedConversation === undefined) throw new Error('conversation.startSession: conversation service unavailable through the new scope') await scopedConversation.send(opts.text, opts.mode, opts.images ?? []) + sessions.open(id) } /** Resolve the caller scope's Session or throw on root contexts. */ diff --git a/packages/client/ui-conversation/tests/service-orchestration.spec.ts b/packages/client/ui-conversation/tests/service-orchestration.spec.ts index 1362fbd851..bc4c0200d0 100644 --- a/packages/client/ui-conversation/tests/service-orchestration.spec.ts +++ b/packages/client/ui-conversation/tests/service-orchestration.spec.ts @@ -2,7 +2,7 @@ /** * ConversationService orchestration half after the store-seat slimming: * scope-addressed send/cancel (result folding, root throw), the startSession - * chain (create → sessions.open → scoped send), and the service-unavailable + * chain (create → scoped send → sessions.open), and the service-unavailable * loud failures. Selection/draft state left this service for the declared * chat store (chat-store.spec.ts / selection-survival.spec.ts); the view * registry left for the 'conversation.view' slot (views-type-chain.spec.tsx). @@ -280,13 +280,23 @@ describe('image admission and URL lifecycle', () => { }) describe('startSession chain', () => { - it('creates, navigates through sessions.open, then sends through the new scope', async () => { + it('creates, sends through the new scope, then navigates through sessions.open', async () => { const b = await bench() await b.svc.startSession({ cwd: '/proj', text: 'first', mode: 'queue' }) expect(b.createMock).toHaveBeenCalledWith({ cwd: '/proj' }) expect(b.openMock).toHaveBeenCalledWith(sid('new-1')) - expect(b.sessionDoubles.get(sid('new-1'))!.prompt).toHaveBeenCalledWith( - [{ type: 'text', text: 'first' }], 'queue') + const prompt = b.sessionDoubles.get(sid('new-1'))!.prompt + expect(prompt).toHaveBeenCalledWith([{ type: 'text', text: 'first' }], 'queue') + // Navigation is the publication point: it must not precede send acceptance. + expect(b.openMock.mock.invocationCallOrder[0]!).toBeGreaterThan(prompt.mock.invocationCallOrder[0]!) + }) + + it('does not navigate when the first send is rejected (empty state keeps the draft)', async () => { + const b = await bench() + const doomed = b.sessionsFake.manager.get(sid('new-1')) as unknown as SessionDouble + doomed.prompt.mockResolvedValue({ ok: false, error: { code: 'agent-busy', message: 'nope' } }) + await expect(b.svc.startSession({ text: 'first', mode: 'queue' })).rejects.toThrow(/agent-busy/) + expect(b.openMock).not.toHaveBeenCalled() }) it('omits cwd from create when not chosen', async () => { diff --git a/packages/cordis/tool-cordis/src/api-catalog.ts b/packages/cordis/tool-cordis/src/api-catalog.ts index 0ef2082f2a..cf4bcbff5c 100644 --- a/packages/cordis/tool-cordis/src/api-catalog.ts +++ b/packages/cordis/tool-cordis/src/api-catalog.ts @@ -156,6 +156,10 @@ export const SERVICE_API: readonly ServiceApiEntry[] = [ key: 'attachments', summary: 'Immutable binary attachment service.', methods: [ + { + signature: 'abstract validateImage(input: SaveImageAttachment): void', + jsDoc: '/**\n * Validate one image against the deployment policy without persisting anything.\n * Callers persisting a multi-image batch validate every member first so a\n * malformed member cannot leave earlier members as unreferenced objects.\n * @param input - encoded bytes, declared media type, and optional display name.\n */', + }, { signature: 'abstract saveImage(input: SaveImageAttachment): Promise', jsDoc: '/**\n * Validate and durably commit one image before its owning session event is appended.\n * @param input - encoded bytes, declared media type, and optional display name.\n * @returns a durable content-addressed reference.\n */', diff --git a/packages/host/runtime/src/api-proxy.ts b/packages/host/runtime/src/api-proxy.ts index 5ca1e853b1..b56a49aab0 100644 --- a/packages/host/runtime/src/api-proxy.ts +++ b/packages/host/runtime/src/api-proxy.ts @@ -54,6 +54,16 @@ async function durablePromptContent(ctx: Context, content: readonly PromptConten if (totalBytes > limits.maxMessageImageBytes) { throw new AttachmentError('Prompt exceeds the configured aggregate image-byte limit.', 'IMAGES_TOO_LARGE') } + // Validate the complete batch before persisting any member: the store has no + // garbage collection, so one malformed image must not leave the batch's + // valid members as published objects no message event will ever reference. + for (const image of images) { + ctx.attachments.validateImage({ + data: image.data, + mediaType: image.part.mediaType, + ...image.part.name === undefined ? {} : { name: image.part.name }, + }) + } return Promise.all(prepared.map(async (item): Promise => { if (!('data' in item)) return { type: 'text', text: item.text } const attachment = await ctx.attachments.saveImage({ diff --git a/packages/host/runtime/tests/host-runtime.spec.ts b/packages/host/runtime/tests/host-runtime.spec.ts index f7cf357431..2abc8ba48f 100644 --- a/packages/host/runtime/tests/host-runtime.spec.ts +++ b/packages/host/runtime/tests/host-runtime.spec.ts @@ -614,6 +614,31 @@ describe('sessions.prompt / cancel', () => { }) }) + it('publishes nothing when one member of a multi-image prompt is malformed', async () => { + const persistenceRoot = mkdtempSync(join(tmpdir(), 'dsh-batch-session-')) + const dshHome = mkdtempSync(join(tmpdir(), 'dsh-batch-home-')) + host = await startHost({ + boot: { persistenceRoot, workspaceContext: false, dshHome, provider: 'scripted', model: 'test-model' }, + }) + host.ctx.llm.registerAdapter(['scripted'], new ScriptedAdapter([textResponse('unused')])) + const { sessionId } = expectOk(await host.api.sessions.create(request({}))) + const response = await host.api.sessions.prompt(request({ + sessionId, + mode: 'queue' as const, + content: [ + { type: 'image' as const, mediaType: 'image/png' as const, data: PNG_BASE64 }, + // Canonical base64, but the bytes are not a PNG: the whole batch must + // be validated before any member persists, or the valid image above + // would become a permanently unreferenced object (this store has no GC). + { type: 'image' as const, mediaType: 'image/png' as const, data: 'AQID' }, + ], + })) + expect(response.result).toMatchObject({ + ok: false, error: { details: { reason: 'INVALID_IMAGE' } }, + }) + expect(existsSync(join(dshHome, 'attachments'))).toBe(false) + }) + it('rejects images for an explicitly text-only model without creating a session event', async () => { const persistenceRoot = mkdtempSync(join(tmpdir(), 'dsh-text-session-')) const dshHome = mkdtempSync(join(tmpdir(), 'dsh-text-home-')) diff --git a/packages/host/webserver/src/index.ts b/packages/host/webserver/src/index.ts index 77c7bf4301..9406b66634 100644 --- a/packages/host/webserver/src/index.ts +++ b/packages/host/webserver/src/index.ts @@ -214,21 +214,19 @@ async function bridge( } const chunks: Buffer[] = [] let received = 0 - let oversized = false for await (const chunk of req) { const buffer = chunk as Buffer received += buffer.byteLength if (received > maxRequestBodyBytes) { - oversized = true - chunks.length = 0 - continue + // Reject the moment the threshold is crossed: draining a chunked body to + // EOF first would let a client without Content-Length stream + // indefinitely while holding the socket and this request task. + res.writeHead(413, { connection: 'close' }) + res.end() + req.destroy() + return } - if (!oversized) chunks.push(buffer) - } - if (oversized) { - res.writeHead(413) - res.end() - return + chunks.push(buffer) } /* v8 ignore next 3 -- `??` arms: node:http always sets url/method on server requests; the fields are only optional on the client-side IncomingMessage type */ diff --git a/packages/host/webserver/tests/webserver.spec.ts b/packages/host/webserver/tests/webserver.spec.ts index 63552127bc..c6ab019564 100644 --- a/packages/host/webserver/tests/webserver.spec.ts +++ b/packages/host/webserver/tests/webserver.spec.ts @@ -435,6 +435,27 @@ describe('/api bridge', () => { expect(status).toBe(413) }) + it('rejects an unterminated chunked body at the threshold without draining to EOF', async () => { + const base = await boot(() => undefined, 8) + const target = new URL(`${base}/api/echo`) + // The client never calls end(): the 413 must arrive the moment the limit + // is crossed, or a hostile stream would hold the socket open forever. + const status = await new Promise((resolve, reject) => { + const request = httpRequest({ + hostname: target.hostname, + port: target.port, + path: target.pathname, + method: 'POST', + }, (response) => { + response.resume() + response.on('end', () => { resolve(response.statusCode) }) + }) + request.on('error', reject) + request.write('123456789') + }) + expect(status).toBe(413) + }) + it('relays a bodyless response', async () => { const base = await boot() const response = await fetch(`${base}/api/empty`, { method: 'POST' }) diff --git a/packages/llm/llm-pi-ai/tests/adapter.spec.ts b/packages/llm/llm-pi-ai/tests/adapter.spec.ts index de12d81e5e..749627e961 100644 --- a/packages/llm/llm-pi-ai/tests/adapter.spec.ts +++ b/packages/llm/llm-pi-ai/tests/adapter.spec.ts @@ -226,6 +226,10 @@ describe('PiAiAdapter provider routing', () => { mediaTypes: ['image/png'], } + validateImage(_input: SaveImageAttachment): void { + throw new Error('not used') + } + saveImage(_input: SaveImageAttachment): Promise { return Promise.reject(new Error('not used')) } diff --git a/packages/llm/llm-pi-ai/tests/provider-apis.e2e.ts b/packages/llm/llm-pi-ai/tests/provider-apis.e2e.ts index 85dcbd5706..09e21dea6e 100644 --- a/packages/llm/llm-pi-ai/tests/provider-apis.e2e.ts +++ b/packages/llm/llm-pi-ai/tests/provider-apis.e2e.ts @@ -74,6 +74,10 @@ async function harness(image?: StoredImageAttachment): Promise { mediaTypes: [fixture.ref.mediaType], } + validateImage(_input: SaveImageAttachment): void { + throw new Error('e2e attachment fixture is read-only') + } + saveImage(_input: SaveImageAttachment): Promise { return Promise.reject(new Error('e2e attachment fixture is read-only')) } diff --git a/scripts/test-invariants.ts b/scripts/test-invariants.ts index 0f58cbd059..c843f94a5a 100644 --- a/scripts/test-invariants.ts +++ b/scripts/test-invariants.ts @@ -96,6 +96,10 @@ class TestAttachmentStore extends AttachmentStore { mediaTypes: ['image/png'], } + validateImage(_input: SaveImageAttachment): void { + throw new Error('test invariant attachment store does not validate images') + } + saveImage(_input: SaveImageAttachment): Promise { return Promise.reject(new Error('test invariant attachment store does not save images')) }