Repository navigation
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
omp speaks ACP natively (`omp acp`), so it is wired like OpenCode: a native binary downloaded from its GitHub releases (version pinnable with --agent-version) and a launcher that execs `omp acp`. omp reports its own model, mode and thinking options from session/new.
2aa9064 to
e260d2f
Compare
![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: 2aa90646d1
ℹ️ 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".
| install_opencode(&path, self.platform, options.version.as_deref())? | ||
| } | ||
| AgentId::Amp => install_amp(&path, self.platform, options.version.as_deref())?, | ||
| AgentId::Omp => install_omp(&path, self.platform, options.version.as_deref())?, |
There was a problem hiding this comment.
On a clean host without an omp binary, this installer is unreachable: AgentManager::install calls install_native only when native_required() is true, but that predicate still lists only Claude, Codex and OpenCode. Installation therefore reaches install_agent_process_fallback, which calls resolve_binary and returns BinaryNotFound instead of downloading omp; lazy installation fails the same way. Add Omp to native_required() and cover installation from an empty install directory.
Useful? React with 👍 / 👎.
| }); | ||
| ``` | ||
|
|
||
| `sandbox-agent install-agent omp` downloads the `omp` release binary from [GitHub](https://github.com/can1357/oh-my-pi/releases) (pin one with `--agent-version`) and runs it with `omp acp`, its native ACP mode. No adapter package is needed. |
There was a problem hiding this comment.
This agent-usage page describes the underlying ACP launch mode and adapter implementation, contrary to the Docs Terminology rules in sandbox-agent/AGENTS.md, which prohibit ACP terminology and protocol implementation details outside ACP-specific documentation. Keep this paragraph focused on installation and SDK usage, and move the protocol details to an ACP-specific reference.
AGENTS.md reference: sandbox-agent/AGENTS.md:L13-L15
Useful? React with 👍 / 👎.
| ] | ||
| })], | ||
| // omp returns its model, mode and thinking options from session/new. | ||
| AgentId::Omp => Vec::new(), |
There was a problem hiding this comment.
Even with a preinstalled, authenticated omp, get_v1_agents and get_v1_agent populate configuration only from this function (router.rs:1603-1605,1718-1720); live probing remains disabled, so the Inspector cannot discover omp's models or modes. build_provider_payload_for_opencode also skips agents without a model option (support.rs:674-680), so omp is omitted from /opencode/provider despite its new list entry. Options returned by session/new do not populate either consumer. Provide fallback metadata or wire live configuration into these paths before returning an empty vector.
Useful? React with 👍 / 👎.
Adds oh-my-pi (
omp) as a native agent. omp speaks ACP itself (omp acp), so it is wired like OpenCode: no registry adapter, just the binary and a launcher.AgentId::Omp(omp), inall()andparse, and native-required, soinstall-agent ompinstalls the binary before the launcher.install_omp: downloads the release binary for the platform from omp's GitHub releases (omp-linux-x64,omp-linux-musl-x64,omp-linux-arm64,omp-darwin-arm64,omp-darwin-x64,omp-windows-{x64,arm64}.exe);--agent-versionpins a release, otherwiselatest.omp acp.support.rs: capabilities (plan mode, permissions, reasoning, MCP tools, images, streaming deltas); no fallback config options, since omp reports its models, modes and thinking levels insession/new.agents/omp.mdxand sidebar entry;ompadded to the native agents inv1_agent_process_matrix.Tested:
cargo test -p sandbox-agent-agent-managementandcargo test -p sandbox-agent --test v1_agent_process_matrixpass. In aDockerfile.fullimage built with this change,install-agent --allwroteagent_processes/omp-acp, and a session created through the TypeScript SDK withagent: "omp"ran a full turn:setModel, a prompt that called an HTTP MCP server passed insession/new, and asession/promptresponse with token usage.Not included: adding omp to
Dockerfile.full's default installs. We run it in our own image with omp'stools.approvalMode: yolo, because unattended sessions have no one to answer permission requests; happy to add an install line if you want it in the full image.