Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a14abcb915
ℹ️ 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".
| - metadata | ||
| - rust-binaries | ||
| - bash-linux | ||
| - bash-darwin | ||
| - zsh-linux |
There was a problem hiding this comment.
Restore Rust binaries as package prerequisites
The package job no longer depends on any Rust binary build, so its staged vendor/ payload is assembled only from Bash/Zsh artifacts. The published launcher still requires vendor/<target>/codex-exec-mcp-server and codex-execve-wrapper at startup (shell-tool-mcp/src/index.ts throws Required binary missing when they are absent), so this workflow now produces npm tarballs that install but fail immediately at runtime on all platforms.
Useful? React with 👍 / 👎.
b1f51a2 to
fcf0efa
Compare
Why
We already plan to remove the shell-tool MCP path, and doing that cleanup first makes the follow-on
shell-escalationwork much simpler.This change removes the last remaining reason to keep
codex-rs/exec-serveraround by moving thecodex-execve-wrapperbinary and shared shell test fixtures to the crates/tests that now own that functionality.What Changed
Delete
codex-rs/exec-serverexec-servercrate, including the MCP server binary, MCP-specific modules, and its test support/test suiteexec-serverfrom thecodex-rsworkspace and updateCargo.lockMove
codex-execve-wrapperintocodex-rs/shell-escalationshell-escalation(src/unix/execve_wrapper.rs)codex-execve-wrapperbinary entrypoint undershell-escalation/src/bin/shell-escalationexports/module layout so the wrapper entrypoint is hosted thereexec-servertoshell-escalation/README.mdMove shared shell test fixtures to
app-serverbash/zshtest fixtures fromexec-server/tests/suite/toapp-server/tests/suite/app-serverzsh-fork tests to reference the new fixture pathsKeep
shell-tool-mcpas a shell-assets package.github/workflows/shell-tool-mcp.ymlpackaging so the npm artifact contains only patched Bash/Zsh payloads (no Rust binaries)shell-tool-mcp/package.json,shell-tool-mcp/src/index.ts, and docs to reflect the shell-assets-only package shapeshell-tool-mcp-ci.ymldoes not need changes because it is already JS-onlyVerification
cargo shearcargo clippy -p codex-shell-escalation --testsjust clippy