Expose local image paths to models#25944
Conversation
![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: 4bd82b8913
ℹ️ 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".
| content: vec![ | ||
| ContentItem::InputText { | ||
| text: codex_protocol::models::local_image_open_tag_text(/*label_number*/ 1), | ||
| text: format!(r#"<image name=[Image #1] path="{}">"#, abs_path.display()), |
There was a problem hiding this comment.
why don't you use your new method?
There was a problem hiding this comment.
the function is set to private, we can set to public and use it here
| let path = path | ||
| .display() | ||
| .to_string() | ||
| .replace('&', "&") |
There was a problem hiding this comment.
we don't need this escaping
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd92f422aa
ℹ️ 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 label = local_image_label_text(label_number); | ||
| format!("{LOCAL_IMAGE_OPEN_TAG_PREFIX}{label}{LOCAL_IMAGE_OPEN_TAG_SUFFIX}") | ||
| let path = path.display(); | ||
| format!("{LOCAL_IMAGE_OPEN_TAG_PREFIX}{label} path=\"{path}\"{LOCAL_IMAGE_OPEN_TAG_SUFFIX}") |
There was a problem hiding this comment.
For local images under very long absolute paths, this writes the entire path into a new model-visible InputText item, and the string has no hard cap before it becomes part of the request context. The model-context review rule requires every injected item to be bounded and flags new individual items that can exceed 1k tokens as P0; see AGENTS.md lines 87-90. Please truncate or otherwise cap only the displayed path while keeping the full path in non-model state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
extremely unlikely for the path (codex home + thread_id + call_id) to get to any level that is concernin
| let label = local_image_label_text(label_number); | ||
| format!("{LOCAL_IMAGE_OPEN_TAG_PREFIX}{label}{LOCAL_IMAGE_OPEN_TAG_SUFFIX}") | ||
| let path = path.display(); | ||
| format!("{LOCAL_IMAGE_OPEN_TAG_PREFIX}{label} path=\"{path}\"{LOCAL_IMAGE_OPEN_TAG_SUFFIX}") |
There was a problem hiding this comment.
When the local image filename contains characters such as " or > that are valid on Unix paths, this emits a malformed model-visible tag and lets part of the path be interpreted as ordinary prompt text rather than the path attribute. For example, the added test path /tmp/a&"<b>.png produces path="/tmp/a&"<b>.png", so please escape or encode attribute delimiters before building this wrapper.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
the local image filename will not ocntain these characters..
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d77965f07
ℹ️ 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 label = local_image_label_text(label_number); | ||
| format!("{LOCAL_IMAGE_OPEN_TAG_PREFIX}{label}{LOCAL_IMAGE_OPEN_TAG_SUFFIX}") | ||
| let path = path.display(); | ||
| format!("{LOCAL_IMAGE_OPEN_TAG_PREFIX}{label} path=\"{path}\"{LOCAL_IMAGE_OPEN_TAG_SUFFIX}") |
There was a problem hiding this comment.
This sends the full Path::display() string into a model-visible InputText without any hard cap. The context rule requires bounded injected context and calls out any new item that can exceed 1k tokens as P0 (AGENTS.md lines 87-90); fresh evidence since the earlier thread is that UserInput::LocalImage also comes from CLI/exec/app-server user-supplied paths, not only generated Codex home/thread/call paths, so a valid long path can still inflate every replayed request and churn cache. Please truncate or otherwise bound the displayed path while retaining the full path only in non-model state.
Useful? React with 👍 / 👎.
2 participants
Why
Local image attachments include image bytes, but the adjacent model-visible label omits the source path. Exposing the path lets model-selected workflows refer back to the intended local image explicitly.
What changed
pathattribute in model-visible local image opening tags.Validation
just fmtjust test -p codex-protocoljust test -p codex-core skips_local_image_label_textjust test -p codex-core copy_paste_local_image_persists_rollout_request_shapegit diff --check