Add support for a proper codex plugin.#11871
Conversation
liliwilson
force-pushed
the
lili/add-codex-plugin
branch
from
fcb4f32 to
43c8c2a
Compare
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds a feature-flagged Codex plugin path, distinguishes Codex OSC 9 fallback from structured OSC 777 plugin sessions, updates UI consumers to use session-level rich-status semantics, and adds Codex plugin install/update management.
Concerns
- The PR changes user-facing notification setup/status behavior and install/update UI copy, but the description has no screenshots or recording and the manual testing checkbox is unchecked. For this user-facing change, please include screenshots or a screen recording demonstrating it working end to end.
- See inline comments for two correctness issues around plugin-version state and rich-status detection.
- No approved repository spec context was provided, so there are no separate implementation-vs-spec findings.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds feature-flagged Codex plugin support, updates Codex OSC 777/OSC 9 session handling, wires plugin install/update UI behavior through session-level rich-status checks, and adds Codex plugin-manager coverage.
Concerns
- Codex's new platform plugin install hook is not reachable from the existing harness setup path because Codex does not override the platform install/version checks that gate that hook.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| FeatureFlag::CodexPlugin.is_enabled() | ||
| } | ||
|
|
||
| async fn install_platform_plugin(&self) -> Result<(), PluginInstallError> { |
There was a problem hiding this comment.
CliAgentPluginManager::is_platform_plugin_installed() defaults to true and Codex does not override it, so setup_harness() skips installing orchestration@codex-warp. Add Codex-specific platform install/version checks, similar to Claude's manager, before relying on this hook.
There was a problem hiding this comment.
assuming this'll be fixed in a follow-up PR for the platform stuff?
harryalbert
left a comment
harryalbert
left a comment
There was a problem hiding this comment.
Nice! didn't realize that the codex plugin supports hooks now.
Just had a few small nits but overall LGTM
| FeatureFlag::CodexPlugin.is_enabled() | ||
| } | ||
|
|
||
| async fn install_platform_plugin(&self) -> Result<(), PluginInstallError> { |
There was a problem hiding this comment.
assuming this'll be fixed in a follow-up PR for the platform stuff?
2 participants
Description
This PR adds support for a proper Codex plugin to back notifications and third-party harness support in Warp, so we no longer need to rely on OSC 9 notifications or installing hooks via a sidecar image for cloud agents.
We gate all of this behind a feature flag,
CodexPlugin, so we can make sure the plugin works in dogfood before enabling for everyone.We also make sure that still fall back to OSC 9 notifications, but store a new field on the
CodexSessionHandler,structured_plugin_active, so that when we receive our first OSC 777 and see we have an active plugin, we ignore all of the less-rich OSC 9 notifications that come afterwards.This PR does not handle automatically installing the plugin for orchestration/cloud agent use cases. That will need to be done in a future PR.
Testing
./script/runScreenshots / Videos
Loom: https://www.loom.com/share/62f87191f6de4ce988f822a44b8fc8a6
Not shown in the video but have also confirmed that if we don't accept the plugin, we still get OSC 9 notifications via the old path.
Agent Mode