Skip to content

fix: pass reasoning effort through to Codex - #1237

Merged
mldangelo-oai merged 4 commits into
mainfrom
mdangelo/codex/effort-pass-through
Oct 5, 2026
Merged

mldangelo-oai merged 4 commits into
mainfrom
mdangelo/codex/effort-pass-through

Conversation

@mldangelo-oai

@mldangelo-oai mldangelo-oai commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The CLI and SDK helpers maintain reasoning-effort lists that can lag behind Codex and reject values supported by a selected model or provider. Accept effort strings and forward them to Codex while retaining existing defaults and precedence.

Changes

  • Keep --effort EFFORT and accept any nonempty string. Generated CLI schemas and help describe pass-through behavior instead of a fixed enum.
  • Widen SDK helper reasoningEffort inputs to string, retaining the compatibility cast required by the pinned SDK type.
  • Preserve model selection, defaults, explicit-option precedence, and original provider errors without substitution or retries.
  • Keep focused future-value coverage, the real SDK subprocess matrix, and concurrent fresh/resumed Deep Scan discovery and reducer coverage. Remove repeated helper scenarios and redundant effort cases.
  • Integrate the worker cases with main's typed executor fixtures and shared override assertions, preserving exact effort arguments, inherited settings, and per-scan authentication/environment isolation.

Testing

Current merge validation:

  • Six affected SDK files, Bun seed 12345: 295 passed, 3 Windows-only skips, 0 failed. This includes CLI schemas/help, future effort values, explicit/default/profile precedence, unchanged provider errors, and the real SDK subprocess-forwarding matrix.

  • The full typed Deep Scan executor test passed under Node 22.13.0, including concurrent discovery/reducer workers with fresh/resumed threads, explicit/future/inherited settings, and preserved authentication, Python, Git, permission and environment forwarding.

  • SDK build:ci and SDK/MCP types, SDK format, focused executor Prettier, bundled plugin build, and git diff --check: passed.

  • Plugin Ruff check and format, the tracked .mts portable compatibility checker, and all 9 compatibility tests: passed. The Node 22.13 invocations use the required strip-types flag.

  • The original SDK source, documentation and tests remain unchanged by the conflict resolution. No new full SDK or full MCP suite was run; the authored integration is confined to the typed executor test. No live model calls were made.

  • Final merge review: three independent native passes and a fresh verifier found no new integration issues at fe940f69.

Historical validation of cleanup commit efa1bd50:

  • Severity classification, owner suggestion, and real SDK subprocess-forwarding tests: 31 passed.
  • SDK build:ci and types, focused Prettier check, and git diff --check: passed.
  • Plugin Ruff check and format check, portable source compatibility check, and all 9 compatibility tests: passed.
  • A broader focused run had 266 passes, 3 skips, and 29 failures involving the missing bundled plugin and temporary-directory ownership. The plugin build then stopped because native prebuilt artifacts were unavailable.
  • The Deep Scan executor rerun stopped with ENOSPC while creating a binary fixture. Full package tests and a fresh native review gate were not completed for this cleanup commit.

The preceding head 5939759 passed node-ci. Those results do not validate the cleanup commit.

Risk and rollout

This broadens existing CLI and SDK inputs; no flags, environment variables, or defaults are added or changed. Existing effort values, including minimal, remain accepted. Support depends on the installed Codex runtime and selected model/provider.

The generated CLI schema uses a nonempty string instead of an enum. Consumers that treat the previous SDK type as a closed enumeration may need to update those assumptions. Tests use synthetic values and local subprocesses; no live model calls were made. The cleanup changed tests only. The merge resolution preserves both parents in the typed worker regression test.

Public disclosure review

  • No customer, partner, prospect, or user identities, data, or identifying details are included.
  • No credentials, personal data, private source, scan findings, or nonpublic links or tickets are included.
  • I reviewed the branch name, title, description, commits, changes, comments, logs, screenshots, attachments, and links for public disclosure.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-05T17:40:10.089360Z fe940f6 New commits
🔒 Security Review ✅ Completed 2026-10-05T17:41:34.397812Z fe940f6 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added the bug Something isn't working label Oct 5, 2026

@alandelong-oai alandelong-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full contribution at efa1bd50a7cd43ff6fdb528a774738f65e81e625 with selected base 398c4ce0edc05763b546a6823a1da86912b1f338, including changed tests and touched context, using three independent reviews followed by root synthesis.

No supported introduced finding.

Static source review; tests were inspected but not executed. CI, mergeability and integration are separate.

@ianw-oai ianw-oai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking findings at fe940f6. Passing effort values through to Codex keeps defaults and option precedence intact without maintaining a separate allowlist. Current-head node-ci passed; I did not rerun tests locally.

@ianw-oai ianw-oai mentioned this pull request Oct 5, 2026
3 tasks done
@mldangelo-oai
mldangelo-oai merged commit 6e9adf2 into main Oct 5, 2026
69 of 85 checks passed
@mldangelo-oai
mldangelo-oai deleted the mdangelo/codex/effort-pass-through branch October 5, 2026 20:08
@github-actions github-actions Bot mentioned this pull request Oct 5, 2026
3 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants