feat: add lrzsz ZMODEM transfers#309
Conversation
zerx-lab
left a comment
zerx-lab
left a comment
There was a problem hiding this comment.
整体判断倾向：Request Changes。ZMODEM 核心逻辑实现扎实，但发现一个会导致用户数据静默丢失的 bug，需要在合并前解决。Diff 超过 23000 行（大部分为 rustfmt 格式化），本次未逐行覆盖全部内容，专注在新增功能文件上抽样 review。
阻塞问题
1. DownloadSession：sz 中止时已完成的文件被静默删除（见 inline comment，zmodem.rs ~748 行）
详见 inline comment。核心问题：Event::Aborted 只清理了 current_file，但 completed_files 里已暂存（尚未 rename 到最终路径）的 NamedTempFile 在 DownloadSession 析构时会被一并删除。用户会看到 UI 显示"文件 A/B 已接收完成"，但磁盘上找不到这些文件。
建议
2. terminal_size_element.rs：dispatch_event 顺序调换（见 inline comment，~97 行）
此处将拖放事件处理移到 child.dispatch_event 之前，并去掉了旧的 !handled_by_child 保护。逻辑上合理（让终端拦截文件拖放而不是让子元素消费），但属于行为变更，建议在 PR 描述里补充说明意图，并确认各平台的拖放行为均已测试。
3. 上传文件大小硬限 ~4 GB
UploadSession::start_current_file 中 u32::try_from(file.size) 意味着单文件超过约 4 GB 时直接报错。这是 zmodem2 crate 的 Position::new(u32) 限制，本身无误。建议在用户可见的错误信息或文档里明确说明此限制，避免用户困惑（报错信息当前是内部描述，用户体验不佳）。
看起来不错的地方
sanitize_wire_file_name正确拦截了路径遍历（split on/和\\，拒绝./..）；raw_upload_file_specs在客户端去重了同名文件，防止远端覆盖。validate_raw_download_limits对单文件大小（32 GB）和总量（128 GB）均有上界检查，且用checked_add防止溢出。remote_raw_upload_command中 shell 脚本通过shell_words::quote对文件名和路径做了正确转义，无注入风险；远端脚本里的case "$name" in ""|.|..|*/*) exit 2;;提供了第二道防御。- 超时机制（
SSH_ZMODEM_INITIAL_OUTPUT_TIMEOUT = 15s，总超时 1h）覆盖了远端卡死的场景，取消信号通过原子 bool + try_kill 传播的方式也处理得比较完整。 ZmodemDetector的"末尾前缀保留"逻辑（longest_zmodem_prefix_suffix_len）处理了 ZMODEM 握手跨 PTY 读取边界的情况，细节到位。
— 由 Claude Routine 自动生成；如需复评请 @ 维护者人工触发。
Generated by Claude Code
| return Ok(true); | ||
| } | ||
| Event::Aborted => { | ||
| self.current_file.take(); |
There was a problem hiding this comment.
Bug：多文件 sz 下载中止时，已完成的文件被静默丢弃
当 sz 发送多个文件时，Event::FileCompleted 会把完成的 NamedTempFile 从 current_file 移入 completed_files，但不会立即 rename 到目标路径（rename 只在 Event::SessionCompleted 触发的 persist_completed_files() 里发生）。
这里的 Event::Aborted 分支只调用 self.current_file.take()（正确地清理了正在传输的文件），但没有处理 self.completed_files：
DownloadSession随即被 event loop 丢弃。completed_files里所有NamedTempFile的析构函数运行，临时文件被删除。- 用户在 UI 里看到"文件 A 已完成、文件 B 已完成、文件 C 失败"，但 A 和 B 实际上不在磁盘上。
建议：在 Aborted 分支里决策：要么调用 persist_completed_files() 保留已完成的文件（更符合用户预期），要么在 UI/文档里明确说明"传输中止时所有文件均被丢弃"（当前行为则应去掉 FileCompleted 事件，否则误导用户）。若选择保留已完成文件，需处理 persist_completed_files 可能返回的错误。
Generated by Claude Code
| }; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Question：dispatch_event 顺序调换是否有意为之？
旧逻辑：先调用 self.child.dispatch_event，若 child 未消费则处理拖放事件（DragFiles/DragFileExit/DragAndDropFiles）。
新逻辑：拖放事件优先处理（命中则 return true），之后才调用 self.child.dispatch_event。
对于 DragAndDropFiles（触发 TerminalAction::DragAndDropFiles → ZMODEM 上传），child 现在不再收到这个事件。如果这是为了让终端本体不干扰文件拖放，逻辑上合理；但如果有子元素在旧代码中确实需要消费拖放事件（例如文件预览组件），此变更可能造成回归。
能否在 PR 描述里补充说明这个顺序调换的原因，以及确认各平台（macOS/Linux/Windows）下拖放行为均已人工测试过？
Generated by Claude Code
3 participants
Description
Adds lrzsz/ZMODEM file transfer support for terminal sessions.
rz/szZMODEM handshakes and drives upload/download flows from the terminal.rz, dragging files onto SSH terminals to start uploads, and choosing a local save directory forszdownloads.rz/szdoes not depend on SFTP, including SSH Manager auth context reuse where available.lrzszfeature for release bundle builds.Testing
Added/updated unit and regression coverage around ZMODEM parsing, PTY event-loop transfer handling, SSH drag/drop upload routing, cancellation cleanup, duplicate upload names, and raw transfer bounds.
git diff --check upstream/main..HEAD-> passcargo check->passServer API dependencies
N/A - this is client-side terminal transfer support and does not require a server API change.
Agent Mode
Changelog Entries for Stable
CHANGELOG-NEW-FEATURE: Add lrzsz/ZMODEM file transfers for terminal sessions, including
rzuploads andszdownloads.