fix(code-executors): preserve collected binary files - #344
Conversation
raychen911
commented
Sep 23, 2026
raychen911
commented
- 修复二进制输出问题
AI Code Review审查结论不通过 审查范围：b80d43a..17bd845（fix(code-executors): preserve collected binary files），5 个变更文件（2 个 SDK 源文件 + 3 个测试文件），84 增 18 删。计划意图：消除 collect()/collect_outputs() 对二进制文件用 utf-8 replace 解码造成的字节破坏，改为 content_base64 + get_bytes() 无损表达。实现层面（_encode_collected_content 双表示、CodeFile/ManifestFileRef 各增 get_bytes、3 个新测试）正确；三个后端（local/container/cube）的 collect/collect_outputs 均路由到被修复的共享辅助函数，无残留 errors=replace 的 collect 路径。核心缺陷：修复只做到编码端，全仓 grep 证实 get_bytes()/content_base64 除测试外零调用——skill_run 的 _prepare_outputs 重建 CodeFile 丢弃 content_base64、_to_run_file 与 _save_artifacts 只读 .content、三个 put_files 后端仍写 file.content or b''，二进制字节在所有产品路径上依旧静默丢失，text/* MIME 的非 UTF-8 文本从"乱码可见"退化为"内容全空"，collect→put 往返写出 0 字节文件；新 API 另有 validate=True 解码崩溃与截断文件 size_bytes 不一致两个边界缺陷；测试只覆盖共享辅助函数与小文件，未覆盖任何消费端、截断与 save_as_artifacts 二进制分支。门禁结论：FAILED。 发现的问题严重
问题： 本修复只覆盖"编码端"，没有任何消费端读取 触发条件： 收集任何非 UTF-8 文件（PNG/PDF/GBK 编码文本等），并发生以下任一情况：(a) 经 skill_run 的 实际影响： 二进制字节在每一条产品路径上依旧静默丢失，且比改动前更隐蔽： 修正方向： 在 中等
问题： 触发条件： 调用方以 实际影响： 二进制恢复的唯一公开 API 在输入非规范 base64 时抛异常而非返回字节，调用方（agent 工具循环、JSON 往返）崩溃；与"无损恢复"的契约相悖。 修正方向： 去掉 中等
问题： 触发条件： (a) 收集含一个坏字节的文本文件（日志/CSV 混入 0xFF）；(b) 收集合法 UTF-8 但本质为二进制的文件并被 实际影响： 场景 (a) 下此前模型可见的文本（仅一个 U+FFFD）现在整文件变成空串加 base64 块，文本内容在 prompt/工具结果中消失（ 修正方向： 以"内容是否可以安全作为文本呈现"（如结合 MIME 与 NUL/控制字符检测）替代单纯的 UTF-8 合法性判定，或至少对截断文件（ 中等
问题： 触发条件： 收集超过 4 MiB 的二进制文件（cube/local/container 的 fetch 都按上限截取），随后调用方按 实际影响： 新公开的"无损"API 对超限文件静默返回部分字节：base64 本身解码成功、无任何异常，调用方得到与 修正方向： 较低
问题： 新增测试只断言了共享辅助函数与"编码端"行为（ 触发条件： 运行 实际影响： 测试给出"修复完成"的误导性信号，掩盖本提交未达成的核心目标； 修正方向： 增加端到端测试：skill_run |
| def get_bytes(self) -> bytes: | ||
| """Return the collected content without losing binary bytes.""" | ||
| if self.content_base64: | ||
| return base64.b64decode(self.content_base64, validate=True) | ||
| return self.content.encode("utf-8") |
There was a problem hiding this comment.
问题： get_bytes() 使用 base64.b64decode(self.content_base64, validate=True)：validate=True 在遇到非标准 base64 字符、缺 = 填充或含空白/换行（如经过 JSON 重新格式化、URL-safe 变体或手工构造）时直接抛出 binascii.Error（ValueError 子类），且没有任何调用方捕获。content_base64 是新增的公开模型字段，CodeFile/ManifestFileRef 均可由外部按 JSON 反序列化构造。
触发条件： 调用方以 CodeFile.model_validate(json.loads(...)) 或手工构造方式传入非规范 base64（未填充、含换行、urlsafe_b64encode 输出），随后调用 get_bytes()。内部 _encode_collected_content 用标准 b64encode（始终填充、无空白）不受影响，但该路径恰好覆盖不了外部构造的取值。
实际影响： 二进制恢复的唯一公开 API 在输入非规范 base64 时抛异常而非返回字节，调用方（agent 工具循环、JSON 往返）崩溃；与"无损恢复"的契约相悖。
修正方向： 去掉 validate=True（b64decode 默认可容忍空白并对非法字符抛错仍可捕获），或先做 re.sub(r'\s+', '', s) 规范化，并对 binascii.Error 给出明确的 ValueError 与降级路径；同时为非法输入补充单元测试。
| def _encode_collected_content(data: bytes) -> Tuple[str, str]: | ||
| """Return lossless text and Base64 representations for collected bytes.""" | ||
| try: | ||
| return data.decode("utf-8"), "" | ||
| except UnicodeDecodeError: | ||
| return "", base64.b64encode(data).decode("ascii") |
There was a problem hiding this comment.
问题： _encode_collected_content 用严格 data.decode('utf-8') 判定"文本 vs 二进制"，仅凭 UTF-8 合法性做界：含单个非法字节的文本文件整体落入 base64 分支（content 变空串）；而合法 UTF-8 的二进制（含 NUL 的 UTF-16、全 ASCII 二进制等）又留在 content 里。旧行为是 errors='replace' 保证任意数据都有可显示的 content。
触发条件： (a) 收集含一个坏字节的文本文件（日志/CSV 混入 0xFF）；(b) 收集合法 UTF-8 但本质为二进制的文件并被 text/* 误判 MIME。
实际影响： 场景 (a) 下此前模型可见的文本（仅一个 U+FFFD）现在整文件变成空串加 base64 块，文本内容在 prompt/工具结果中消失（_should_inline_file_content 恰好在 content 为空时返回 True，使空内容被"内联"）；场景 (b) 下 NUL 字符字符串进入 content，依赖 content 纯文本的消费者（拼 prompt、日志）处理到裸 NUL/控制字符。
修正方向： 以"内容是否可以安全作为文本呈现"（如结合 MIME 与 NUL/控制字符检测）替代单纯的 UTF-8 合法性判定，或至少对截断文件（raw_size > len(data)）显式走 base64 分支；并为单坏字节文本文件补测试。
| def get_bytes(self) -> bytes: | ||
| """Return the collected content without losing binary bytes.""" | ||
| if self.content_base64: | ||
| return base64.b64decode(self.content_base64, validate=True) | ||
| return self.content.encode("utf-8") |
There was a problem hiding this comment.
问题： get_bytes() 返回的是截断后前缀（content_base64 只编码了按 max_read_size（4 MiB）读取的前缀），而 size_bytes 报告完整 raw_size；CodeFile.truncated 为 True 时二者不一致，且没有任何消费方在使用 get_bytes() 前检查 truncated。
触发条件： 收集超过 4 MiB 的二进制文件（cube/local/container 的 fetch 都按上限截取），随后调用方按 size_bytes 或按"base64 解码即完整数据"的假设处理 get_bytes() 结果（哈希、上传、artifact 保存）。
实际影响： 新公开的"无损"API 对超限文件静默返回部分字节：base64 本身解码成功、无任何异常，调用方得到与 size_bytes 不符的截断数据且无从察觉（truncated 标志被忽略），可能将半截文件写入 artifact 或下游存储。
修正方向： get_bytes() 至少在 truncated=True 时抛出可识别的异常或返回 (bytes, truncated) 元组，并把"截断文件"分支纳入 test_base_workspace_fs_collect.py 的覆盖（现有新增测试只覆盖小文件）。
| files = await fs.collect(ws, ["*.png"]) | ||
| assert len(files) == 1 | ||
| # The raw bytes should be recoverable. They are not: utf-8 | ||
| # replace turns \x80 into U+FFFD, and re-encoding does not roundtrip. | ||
| assert files[0].content.encode("utf-8") == binary, ( | ||
| "binary file silently corrupted by utf-8 replace" | ||
| ) | ||
| assert files[0].content == "" | ||
| assert files[0].content_base64 | ||
| assert files[0].get_bytes() == binary |
There was a problem hiding this comment.
问题： 新增测试只断言了共享辅助函数与"编码端"行为（content_base64 非空、get_bytes() 等于原字节），没有覆盖任何消费端（_skill_run.py 的 _to_run_file/_prepare_outputs/_save_artifacts、三个后端 put_files），而消费端恰恰全部未迁移（见 _base_workspace_runtime.py:171 的严重问题），因此测试套件全绿也无法发现"收集无损但产品路径丢失字节"的缺陷；test_bug9 还断言了 content == '' 这一实现细节（未来若改为对二进制仍填充非空 content 会误报失败）。
触发条件： 运行 test_bug9_collect_preserves_binary_bytes 及相关新增测试时全部通过，但真实 skill_run/跨 workspace 拷贝场景仍丢字节。
实际影响： 测试给出"修复完成"的误导性信号，掩盖本提交未达成的核心目标；save_as_artifacts 二进制、截断二进制、collect→put 往返均无测试。
修正方向： 增加端到端测试：skill_run outputs.inline 收集二进制并断言 SkillRunOutput.output_files 可恢复字节（修复 _skill_run.py 后再加）；cube put_files 往返测试；save_as_artifacts 二进制文件测试；同时把断言从 content == '' 改为行为不变量（get_bytes() == binary）。
17bd845 to
7a0af21
Compare
AI Code Review审查结论通过 审查范围： 计划符合性：核心目标在收集层达成—— 主要风险：变更引入的行为回归集中在 测试充分性：收集层新增测试充分，但 门禁结论：无高置信的 SEVERE 级缺陷（经典二进制文件在内联被遮挡/空产物保存方面在基线版本即已存在，非本变更引入）；存在 2 个 MODERATE 级回归和 3 个 LOW 级问题，均给出可执行修正方向。 发现的问题中等
问题： 触发条件： ① 文本 MIME 文件含非 UTF-8 字节（如 GBK/latin-1 编码的 实际影响： 变更前这些文件的 修正方向： 不再以解码成败判定二进制：对解码失败的文本 MIME 文件保留 中等
问题： 本变更新增的 触发条件： 任意经 实际影响： 工具返回的 修正方向： 给 较低
问题： 新增的 触发条件： 实际影响： 前者直接抛出 修正方向： 为 较低
问题： 触发条件： 任意产生二进制文件的 实际影响： 每个二进制文件多出约 1.33 倍字节的无效编码工作与峰值内存（编码期间同时持有原始 bytes 与 base64 str），在批量收集（如 auto-export 20 个文件）时放大为数百 MiB 的临时内存占用；同时 修正方向： 与 较低
问题： 本变更对 触发条件： 需要新增覆盖场景：① 实际影响： 上述 MODERATE 级回归（文本内容消失、空产物保存）当前无任何自动测试兜底，后续改动极易将其固化或再次引入。 修正方向： 在 |
| def _encode_collected_content(data: bytes) -> Tuple[str, str]: | ||
| """Return lossless text and Base64 representations for collected bytes.""" | ||
| try: | ||
| return data.decode("utf-8"), "" | ||
| except UnicodeDecodeError: | ||
| return "", base64.b64encode(data).decode("ascii") |
There was a problem hiding this comment.
问题： _encode_collected_content 以"能否通过严格 UTF-8 解码"作为二进制判定标准，把所有解码失败的内容（包括文本文件）路由进 content="" + content_base64，而下游 _should_inline_file_content 的新守卫（_skill_run.py:131）对任何设置了 content_base64 的文件直接返回 False，导致原本内联可见的文本内容从 skill_run 响应中消失。
触发条件： ① 文本 MIME 文件含非 UTF-8 字节（如 GBK/latin-1 编码的 .csv/.txt，detect_content_type 按扩展名返回 text/plain）；② 超过 MAX_READ_SIZE_BYTES（4 MiB）的 UTF-8 文本文件被 fetcher 按字节截断（local read_bytes()[:max_bytes]、cube data[:max_bytes]、container f.read(max_bytes)），截断点落在一个多字节字符中间导致解码失败。
实际影响： 变更前这些文件的 content 为可读的部分文本（含 U+FFFD 替换符），模型可看到内容；变更后 skill_run 的 output_files[].content、primary_output 均为空，模型把有效文本文件误读为空文件，且 _filter_failed_empty_outputs（失败运行时 f.content 为空 + declarative 路径 size_bytes 恒为 0）会直接丢弃该文件并提示"empty outputs"。
修正方向： 不再以解码成败判定二进制：对解码失败的文本 MIME 文件保留 errors="replace" 的部分文本到 content 同时附加 content_base64 供无损恢复；或在 _should_inline_file_content 中当 content 为空但 content_base64 非空且 MIME 为文本类型时，改为允许内联 base64 解码后的内容。
| def get_bytes(self) -> bytes: | ||
| """Return the collected content without losing binary bytes.""" | ||
| if self.content_base64: | ||
| return base64.b64decode(self.content_base64, validate=True) | ||
| return self.content.encode("utf-8") |
There was a problem hiding this comment.
问题： 新增的 get_bytes()（CodeFile 与 ManifestFileRef 双份实现）使用 base64.b64decode(self.content_base64, validate=True)：对任何缺 padding（如 'gA'）或含非 base64 字母字符的输入抛出未捕获的 binascii.Error；且对 truncated=True 的文件静默返回被截断的字节前缀，无任何提示——ManifestFileRef 没有 truncated/size_bytes 字段，declarative 路径上 getattr(fr, "truncated", False) 恒为 False。
触发条件： CodeFile/ManifestFileRef 是公开导出的 SDK 类型（code_executors/__init__.py），外部调用方从协议数据/JSON 反序列化时构造了非规范 base64（如 JSON 序列化中 = 被转义、换行折叠、截断的流式 payload），或调用 get_bytes() 恢复一个被 4 MiB 上限截断的文件。
实际影响： 前者直接抛出 binascii.Error: Incorrect padding 使调用方崩溃（内部 b64encode 产物总是规范 padding，故仅外部构造可达）；后者静默返回不完整字节，按 get_bytes() 重建的文件损坏且无法感知截断。
修正方向： 为 content_base64 增加 pydantic 校验/规范化（或 get_bytes() 内捕获 binascii.Error 并转为可读错误），并在 ManifestFileRef 补齐 truncated/size_bytes 字段或在 get_bytes() 的文档/返回中显式暴露截断状态；建议两处 get_bytes() 抽为共享函数避免双份漂移。
| content, content_base64 = _encode_collected_content(data) | ||
| out.append( | ||
| CodeFile( | ||
| name=rel, | ||
| content=data.decode("utf-8", errors="replace"), | ||
| content=content, | ||
| content_base64=content_base64, |
There was a problem hiding this comment.
问题： _build_code_files（collect() 路径）对每个非 UTF-8 文件无条件执行 base64 编码（_base_workspace_runtime.py:171），即使调用方从不读取内容（skill_run 的 _to_run_file 立即丢弃）；而 _build_manifest_output（:288）做到了仅在 spec.inline 时编码。同类逻辑却不对称。
触发条件： 任意产生二进制文件的 collect() 调用——默认最多 100 个文件、每个上限 4 MiB，单文件 base64 结果约 5.6 MiB 的 str 常驻 CodeFile 直至请求结束。
实际影响： 每个二进制文件多出约 1.33 倍字节的无效编码工作与峰值内存（编码期间同时持有原始 bytes 与 base64 str），在批量收集（如 auto-export 20 个文件）时放大为数百 MiB 的临时内存占用；同时 total_bytes 预算按原始字节统计，模型侧实际驻留的 base64 文本约超出 1.33 倍。
修正方向： 与 _build_manifest_output 对齐：为 _build_code_files 增加惰性编码（如提供 content_base64: Optional[str] = None 由消费方按需解码），或在收集层不生成 base64、仅保留原始 bytes 的 get_bytes() 来源。
3 participants
