Skip to content

fix(codex): omit collaboration mode until a model is known - #771

Open
Slowper wants to merge 1 commit into
hardbeat920:mainfrom
Slowper:fix/codex-null-collaboration-model
Open

Slowper wants to merge 1 commit into
hardbeat920:mainfrom
Slowper:fix/codex-null-collaboration-model

Conversation

@Slowper

@Slowper Slowper commented Oct 6, 2026 •

Copy link
Copy Markdown

What changed

turn/start no longer sends collaborationMode when no model has been selected. A selected model still sends the same mode, effort, and built-in instructions as before. A plan turn with no model still uses the read-only sandbox and approvalPolicy: "never".

Why

Codex 0.160 rejects the turn before it starts:

Invalid request: invalid type: null, expected a string

collaborationMode.settings.model is a required string. Sending null is that error. Omitting the field is missing field model. Reproduced against codex app-server 0.160.0: a thread starts and Codex chooses the model, then turn/start with settings.model: null fails immediately. The same request with the model omitted from collaborationMode, or with settings.model set to a real model id, is accepted. reasoning_effort: null and developer_instructions: null are fine once model is a string.

UI

No layout changes. A Codex turn that previously failed with no reply now starts.

Validation

  • npm test — 4261 passed, 13 skipped
  • npx tsc --noEmit
  • npm run test:host and npm run host:package on Node 24.21.0 (the host job's Node version). Node 20 cannot load node:sqlite, which the host job does not use.
  • cargo fmt --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo check
  • cargo test — 539 passed, 1 ignored

Checklist

  • I ran npm run check (web and Rust steps, matching CI)
  • This PR is small and focused
  • I did not mix unrelated changes

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Codex requests now omit model settings when no model is specified, preventing blank model values from being sent.
    • Plan turns continue to use read-only sandbox settings and never request approval when the model is blank.

Codex 0.160 rejects turn/start when settings.model is null.
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 654e1ad9-8169-4c56-8cb1-e66ca047462e
📥 Commits

Reviewing files that changed from the base of the PR and between 98da85a and 4d289eb.

📒 Files selected for processing (2)
  • src/integrations/harness/providers/codex/codexProtocol.test.ts
  • src/integrations/harness/providers/codex/codexProtocol.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The Codex turn parameter builder now trims the model value and omits model and collaborationMode when it is empty. Tests cover empty-model behavior for standard and plan turns.

Changes

Codex turn parameters

Layer / File(s) Summary
Conditional model fields
src/integrations/harness/providers/codex/codexProtocol.ts, src/integrations/harness/providers/codex/codexProtocol.test.ts
The builder trims the model and includes model and collaborationMode only when the trimmed value is nonempty. Tests check empty-model parameters and verify that plan turns retain a read-only sandbox and approvalPolicy: "never".

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: sambhavthakkar

Merge Risk: 🔵 Low · up to 4d289

A model-less planning side answer may lack the Plan instruction preset, though edits remain blocked. This is a bounded behavior issue that can be accepted or addressed separately.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: omitting collaboration mode until a model is known.
Description check ✅ Passed The description covers what changed, why, UI impact, validation, and all checklist items required by the template.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant