refactor(codex): replace the SDK with a shared App Server runtime - #192
refactor(codex): replace the SDK with a shared App Server runtime#192Waishnav wants to merge 6 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughCodex execution now uses the installed ChangesCodex App Server integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Adapter as CodexLocalAgentAdapter
participant Runtime as CodexAppServerRuntime
participant Transport as CodexAppServerTransport
participant CLI as Codex CLI
Adapter->>Runtime: Run input
Runtime->>Transport: Start or resume thread
Transport->>CLI: Send JSON-RPC request
CLI-->>Transport: Return response and notifications
Transport-->>Runtime: Deliver turn events
Runtime-->>Adapter: Return final agent message
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryRefactors the Codex integration from the bundled SDK to the user's installed Codex CLI and its App Server protocol.
Confidence Score: 5/5The PR appears safe to merge with no concrete blocking or independently actionable non-blocking issue identified. The new Codex App Server path consistently resolves the CLI, pools runtime ownership, correlates thread notifications, closes child processes, and preserves the existing input and sandbox contracts.
|
| Filename | Overview |
|---|---|
| src/local-agent-codex/app-server-transport.ts | Adds JSON-lines RPC transport, process lifecycle management, pending-request handling, and notification dispatch for the Codex App Server. |
| src/local-agent-codex/runtime.ts | Implements pooled Codex thread and turn execution with sandbox mapping, resumption, event correlation, and final-response extraction. |
| src/local-agent-codex/command.ts | Resolves the configured Codex executable and checks whether its App Server command is available. |
| src/local-agent-runtime-registry.ts | Routes Codex runs through the shared harness runtime pool. |
| src/local-agent-path.ts | Adds cross-platform executable resolution using PATH, PATHEXT, and platform-appropriate access checks. |
| src/local-agent-codex/runtime.test.ts | Covers concurrent threads, thread resumption, sandbox modes, model selection, reasoning effort, and early notifications. |
| package.json | Removes the bundled Codex SDK and updates the test command for the App Server runtime tests. |
Sequence Diagram
sequenceDiagram
participant Manager as LocalAgentManager
participant Registry as RuntimeRegistry
participant Pool as HarnessRuntimePool
participant Runtime as CodexAppServerRuntime
participant CLI as codex app-server
Manager->>Registry: run("codex", input)
Registry->>Pool: run(codexDriver, input)
Pool->>Runtime: create or reuse runtime
Runtime->>CLI: thread/start or thread/resume
CLI-->>Runtime: thread id
Runtime->>CLI: turn/start
CLI-->>Runtime: item/completed notifications
CLI-->>Runtime: turn/completed
Runtime-->>Manager: session id, response, items
Reviews (1): Last reviewed commit: "build(codex): remove bundled sdk runtime" | Re-trigger Greptile
Waishnav
changed the title
00a287a to
0cc1046
Compare
0cc1046 to
93fd551
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/local-agent-codex/app-server-transport.ts`:
- Around line 53-61: Update the app-server launch logic in the transport
constructor and checkCodexAppServerAvailability to distinguish .CMD/.BAT shims
from native executables: spawn native executables without a shell, while
launching batch shims through a safely quoted and validated Windows shell
command. Reuse the same executable classification and argument strategy in both
paths, preserving existing non-Windows behavior.
In `@src/local-agent-codex/runtime.ts`:
- Around line 35-39: Update src/local-agent-codex/runtime.ts lines 35-39 in the
constructor to subscribe to CodexAppServerConnection’s new close hook and reject
every entry in pendingTurns when the App Server terminates. Update
src/local-agent-codex/runtime.test.ts lines 5-69 by adding the corresponding
close hook to FakeCodexConnection and a test that starts runtime.run, triggers
closure, and verifies the promise rejects instead of hanging.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a721550-349f-4552-81cf-fee76894299b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
docs/agent-profile-schema.mddocs/configuration.mdpackage.jsonsrc/local-agent-adapters.tssrc/local-agent-availability.test.tssrc/local-agent-availability.tssrc/local-agent-codex/app-server-transport.tssrc/local-agent-codex/command.tssrc/local-agent-codex/runtime.test.tssrc/local-agent-codex/runtime.tssrc/local-agent-path.tssrc/local-agent-runtime-registry.tssrc/local-agent-runtime.test.tssrc/local-agent-runtime.ts
💤 Files with no reviewable changes (2)
- src/local-agent-runtime.test.ts
- src/local-agent-runtime.ts
93fd551 to
ea5f12f
Compare
1 participant
DevSpace currently depends on
@openai/codex-sdk, which also makes DevSpace own a bundled Codex runtime and still starts Codex work per invocation. This switches the adapter to the user-installedcodex app-server, adds a narrow JSONL transport, and lets one compatible App Server runtime host multiple DevSpace Codex threads. Existing thread IDs remain the durable continuation handle, so a discarded App Server can be recreated and resumed.\n\nCODEX_COMMANDcan select a specific installation, default resolution avoids DevSpace-ownednode_modules/.bin, and the SDK/bundled Codex dependency is removed. DevSpace intentionally does not commit generated App Server bindings; it only owns the small stable protocol surface it needs. Stacked on #191.Summary by CodeRabbit
New Features
CODEX_HOMEsettings are respected.CODEX_COMMAND.Documentation