Use remote compaction for compatible providers - #1
lawrencecchen wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughChangesOpenAI Codex protocol handling
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: ⚪ Minimal · up to This localized provider-detection change is merge-ready after normal checks; no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
![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: 6ff93bdc12
ℹ️ 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".
| // Codex-compatible providers can be local/subrouter aliases while still | ||
| // speaking the OpenAI Responses protocol. Keep the remote compaction and | ||
| // checkpoint lifecycle attached to the protocol, not only the provider name. | ||
| return model?.provider === "openai-codex" || model?.api === "openai-codex-responses"; |
There was a problem hiding this comment.
For a custom provider that uses openai-codex-responses for ordinary Responses requests but does not implement Codex RemoteCompactionV2 or use a ChatGPT OAuth JWT, this predicate now intercepts automatic compaction instead of leaving it to Pi's local summarizer. The handler subsequently calls buildCompactHeaders, which extracts a ChatGPT account claim from the token, and posts a compaction_trigger; an API-key proxy or partial implementation therefore throws when the context reaches the compaction boundary and prevents the session from continuing. Restrict this to known compatible aliases or an explicit remote-compaction capability rather than treating the transport API as sufficient.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@extensions/pi-codex.ts`:
- Around line 105-109: Add regression coverage for isOpenAICodexModel using a
model with a non-"openai-codex" provider and api set to
"openai-codex-responses". Exercise the relevant remote compaction, checkpoint
reinjection, and retry turn-state paths to verify API-only Codex detection
preserves all lifecycle behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d81a28a-0c5d-47e7-b536-438c127ff36c
📒 Files selected for processing (1)
extensions/pi-codex.ts
| function isOpenAICodexModel(model: Model<any> | undefined): model is Model<any> { | ||
| // Codex-compatible providers can be local/subrouter aliases while still | ||
| // speaking the OpenAI Responses protocol. Keep the remote compaction and | ||
| // checkpoint lifecycle attached to the protocol, not only the provider name. | ||
| return model?.provider === "openai-codex" || model?.api === "openai-codex-responses"; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Add regression coverage for API-only Codex detection.
The supplied tests keep provider: "openai-codex", including the remote-compaction test. They do not prove that a different provider using api: "openai-codex-responses" receives compaction, checkpoint reinjection, and retry turn-state handling. Add one API-only model case to prevent regression to provider-only detection.
🤖 Prompt for 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.
In `@extensions/pi-codex.ts` around lines 105 - 109, Add regression coverage for
isOpenAICodexModel using a model with a non-"openai-codex" provider and api set
to "openai-codex-responses". Exercise the relevant remote compaction, checkpoint
reinjection, and retry turn-state paths to verify API-only Codex detection
preserves all lifecycle behavior.
1 participant
Treat providers speaking the OpenAI Codex Responses API, including the local Subrouter alias, as Codex-compatible for remote compaction and checkpoint reinjection.
Tests: npm test; npm run check.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Enable Codex remote compaction and checkpoint reinjection for any model using the OpenAI Codex Responses API. Previously this ran only when provider was "openai-codex"; it now also runs when model.api is "openai-codex-responses" (e.g., local
subrouteraliases).Written for commit 6ff93bd. Summary will update on new commits.
Summary by CodeRabbit