Skip to content

fix(chat): stage selected attachments before sending#274

Merged
RachelForster merged 2 commits into
RachelForster:mainfrom
End0rph1nww:fix/stage-selected-chat-attachments
Jul 25, 2026
Merged

fix(chat): stage selected attachments before sending#274
RachelForster merged 2 commits into
RachelForster:mainfrom
End0rph1nww:fix/stage-selected-chat-attachments

Conversation

@End0rph1nww

@End0rph1nww End0rph1nww commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

What

  • replace the chat Image/File path-picker flow with native file selection that reuses the existing attachment upload endpoint
  • copy both image and ordinary file selections into managed data/chat_attachments storage before sending
  • disable only message submission while attachment staging is pending, including keyboard submission
  • update the chat-stage test to cover both picker buttons, staged paths, and the pending-upload race

Why

The previous picker returned an absolute local path and submitted it directly with /api/chat/command. The backend intentionally rejects raw paths outside the allowed attachment root, so files selected from Downloads, another drive, or another external folder failed with Attachment path is outside the allowed directory.

Drag-and-drop already avoided this failure by calling /api/chat/attachments/upload first. This change makes the picker buttons use that same established flow.

Impact

Users can select images or ordinary files from outside the project. The files are copied into managed attachment storage, and the chat command receives only the staged path. Backend path validation is unchanged, so raw outside paths remain blocked and the security boundary is not expanded.

Checks

  • pnpm test — 109 files / 717 tests passed
  • pnpm lint:types
  • pnpm format:check
  • pnpm build
  • python -m pytest -q test/unit/core/media/test_chat_attachments.py test/unit/plugins/test_user_input_attachments.py — 12 passed

Fixes #273

Summary by Ameath

小爱先把这次核对的重点轻轻拎出来,方便快速进入后续内容。

该 PR 将聊天舞台的 Image/File 路径选择器替换为浏览器原生文件选择,并通过既有附件上传端点暂存文件;普通发送按钮和 Enter 在暂存中会禁用。审查发现,选项提交仍能绕过该保护,且重复选择同一文件会作为不同暂存路径重复加入。

Enhancements

  • 原生选择将 File[] 交给 uploadChatAttachments,返回的受管路径会进入 send-message payload。
  • InputLayer 在附件上传未完成时禁用普通发送按钮和键盘提交。

Tests

  • 更新 ChatStagePage 测试,覆盖两个选择器、暂存路径及键盘发送竞态。
Original summary in English

This PR replaces the chat-stage Image/File path picker with native browser file selection and stages files through the existing upload endpoint; the normal send button and Enter submission are disabled while staging is pending. The review found that option submission still bypasses this protection, and selecting the same file repeatedly adds it multiple times under distinct staged paths.

Enhancements

  • Native selection passes File[] to uploadChatAttachments, and the managed paths are included in the send-message payload.
  • InputLayer disables the normal send button and keyboard submission until attachment uploads finish.

Tests

  • Updates ChatStagePage tests for both pickers, staged paths, and the keyboard-send race.

@End0rph1nww
End0rph1nww marked this pull request as ready for review July 24, 2026 17:56
@ameath-review

ameath-review Bot commented Jul 24, 2026

Copy link
Copy Markdown

Reviewer's Guide by Ameath

小爱整理了一条清晰的查看路径,按着看会更顺手一些。

审查聚焦 ChatStagePageuploadChatAttachments/api/chat/attachments/upload → 暂存路径 → send-message 的生产链路,以及 InputLayer 的按钮和键盘提交。PR 已链接 #273,提交标题符合 Conventional Commits,前端实现和测试均位于对应的 chat-stage feature 边界内。

  • 优先检查附件暂存期间的 OptionsLayer:它仍可触发 submit-option,而该命令与普通消息共享回合提交路径。
  • 优先检查重复选择:后端为每次上传生成 UUID 暂存目录,但客户端去重依据是 kind + path

文件级变更

Change Details Files
修改文件 3 个文件,+76 / -66 frontend/src/features/chat-stage/ChatStagePage.tsx
frontend/src/features/chat-stage/components/InputLayer.tsx
frontend/src/test/features/chat-stage/ChatStagePage.test.tsx

关联 Issue

  • #273 Chat attachment picker submits paths outside the allowed directory:PR 正文明确引用;完成度分析见上方审查指南。

Tips and commands
  • 重新审查: @Ame4th review
  • 重新生成摘要: @Ame4th summary
  • 重新生成指南: @Ame4th guide
  • 生成标题: @Ame4th title
  • 解决本机器人线程: @Ame4th resolve
  • 清理本机器人审查: @Ame4th dismiss
  • 从行级评论创建 Issue: 回复 @Ame4th issue
  • 继续讨论: 直接回复任一行级评论。
  • 反馈偏好: 对行级评论点 👍 或 👎。
Original review guide in English

The review focused on the production path from ChatStagePage through uploadChatAttachments, /api/chat/attachments/upload, managed staged paths, and send-message, as well as button and keyboard submission in InputLayer. The PR links #273, its commit title follows Conventional Commits, and the frontend implementation and test remain within the chat-stage feature boundary.

  • Prioritize the OptionsLayer while attachment staging is pending: it can still issue submit-option, which shares the turn-submission path with normal messages.
  • Prioritize repeated selection: the backend creates a UUID staging directory for every upload, while client-side deduplication keys on kind + path.

@ameath-review ameath-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

小爱在需要留意的地方补了简短说明,方便逐处确认。

小爱已完成全部变更审查,共发布 2 条行级建议(高风险 0,中风险 1)。

Prompt for AI Agents

请逐项处理以下审查问题;修改前先核实上下文,修改后运行相关测试。

1. 暂存上传仍可提交选项 / Option submission bypasses staging guard

frontend/src/features/chat-stage/ChatStagePage.tsx:589 (RIGHT)
<issue_to_address>submitDisabled 仅作用于 InputLayer。选项出现时输入层仍可见,而 OptionsLayer 的按钮没有读取该状态;因此通过新 picker 发起上传后,用户仍能立刻选择选项并发送 submit-option。该命令与 send-message 共享回合提交路径,上传完成后附件会落入下一条草稿,而不是阻止这次提交。

English: submitDisabled only reaches InputLayer. Options remain enabled during a new picker upload and can immediately submit submit-option; the staged attachment is then added to the next draft after the turn has already advanced.</issue_to_address>

2. 重复选择同一文件会产生重复附件 / Repeated selection bypasses deduplication

frontend/src/features/chat-stage/ChatStagePage.tsx:295 (RIGHT)
<issue_to_address>这里每次选择都会清空 input 并发起新的暂存上传。暂存端点为每个文件生成 UUID 子目录并返回不同的 path,而 mergeChatAttachmentInputs 只按 kind + path 去重。因此重复选择同一文件会显示并发送两份附件,还可能占满 8 个槽位;旧 PathPickerDialog 传入相同原始路径时会被去重。

English: Every repeat selection is staged under a new UUID path, while deduplication uses kind + path. Selecting the same file twice therefore sends two attachments and can consume the attachment limit.</issue_to_address>

Comment thread frontend/src/features/chat-stage/ChatStagePage.tsx
Comment thread frontend/src/features/chat-stage/ChatStagePage.tsx
@RachelForster

Copy link
Copy Markdown
Owner

这一块儿之后可能要重构下,我们现在ChatVisionService里对文件和图片进行区分,边界有点不明确

@RachelForster
RachelForster merged commit b7710a8 into RachelForster:main Jul 25, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chat attachment picker submits paths outside the allowed directory

2 participants