From f40bcddfb9fbd4a8b8e803553d5860649075964b Mon Sep 17 00:00:00 2001 From: khsaurabh Date: Sat, 5 Sep 2026 14:26:13 +0530 Subject: [PATCH 1/5] fix(ce-work): honor CROSS_MODEL_EFFORT_OVERRIDE in cross-model routes 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 #1569 --- skills/ce-work/scripts/cross-model-work.sh | 8 +++++--- .../skills/ce-work-cross-model-routes.test.ts | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/skills/ce-work/scripts/cross-model-work.sh b/skills/ce-work/scripts/cross-model-work.sh index df8abf884e..95beeebcf1 100755 --- a/skills/ce-work/scripts/cross-model-work.sh +++ b/skills/ce-work/scripts/cross-model-work.sh @@ -100,9 +100,11 @@ adapter_argv() { codex) # --ignore-user-config drops the user's model_reasoning_effort, so pin the # editorial tier explicitly, matching the claude/grok routes' --effort high. + # CROSS_MODEL_EFFORT_OVERRIDE retunes all three effort-taking routes, the + # same knob the ce-code-review / ce-doc-review peer paths honor. 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}" [ "$(route_model codex)" = auto ] || printf '%s\0' -m "$(route_model codex)" printf '%s\0' - ;; @@ -112,14 +114,14 @@ adapter_argv() { printf '%s\0' claude -p --safe-mode --no-session-persistence \ --permission-mode bypassPermissions --tools Read,Write,Edit,Bash \ --allowed-tools 'Bash(*)' \ - --effort high --output-format stream-json --verbose + --effort "${CROSS_MODEL_EFFORT_OVERRIDE:-high}" --output-format stream-json --verbose [ "$claude_model" = auto ] || printf '%s\0' --model "$claude_model" ;; grok-cli) local grok_model grok_model="$(route_model grok-cli)" printf '%s\0' grok --prompt-file "$PROMPT_FILE" --cwd "$WORKSPACE" \ - --effort high --permission-mode acceptEdits \ + --effort "${CROSS_MODEL_EFFORT_OVERRIDE:-high}" --permission-mode acceptEdits \ --tools Read,Write,Edit --disable-web-search --no-memory --no-subagents \ --no-plan --max-turns 50 --output-format streaming-json --verbatim [ "$grok_model" = auto ] || printf '%s\0' --model "$grok_model" diff --git a/tests/skills/ce-work-cross-model-routes.test.ts b/tests/skills/ce-work-cross-model-routes.test.ts index 50acd24aad..697be58e04 100644 --- a/tests/skills/ce-work-cross-model-routes.test.ts +++ b/tests/skills/ce-work-cross-model-routes.test.ts @@ -265,6 +265,24 @@ describe("ce-work fixed write routes", () => { expect(opencode).not.toContain("--model") }) + test("CROSS_MODEL_EFFORT_OVERRIDE retunes the effort-taking routes and stays off by default", () => { + const withOverride = (route: string, value: string) => + emit(route, { ...process.env, CROSS_MODEL_EFFORT_OVERRIDE: value }).stdout + + expect(emit("codex").stdout).toContain("-c model_reasoning_effort=high") + expect(withOverride("codex", "max")).toContain("-c model_reasoning_effort=max") + expect(withOverride("codex", "minimal")).toContain("-c model_reasoning_effort=minimal") + + expect(emit("claude").stdout).toContain("--effort high") + expect(withOverride("claude", "low")).toContain("--effort low") + + expect(emit("grok-cli").stdout).toContain("--effort high") + expect(withOverride("grok-cli", "medium")).toContain("--effort medium") + + expect(withOverride("cursor", "max")).not.toContain("--effort") + expect(withOverride("opencode", "max")).not.toContain("--effort") + }) + test.each(ROUTES)("%s receives one workspace and bounded packet", (route) => { const f = fixture() const bin = fakeBin(route, f.capture) From 98540cfe7ce26185626daa492762c7d259b2fafe Mon Sep 17 00:00:00 2001 From: khsaurabh Date: Sat, 5 Sep 2026 15:19:28 +0530 Subject: [PATCH 2/5] fix(ce-work): validate effort overrides per route before dispatch Codex review on #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. --- skills/ce-work/scripts/cross-model-work.sh | 24 ++++++++++++++ .../skills/ce-work-cross-model-routes.test.ts | 31 ++++++++++++++----- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/skills/ce-work/scripts/cross-model-work.sh b/skills/ce-work/scripts/cross-model-work.sh index 95beeebcf1..0eb0f0943f 100755 --- a/skills/ce-work/scripts/cross-model-work.sh +++ b/skills/ce-work/scripts/cross-model-work.sh @@ -95,6 +95,21 @@ validate_model_override() { esac } +validate_effort_override() { + # 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. + local route="$1" effort="${CROSS_MODEL_EFFORT_OVERRIDE:-}" + [ -n "$effort" ] || return 0 + case "$route:$effort" in + claude:low|claude:medium|claude:high|claude:xhigh|claude:max) ;; + codex:minimal|codex:low|codex:medium|codex:high|codex:xhigh) ;; + grok-cli:low|grok-cli:medium|grok-cli:high) ;; + *) return 1 ;; + esac +} + adapter_argv() { case "$1" in codex) @@ -159,6 +174,10 @@ if [ "${1:-}" = "--emit-adapter" ]; then printf "model override '%s' not compatible with route '%s'\n" "${CE_WORK_MODEL_OVERRIDE:-}" "$ROUTE" >&2 exit 2 } + validate_effort_override "$ROUTE" || { + printf "effort override '%s' not compatible with route '%s'\n" "${CROSS_MODEL_EFFORT_OVERRIDE:-}" "$ROUTE" >&2 + exit 2 + } adapter_argv "$ROUTE" >/dev/null 2>&1 || { printf "unknown route '%s'\n" "$ROUTE" >&2; exit 2; } adapter_argv "$ROUTE" | tr '\0' ' ' printf '\n' @@ -694,6 +713,11 @@ if ! command -v "$BINARY" >/dev/null 2>&1; then exit 2 fi +validate_effort_override "$ROUTE" || { + publish_unavailable "effort override '${CROSS_MODEL_EFFORT_OVERRIDE:-}' not compatible with route '$ROUTE'" || exit 2 + exit 2 +} + ARGS=() while IFS= read -r -d '' token; do ARGS+=("$token"); done < <(adapter_argv "$ROUTE") diff --git a/tests/skills/ce-work-cross-model-routes.test.ts b/tests/skills/ce-work-cross-model-routes.test.ts index 697be58e04..01c51300cd 100644 --- a/tests/skills/ce-work-cross-model-routes.test.ts +++ b/tests/skills/ce-work-cross-model-routes.test.ts @@ -267,20 +267,37 @@ describe("ce-work fixed write routes", () => { test("CROSS_MODEL_EFFORT_OVERRIDE retunes the effort-taking routes and stays off by default", () => { const withOverride = (route: string, value: string) => - emit(route, { ...process.env, CROSS_MODEL_EFFORT_OVERRIDE: value }).stdout + emit(route, { ...process.env, CROSS_MODEL_EFFORT_OVERRIDE: value }) expect(emit("codex").stdout).toContain("-c model_reasoning_effort=high") - expect(withOverride("codex", "max")).toContain("-c model_reasoning_effort=max") - expect(withOverride("codex", "minimal")).toContain("-c model_reasoning_effort=minimal") + expect(withOverride("codex", "xhigh").stdout).toContain("-c model_reasoning_effort=xhigh") + expect(withOverride("codex", "minimal").stdout).toContain("-c model_reasoning_effort=minimal") expect(emit("claude").stdout).toContain("--effort high") - expect(withOverride("claude", "low")).toContain("--effort low") + expect(withOverride("claude", "low").stdout).toContain("--effort low") + expect(withOverride("claude", "max").stdout).toContain("--effort max") expect(emit("grok-cli").stdout).toContain("--effort high") - expect(withOverride("grok-cli", "medium")).toContain("--effort medium") + expect(withOverride("grok-cli", "medium").stdout).toContain("--effort medium") + }) + + test("CROSS_MODEL_EFFORT_OVERRIDE rejects tiers the route cannot honor, failing closed before dispatch", () => { + const rejected = (route: string, value: string) => { + const proc = emit(route, { ...process.env, CROSS_MODEL_EFFORT_OVERRIDE: value }) + expect(proc.status).toBe(2) + expect(proc.stderr).toContain(`effort override '${value}' not compatible with route '${route}'`) + } - expect(withOverride("cursor", "max")).not.toContain("--effort") - expect(withOverride("opencode", "max")).not.toContain("--effort") + rejected("codex", "max") // codex tops out at xhigh + rejected("codex", "none") + rejected("claude", "minimal") + rejected("grok-cli", "xhigh") + rejected("grok-cli", "max") + // routes with no effort knob reject any override rather than silently ignoring it + rejected("cursor", "high") + rejected("composer", "high") + rejected("grok-cursor", "high") + rejected("opencode", "high") }) test.each(ROUTES)("%s receives one workspace and bounded packet", (route) => { From 001d560a3fc26603f5081723798b449b094c3edc Mon Sep 17 00:00:00 2001 From: khsaurabh Date: Sat, 5 Sep 2026 18:02:15 +0530 Subject: [PATCH 3/5] fix(ce-work): carry OpenCode effort overrides through --variant Codex review on #1634: opencode is effort-bearing in the review adapters (--variant ), 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. --- skills/ce-work/scripts/cross-model-work.sh | 5 ++++- tests/skills/ce-work-cross-model-routes.test.ts | 9 ++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/skills/ce-work/scripts/cross-model-work.sh b/skills/ce-work/scripts/cross-model-work.sh index 0eb0f0943f..cf364a8317 100755 --- a/skills/ce-work/scripts/cross-model-work.sh +++ b/skills/ce-work/scripts/cross-model-work.sh @@ -99,13 +99,14 @@ validate_effort_override() { # 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. + # no effort knob (cursor, composer, grok-cursor) reject any override. local route="$1" effort="${CROSS_MODEL_EFFORT_OVERRIDE:-}" [ -n "$effort" ] || return 0 case "$route:$effort" in claude:low|claude:medium|claude:high|claude:xhigh|claude:max) ;; codex:minimal|codex:low|codex:medium|codex:high|codex:xhigh) ;; grok-cli:low|grok-cli:medium|grok-cli:high) ;; + opencode:none|opencode:minimal|opencode:low|opencode:medium|opencode:high|opencode:xhigh|opencode:max|opencode:default) ;; *) return 1 ;; esac } @@ -160,6 +161,8 @@ adapter_argv() { printf '%s\0' opencode run --dir "$WORKSPACE" --format json --auto --file "$PROMPT_FILE" printf '%s\0' "Follow the attached unit packet. Return only the implementation result JSON." [ "$(route_model opencode)" = auto ] || printf '%s\0' --model "$(route_model opencode)" + # OpenCode carries effort through --variant, same as the review adapters. + [ -z "${CROSS_MODEL_EFFORT_OVERRIDE:-}" ] || printf '%s\0' --variant "$CROSS_MODEL_EFFORT_OVERRIDE" ;; *) return 1 ;; esac diff --git a/tests/skills/ce-work-cross-model-routes.test.ts b/tests/skills/ce-work-cross-model-routes.test.ts index 01c51300cd..34b83a5ca3 100644 --- a/tests/skills/ce-work-cross-model-routes.test.ts +++ b/tests/skills/ce-work-cross-model-routes.test.ts @@ -297,7 +297,14 @@ describe("ce-work fixed write routes", () => { rejected("cursor", "high") rejected("composer", "high") rejected("grok-cursor", "high") - rejected("opencode", "high") + // opencode is effort-bearing through --variant, but only for its own enum + rejected("opencode", "bogus") + }) + + test("opencode carries the override through --variant, matching the review adapters", () => { + const out = emit("opencode", { ...process.env, CROSS_MODEL_EFFORT_OVERRIDE: "max" }).stdout + expect(out).toContain("--variant max") + expect(emit("opencode").stdout).not.toContain("--variant") }) test.each(ROUTES)("%s receives one workspace and bounded packet", (route) => { From 006b17bdfd9c82caa1fcf9ebacd69ce4d7770e06 Mon Sep 17 00:00:00 2001 From: khsaurabh Date: Sat, 5 Sep 2026 19:28:57 +0530 Subject: [PATCH 4/5] test(ce-work): scrub ambient CROSS_MODEL_EFFORT_OVERRIDE from default assertions Codex review on #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. --- .../skills/ce-work-cross-model-routes.test.ts | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/tests/skills/ce-work-cross-model-routes.test.ts b/tests/skills/ce-work-cross-model-routes.test.ts index 34b83a5ca3..4742877e4f 100644 --- a/tests/skills/ce-work-cross-model-routes.test.ts +++ b/tests/skills/ce-work-cross-model-routes.test.ts @@ -221,18 +221,26 @@ function emit(route: string, env: NodeJS.ProcessEnv = process.env) { return spawnSync("bash", [SCRIPT, "--emit-adapter", route], { encoding: "utf8", env }) } +// The script honors CROSS_MODEL_EFFORT_OVERRIDE, so default-posture assertions +// must not inherit an ambient override from the suite's own environment. +function cleanEnv(): NodeJS.ProcessEnv { + const env = { ...process.env } + delete env.CROSS_MODEL_EFFORT_OVERRIDE + return env +} + describe("ce-work fixed write routes", () => { test("production argv uses the qualified noninteractive write posture", () => { - for (const route of ROUTES) expect(emit(route).status).toBe(0) + for (const route of ROUTES) expect(emit(route, cleanEnv()).status).toBe(0) - const codex = emit("codex").stdout + const codex = emit("codex", cleanEnv()).stdout expect(codex).toContain("exec") expect(codex).toContain("--ephemeral") expect(codex).toContain("-s workspace-write") expect(codex).toContain("-C ") expect(codex).toContain("-c model_reasoning_effort=high") - const claude = emit("claude").stdout + const claude = emit("claude", cleanEnv()).stdout expect(claude).toContain("--safe-mode") expect(claude).toContain("--permission-mode bypassPermissions") expect(claude).toContain("--tools Read,Write,Edit,Bash") @@ -240,7 +248,7 @@ describe("ce-work fixed write routes", () => { expect(claude).toContain("--no-session-persistence") expect(claude).not.toContain("--model") - const grok = emit("grok-cli").stdout + const grok = emit("grok-cli", cleanEnv()).stdout expect(grok).toContain("--cwd ") expect(grok).toContain("--permission-mode acceptEdits") expect(grok).toContain("--no-memory") @@ -248,15 +256,15 @@ describe("ce-work fixed write routes", () => { expect(grok).not.toContain("--model") for (const route of ["cursor", "composer", "grok-cursor"]) { - const command = emit(route).stdout + const command = emit(route, cleanEnv()).stdout expect(command).toContain("--sandbox enabled") expect(command).toContain("--workspace ") expect(command).toContain("--output-format stream-json") } - expect(emit("cursor").stdout).not.toContain("--model") - expect(emit("composer").stdout).toContain("--model composer-2.5-fast") - expect(emit("grok-cursor").stdout).toContain("--model cursor-grok-4.6-high") - const opencode = emit("opencode").stdout + expect(emit("cursor", cleanEnv()).stdout).not.toContain("--model") + expect(emit("composer", cleanEnv()).stdout).toContain("--model composer-2.5-fast") + expect(emit("grok-cursor", cleanEnv()).stdout).toContain("--model cursor-grok-4.6-high") + const opencode = emit("opencode", cleanEnv()).stdout expect(opencode).toContain("opencode run") expect(opencode).toContain("--dir ") expect(opencode).toContain("--format json") @@ -267,23 +275,23 @@ describe("ce-work fixed write routes", () => { test("CROSS_MODEL_EFFORT_OVERRIDE retunes the effort-taking routes and stays off by default", () => { const withOverride = (route: string, value: string) => - emit(route, { ...process.env, CROSS_MODEL_EFFORT_OVERRIDE: value }) + emit(route, { ...cleanEnv(), CROSS_MODEL_EFFORT_OVERRIDE: value }) - expect(emit("codex").stdout).toContain("-c model_reasoning_effort=high") + expect(emit("codex", cleanEnv()).stdout).toContain("-c model_reasoning_effort=high") expect(withOverride("codex", "xhigh").stdout).toContain("-c model_reasoning_effort=xhigh") expect(withOverride("codex", "minimal").stdout).toContain("-c model_reasoning_effort=minimal") - expect(emit("claude").stdout).toContain("--effort high") + expect(emit("claude", cleanEnv()).stdout).toContain("--effort high") expect(withOverride("claude", "low").stdout).toContain("--effort low") expect(withOverride("claude", "max").stdout).toContain("--effort max") - expect(emit("grok-cli").stdout).toContain("--effort high") + expect(emit("grok-cli", cleanEnv()).stdout).toContain("--effort high") expect(withOverride("grok-cli", "medium").stdout).toContain("--effort medium") }) test("CROSS_MODEL_EFFORT_OVERRIDE rejects tiers the route cannot honor, failing closed before dispatch", () => { const rejected = (route: string, value: string) => { - const proc = emit(route, { ...process.env, CROSS_MODEL_EFFORT_OVERRIDE: value }) + const proc = emit(route, { ...cleanEnv(), CROSS_MODEL_EFFORT_OVERRIDE: value }) expect(proc.status).toBe(2) expect(proc.stderr).toContain(`effort override '${value}' not compatible with route '${route}'`) } @@ -302,9 +310,9 @@ describe("ce-work fixed write routes", () => { }) test("opencode carries the override through --variant, matching the review adapters", () => { - const out = emit("opencode", { ...process.env, CROSS_MODEL_EFFORT_OVERRIDE: "max" }).stdout + const out = emit("opencode", { ...cleanEnv(), CROSS_MODEL_EFFORT_OVERRIDE: "max" }).stdout expect(out).toContain("--variant max") - expect(emit("opencode").stdout).not.toContain("--variant") + expect(emit("opencode", cleanEnv()).stdout).not.toContain("--variant") }) test.each(ROUTES)("%s receives one workspace and bounded packet", (route) => { From 54fbfff7cf2128f3b4f63f84780a316292adaab5 Mon Sep 17 00:00:00 2001 From: khsaurabh Date: Sat, 5 Sep 2026 20:52:31 +0530 Subject: [PATCH 5/5] test(ce-work): make the scrubbed environment suite-wide Codex review on #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. --- tests/skills/ce-work-cross-model-routes.test.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/skills/ce-work-cross-model-routes.test.ts b/tests/skills/ce-work-cross-model-routes.test.ts index 4742877e4f..e26cea6698 100644 --- a/tests/skills/ce-work-cross-model-routes.test.ts +++ b/tests/skills/ce-work-cross-model-routes.test.ts @@ -21,6 +21,11 @@ import { createHash } from "node:crypto" setDefaultTimeout(20_000) +// cross-model-work.sh honors CROSS_MODEL_EFFORT_OVERRIDE; make a clean +// environment the suite-wide default so an ambient export cannot leak into +// baseline assertions. Tests that exercise the override set it explicitly. +delete process.env.CROSS_MODEL_EFFORT_OVERRIDE + const SCRIPT = path.join(process.cwd(), "skills/ce-work/scripts/cross-model-work.sh") const CONTROLLER = path.join(process.cwd(), "skills/ce-work/scripts/unit-workspace.py") const SCHEMA = path.join(process.cwd(), "skills/ce-work/references/implementation-result-schema.json")