fix(ce-work): honor CROSS_MODEL_EFFORT_OVERRIDE in cross-model routes - #1634
Conversation
adapter_argv() pinned model_reasoning_effort=high / --effort high inline with no override path. Route the codex, claude, and grok-cli adapters through the same CROSS_MODEL_EFFORT_OVERRIDE knob the review skills honor; unset keeps the editorial high tier. Closes EveryInc#1569
![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: f40bcddfb9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| printf '%s\0' codex exec --ignore-user-config --ignore-rules --ephemeral \ | ||
| -s workspace-write -C "$WORKSPACE" --json -o "$RAW_RESULT" \ | ||
| -c model_reasoning_effort=high | ||
| -c model_reasoning_effort="${CROSS_MODEL_EFFORT_OVERRIDE:-high}" |
There was a problem hiding this comment.
When CROSS_MODEL_EFFORT_OVERRIDE contains a tier unsupported by the selected route, this now forwards it unvalidated—for example, the new test explicitly accepts max for Codex even though the existing peer contract permits Codex only through xhigh; likewise Claude minimal and Grok xhigh are accepted here. Those inputs reach the external CLI only after controller authorization and then fail the implementation attempt, while effort-less routes silently ignore the same request. Add route-specific validation before constructing or dispatching the adapter, rejecting values and routes that cannot honor the override as the code-review and doc-review wrappers already do.
AGENTS.md reference: AGENTS.md:L136-L138
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 98540cf. validate_effort_override now mirrors the review wrappers' per-route allowlists (claude low..max, codex minimal..xhigh, grok-cli low..high) and fails closed with exit 2 in both the --emit-adapter path and the dispatch path before any CLI invocation; routes with no effort knob (cursor, composer, grok-cursor, opencode) reject any override instead of silently ignoring it. Tests updated: valid tiers per route pass, codex:max / grok-cli:xhigh / cursor:high etc. exit 2 with a named incompatibility.
Codex review on EveryInc#1634: the override was forwarded unvalidated, so a tier the route cannot honor (codex max, grok-cli xhigh) reached the CLI after controller authorization and failed the attempt, while effort-less routes silently ignored the request. Add validate_effort_override with the same per-route allowlists the review wrappers use, failing closed in both the --emit-adapter path and the dispatch path.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98540cfe7c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # Same per-route allowlists as the ce-code-review / ce-doc-review peer paths: | ||
| # reject a tier the selected route cannot honor instead of forwarding it to a | ||
| # CLI that will fail the attempt after controller authorization. Routes with | ||
| # no effort knob (cursor, composer, grok-cursor, opencode) reject any override. |
There was a problem hiding this comment.
When the selected route is opencode, any nonempty override now reaches the default rejection even though the existing OpenCode adapters in skills/ce-code-review/scripts/cross-model-adversarial-review.sh:294-296 and skills/ce-doc-review/scripts/cross-model-doc-review.sh:293-295 pass these effort values to opencode run through --variant. Consequently, an override that works for OpenCode review routes makes the same CE Work route unavailable. Treat OpenCode as effort-bearing and append its validated --variant rather than classifying it with cursor-agent routes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 001d560. Verified the review adapters pass effort to opencode run via --variant; ce-work's opencode route now accepts the same none|minimal|low|medium|high|xhigh|max|default enum in validate_effort_override and appends --variant. Non-enum values still fail closed.
Codex review on EveryInc#1634: opencode is effort-bearing in the review adapters (--variant <effort>), so classifying it with the effort-less routes made a valid override fail the ce-work opencode route. Accept the same none..max|default enum and append --variant.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 001d560a3f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const withOverride = (route: string, value: string) => | ||
| emit(route, { ...process.env, CROSS_MODEL_EFFORT_OVERRIDE: value }) | ||
|
|
||
| expect(emit("codex").stdout).toContain("-c model_reasoning_effort=high") |
There was a problem hiding this comment.
When the suite is launched with CROSS_MODEL_EFFORT_OVERRIDE already exported, emit() inherits it through process.env, so this assertion observes the override rather than high; the same ambient value also makes the earlier cursor/composer/grok-cursor probes exit 2. I reproduced this with CROSS_MODEL_EFFORT_OVERRIDE=low, which emitted model_reasoning_effort=low for Codex and rejected Cursor. Remove this variable from the baseline test environment and add it only in cases explicitly testing an override.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 006b17b. Default-posture assertions now run under a cleaned env (CROSS_MODEL_EFFORT_OVERRIDE deleted), and override cases set the variable explicitly. This also hardens the pre-existing production-argv test, which my change had made sensitive to the same ambient value. Verified: with CROSS_MODEL_EFFORT_OVERRIDE=low exported, the four affected tests all pass.
… assertions Codex review on EveryInc#1634: with the override exported in the suite's own environment, default-posture assertions observed the override instead of the editorial high tier (reproduced: codex emitted model_reasoning_effort=low, cursor exited 2). Baseline assertions now run under a cleaned env; override cases set the variable explicitly. Verified the full affected set passes with CROSS_MODEL_EFFORT_OVERRIDE=low exported.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 006b17bdfd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function cleanEnv(): NodeJS.ProcessEnv { | ||
| const env = { ...process.env } | ||
| delete env.CROSS_MODEL_EFFORT_OVERRIDE | ||
| return env |
There was a problem hiding this comment.
When the test process exports CROSS_MODEL_EFFORT_OVERRIDE (for example, low), this helper protects only the few calls explicitly changed to use it; later tests still build their environments from process.env. I reproduced the current-head test Cursor accepts a controller-bounded explicit model while Composer stays family-locked failing with status 2 under that environment. Fresh evidence after the prior thread is that those remaining calls still spread process.env, so make the cleaned environment the suite-wide default and add the override back only in tests that exercise it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed suite-wide in the latest push. Reproduced your exact case first ('Cursor accepts a controller-bounded explicit model' fails with status 2 under CROSS_MODEL_EFFORT_OVERRIDE=low), then moved the scrub to module load: the suite deletes the variable from process.env up front, and only the override tests set it explicitly. Both the reproduction test and the override tests pass under the ambient export.
Codex review on EveryInc#1634 round 2: per-call cleaning left later tests that spread process.env exposed (reproduced: 'Cursor accepts a controller-bounded explicit model' exits 2 with CROSS_MODEL_EFFORT_OVERRIDE=low exported). Delete the variable from process.env at module load so every test starts clean; override cases still set it explicitly per call.
3 participants
Summary
cross-model-work.shpinnedmodel_reasoning_effort=high(codex) and--effort high(claude, grok-cli) inline inadapter_argv()with no override path, unlike the ce-code-review / ce-doc-review peer paths which honorCROSS_MODEL_EFFORT_OVERRIDE.CROSS_MODEL_EFFORT_OVERRIDEknob; unset keeps the editorialhightier, so default behavior is unchanged.Test plan
ce-work-cross-model-routes.test.ts: override retunes codex/claude/grok-cli, default stayshigh, non-effort routes (cursor, opencode) unaffectedbun test tests/skills/ce-work-cross-model-routes.test.tsgreenSecurity Disclosure
No security-relevant behavior change. The override is an operator-set environment variable; invalid values are passed through to the target CLI, which rejects them, matching the review-path semantics.
Agent Disclosure
This PR was prepared by an Instinct agent (harness does not expose the model identity) on behalf of @khsaurabh.