Conversation
|
Superseded by #14989 after splitting the stack; closing the duplicate PR created during resubmission. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40ba3162cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let output = execute_env_bytes(exec_request, /*stdout_stream*/ None) | ||
| .await | ||
| .map_err(|error| map_exec_error(path, error))?; | ||
| if output.exit_code != 0 { | ||
| return Err(parse_helper_failure( | ||
| path, | ||
| output.exit_code, | ||
| &output.stderr.text, | ||
| &output.stdout.text, | ||
| )); | ||
| } | ||
|
|
||
| Ok(output) |
There was a problem hiding this comment.
view_image now trusts the bytes returned by execute_env_bytes, but that path still goes through the generic exec collector in core/src/exec.rs (EXEC_OUTPUT_MAX_BYTES plus read_capped), which retains only the first 1 MiB of stdout. Any PNG/JPEG larger than that will be silently truncated here before local_image_content_items_with_label_number tries to decode it, so previously valid large images now fail or produce corrupted uploads.
Useful? React with 👍 / 👎.
| let image_bytes = sandboxed_fs::read_file(&session, &turn, abs_path.as_path()) | ||
| .await | ||
| .map_err(|error| { | ||
| FunctionCallError::RespondToModel(render_view_image_read_error( | ||
| abs_path.as_path(), | ||
| &error, | ||
| )) | ||
| })?; |
There was a problem hiding this comment.
The previous implementation checked metadata.is_file before reading; this version passes every readable path straight to the helper. In fs-ops/src/runner.rs, std::io::copy reads until EOF, so in full-access or unrestricted sessions a path like /dev/zero or a FIFO will block until the 30s helper timeout instead of immediately returning image path ... is not a file.
Useful? React with 👍 / 👎.
1 participant
Stack created with Sapling. Best reviewed with ReviewStack.