Commit Graph

697 Commits

Author SHA1 Message Date
Michael Bolin
ab1ea9f5d9 Merge 306d0b5618 into sapling-pr-archive-bolinfest 2025-05-05 09:57:45 -07:00
Michael Bolin
306d0b5618 feat: mcp-client 2025-05-05 09:57:35 -07:00
Michael Bolin
2b72d05c5e feat: make Codex available as a tool when running it as an MCP server (#811)
This PR replaces the placeholder `"echo"` tool call in the MCP server
with a `"codex"` tool that calls Codex. Events such as
`ExecApprovalRequest` and `ApplyPatchApprovalRequest` are not handled
properly yet, but I have `approval_policy = "never"` set in my
`~/.codex/config.toml` such that those codepaths are not exercised.

The schema for this MPC tool is defined by a new `CodexToolCallParam`
struct introduced in this PR. It is fairly similar to `ConfigOverrides`,
as the param is used to help create the `Config` used to start the Codex
session, though it also includes the `prompt` used to kick off the
session.

This PR also introduces the use of the third-party `schemars` crate to
generate the JSON schema, which is verified in the
`verify_codex_tool_json_schema()` unit test.

Events that are dispatched during the Codex session are sent back to the
MCP client as MCP notifications. This gives the client a way to monitor
progress as the tool call itself may take minutes to complete depending
on the complexity of the task requested by the user.

In the video below, I launched the server via:

```shell
mcp-server$ RUST_LOG=debug npx @modelcontextprotocol/inspector cargo run --
```

In the video, you can see the flow of:

* requesting the list of tools
* choosing the **codex** tool
* entering a value for **prompt** and then making the tool call

Note that I left the other fields blank because when unspecified, the
values in my `~/.codex/config.toml` were used:


https://github.com/user-attachments/assets/1975058c-b004-43ef-8c8d-800a953b8192

Note that while using the inspector, I did run into
https://github.com/modelcontextprotocol/inspector/issues/293, though the
tip about ensuring I had only one instance of the **MCP Inspector** tab
open in my browser seemed to fix things.
2025-05-05 07:16:19 -07:00
Michael Bolin
cf54cd1898 Merge ea3e4e126e into sapling-pr-archive-bolinfest 2025-05-04 23:05:27 -07:00
Michael Bolin
ea3e4e126e feat: initial work by Codex to create Codex MCP tool call 2025-05-04 22:33:44 -07:00
Michael Bolin
3e24b50a5d merge commit for archive created by Sapling 2025-05-04 17:14:11 -07:00
Michael Bolin
f73f053290 feat: initial work by Codex to create Codex MCP tool call 2025-05-04 17:13:24 -07:00
Michael Bolin
befb74af33 merge commit for archive created by Sapling 2025-05-04 17:05:35 -07:00
Michael Bolin
0c56a7826a feat: initial work by Codex to create Codex MCP tool call 2025-05-04 17:05:25 -07:00
Michael Bolin
28680d0b63 merge commit for archive created by Sapling 2025-05-04 16:36:33 -07:00
Michael Bolin
01ec277a50 feat: initial work by Codex to create Codex MCP tool call 2025-05-04 16:36:25 -07:00
Michael Bolin
c5db9e1db7 merge commit for archive created by Sapling 2025-05-04 14:54:47 -07:00
Michael Bolin
b5173536d1 feat: initial work by Codex to create Codex MCP tool call 2025-05-04 14:54:41 -07:00
Michael Bolin
e85b02e7bd Merge 7a3ebc6b03 into sapling-pr-archive-bolinfest 2025-05-04 13:08:42 -07:00
Michael Bolin
7a3ebc6b03 feat: initial work by Codex to create Codex MCP tool call 2025-05-04 13:08:35 -07:00
Michael Bolin
5d924d44cf fix: ensure apply_patch resolves relative paths against workdir or project cwd (#810)
https://github.com/openai/codex/pull/800 kicked off some work to be more
disciplined about honoring the `cwd` param passed in rather than
assuming `std::env::current_dir()` as the `cwd`. As part of this, we
need to ensure `apply_patch` calls honor the appropriate `cwd` as well,
which is significant if the paths in the `apply_patch` arg are not
absolute paths themselves. Failing that:

- The `apply_patch` function call can contain an optional`workdir`
param, so:
- If specified and is an absolute path, it should be used to resolve
relative paths
- If specified and is a relative path, should be resolved against
`Config.cwd` and then any relative paths will be resolved against the
result
- If `workdir` is not specified on the function call, relative paths
should be resolved against `Config.cwd`

Note that we had a similar issue in the TypeScript CLI that was fixed in
https://github.com/openai/codex/pull/556.

As part of the fix, this PR introduces `ApplyPatchAction` so clients can
deal with that instead of the raw `HashMap<PathBuf,
ApplyPatchFileChange>`. This enables us to enforce, by construction,
that all paths contained in the `ApplyPatchAction` are absolute paths.
2025-05-04 12:32:51 -07:00
Michael Bolin
76ed513b44 merge commit for archive created by Sapling 2025-05-04 12:21:21 -07:00
Michael Bolin
3c5104374f fix: ensure apply_patch resolves relative paths against workdir or project cwd 2025-05-04 12:21:16 -07:00
Michael Bolin
f53d9f73ac Merge f60e43a101 into sapling-pr-archive-bolinfest 2025-05-04 12:20:51 -07:00
Michael Bolin
f60e43a101 fix: ensure apply_patch resolves relative paths against workdir or project cwd 2025-05-04 12:20:40 -07:00
Michael Bolin
a134bdde49 fix: is_inside_git_repo should take the directory as a param (#809)
https://github.com/openai/codex/pull/800 made `cwd` a property of
`Config` and made it so the `cwd` is not necessarily
`std::env::current_dir()`. As such, `is_inside_git_repo()` should check
`Config.cwd` rather than `std::env::current_dir()`.

This PR updates `is_inside_git_repo()` to take `Config` instead of an
arbitrary `PathBuf` to force the check to operate on a `Config` where
`cwd` has been resolved to what the user specified.
2025-05-04 11:39:10 -07:00
Michael Bolin
1d4e6e275e merge commit for archive created by Sapling 2025-05-04 11:22:45 -07:00
Michael Bolin
92c5135060 fix: is_inside_git_repo should take the directory as a param 2025-05-04 11:22:41 -07:00
Michael Bolin
f8dae3f12c merge commit for archive created by Sapling 2025-05-04 11:22:35 -07:00
Michael Bolin
154fb92f02 fix: is_inside_git_repo should take the directory as a param 2025-05-04 11:22:31 -07:00
Michael Bolin
7fd16e13ec merge commit for archive created by Sapling 2025-05-04 11:21:58 -07:00
Michael Bolin
bb06b80404 fix: is_inside_git_repo should take the directory as a param 2025-05-04 11:21:53 -07:00
Michael Bolin
306f81f39c Merge 991bb2db44 into sapling-pr-archive-bolinfest 2025-05-04 11:17:45 -07:00
Michael Bolin
991bb2db44 fix: is_inside_git_repo should take the directory as a param 2025-05-04 11:17:29 -07:00
Michael Bolin
cd12f0c24a fix: TUI should use cwd from Config (#808)
https://github.com/openai/codex/pull/800 made `cwd` a property of
`Config`, so the TUI should use this instead of running
`std::env::current_dir()`.
2025-05-04 11:12:40 -07:00
Michael Bolin
29ebd0dc85 Merge 506b66e761 into sapling-pr-archive-bolinfest 2025-05-04 11:07:28 -07:00
Michael Bolin
506b66e761 fix: TUI should use cwd from Config 2025-05-04 11:07:18 -07:00
Michael Bolin
fb6f104765 Merge 5662a708e2 into sapling-pr-archive-bolinfest 2025-05-04 11:06:55 -07:00
Michael Bolin
5662a708e2 fix: TUI should use cwd from Config 2025-05-04 11:06:47 -07:00
Michael Bolin
421e159888 feat: make cwd a required field of Config so we stop assuming std::env::current_dir() in a session (#800)
In order to expose Codex via an MCP server, I realized that we should be
taking `cwd` as a parameter rather than assuming
`std::env::current_dir()` as the `cwd`. Specifically, the user may want
to start a session in a directory other than the one where the MCP
server has been started.

This PR makes `cwd: PathBuf` a required field of `Session` and threads
it all the way through, though I think there is still an issue with not
honoring `workdir` for `apply_patch`, which is something we also had to
fix in the TypeScript version: https://github.com/openai/codex/pull/556.

This also adds `-C`/`--cd` to change the cwd via the command line.

To test, I ran:

```
cargo run --bin codex -- exec -C /tmp 'show the output of ls'
```

and verified it showed the contents of my `/tmp` folder instead of
`$PWD`.
2025-05-04 10:57:12 -07:00
Michael Bolin
2e1aa83cdb merge commit for archive created by Sapling 2025-05-04 09:36:21 -07:00
Michael Bolin
518023dbd9 feat: make cwd a required field of Config so we stop assuming std::env::current_dir() in a session 2025-05-04 09:36:16 -07:00
Michael Bolin
f6b05ac326 merge commit for archive created by Sapling 2025-05-04 09:26:45 -07:00
Michael Bolin
9e3326e81e feat: make cwd a required field of Config so we stop assuming std::env::current_dir() in a session 2025-05-04 09:26:40 -07:00
Michael Bolin
fd437390bf merge commit for archive created by Sapling 2025-05-04 09:26:12 -07:00
Michael Bolin
f9f0490ca2 feat: make cwd a required field of Config so we stop assuming std::env::current_dir() in a session 2025-05-04 09:26:08 -07:00
Michael Bolin
c825ce2c72 merge commit for archive created by Sapling 2025-05-04 09:14:02 -07:00
Michael Bolin
f0ad889ebb feat: make cwd a required field of Config so we stop assuming std::env::current_dir() in a session 2025-05-04 09:13:57 -07:00
Michael Bolin
71db7c5bfe merge commit for archive created by Sapling 2025-05-04 09:12:23 -07:00
Michael Bolin
a9816188c6 feat: make cwd a required field of Config so we stop assuming std::env::current_dir() in a session 2025-05-04 09:12:18 -07:00
Michael Bolin
9514c89a6d merge commit for archive created by Sapling 2025-05-04 09:01:28 -07:00
Michael Bolin
b2a2481516 feat: make cwd a required field of Config so we stop assuming std::env::current_dir() in a session 2025-05-04 09:01:23 -07:00
Michael Bolin
5ba44386a2 Merge 97bc9314ca into sapling-pr-archive-bolinfest 2025-05-04 08:56:28 -07:00
Michael Bolin
97bc9314ca feat: make cwd a required field of Config so we stop assuming std::env::current_dir() in a session 2025-05-04 08:56:24 -07:00
Michael Bolin
e12cf808b5 merge commit for archive created by Sapling 2025-05-04 08:48:37 -07:00