[codex] ChatGPT Library file upload/download hooks#19458
[codex] ChatGPT Library file upload/download hooks#19458
Conversation
lt-oai
force-pushed
the
lt/integrate-lib-app-cleanup-hooks
branch
2 times, most recently
from
7d77a77 to
07f5815
Compare
|
@codex review |
![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: 07f5815ef1
ℹ️ 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".
lt-oai
changed the title
lt-oai
force-pushed
the
lt/integrate-lib-app-cleanup-hooks
branch
from
500f3e0 to
75102a2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75102a2dab
ℹ️ 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".
dylan-hurd-oai
left a comment
dylan-hurd-oai
left a comment
There was a problem hiding this comment.
Couple things to address
| .call_tool(server, tool_name, rewritten_arguments, request_meta) | ||
| .await | ||
| .map_err(|e| format!("tool call error: {e:?}"))?; | ||
| let result = maybe_materialize_codex_apps_file_download_result( |
There was a problem hiding this comment.
I'd love to consider if there's a better approach here
There was a problem hiding this comment.
how do you feel about this refactoring? 9b178ab
Now maybe_materialize_codex_apps_file_download_result is outside of execute_mcp_tool_call(), and wrapped into postprocess_mcp_tool_result_for_openai_files. Still inside handle_approved_mcp_tool_call(). Basically postprocess_mcp_tool_result_for_openai_file() is now symmetrical to rewrite_mcp_tool_arguments_for_openai_files() that was introduced prior to this PR.
This reverts commit 2fcc17e.
…cute_mcp_tool_call
lt-oai
force-pushed
the
lt/integrate-lib-app-cleanup-hooks
branch
from
1e743ba to
b27a245
Compare
3 participants
Summary
connector_openai_librarythrough Codex apps connector filtering./files/process_upload_streamafter the file bytes uploadlibrary_file_idinto uploaded MCP file arguments.materialize_file_downloadkey exists in results meta