Skip to content

fix(bin): thread codex max reasoning effort instead of dropping it - #1203

Open
talktejas wants to merge 3 commits into
kunchenguid:mainfrom
talktejas:fm/fm-codex-effort-ceiling
Open

fix(bin): thread codex max reasoning effort instead of dropping it#1203
talktejas wants to merge 3 commits into
kunchenguid:mainfrom
talktejas:fm/fm-codex-effort-ceiling

Conversation

@talktejas

Copy link
Copy Markdown

Intent

Fix a silent thinking-level downgrade when firstmate spawns a Codex worker. bin/fm-spawn.sh capped codex effort at xhigh and silently omitted the flag for max, so a caller asking for max got the worker's ambient default effort instead - a two-step downgrade with no error. Empirically verified against the installed codex-cli 0.145.0: the model catalog advertises max on the GPT-5.6 family, a live codex exec run at max reported 'reasoning effort: max', and an invalid value is rejected by the API with a visible 400 rather than downgraded. Chose to raise the ceiling rather than refuse loudly, because max is genuinely supported and, since the codex case now covers the entire vocabulary fm-spawn already validates (low|medium|high|xhigh|max), no requested level can fall through unmatched - the drop path is structurally gone rather than guarded. ultra is deliberately excluded even though the catalog advertises it: its parallel-agent fan-out changes execution shape and must never nest inside firstmate supervision. Scope was deliberately limited to codex; grok, opencode, kimi, claude and pi behaviour is intentionally unchanged, including grok's genuine low|medium|high ceiling. The old test test_codex_omits_invalid_max_effort was replaced by test_codex_threads_every_accepted_effort_level, which loops all five levels and asserts each reaches the launch command; it was confirmed to fail against the old code. The single-level codex test was folded into that loop as now-redundant. The harness-adapters skill's codex verification stamp was updated from 0.142.1 to 0.145.0. This is firstmate's own tracked material, so repo style applies: one sentence per line in Markdown, plain dash, no agent co-author.

What Changed

  • bin/fm-spawn.sh now includes max in the codex branch of effort_flag_for_harness, so --effort max emits -c 'model_reasoning_effort="max"' instead of silently omitting the flag and letting the worker fall back to its ambient default. The codex case now covers the full vocabulary fm-spawn already validates (low|medium|high|xhigh|max), so no accepted level can fall through unmatched; ultra stays excluded because its parallel-agent fan-out must not nest inside firstmate supervision.
  • bin/fm-bootstrap.sh crew_dispatch_validate accepts codex/max as well, so a config-driven dispatch profile agrees with the direct spawn path. grok, opencode, kimi, claude and pi effort vocabularies are unchanged, including grok's genuine low|medium|high ceiling.
  • Tests: test_codex_omits_invalid_max_effort was replaced by test_codex_threads_every_accepted_effort_level, which loops all five levels and asserts each reaches the launch command (the single-level codex test folded into it as redundant); the two bootstrap crew-dispatch rows now assert codex max is accepted. The harness-adapters skill's codex verification stamp moved from codex-cli 0.142.1 to 0.145.0 with the max evidence recorded.

Risk Assessment

✅ Low: The follow-up commit is a two-line vocabulary alignment plus the two test rows that asserted the contradicted behavior, leaving every other harness and the excluded ultra level untouched, and it closes the only reachable divergence found in round 1.

Testing

I ran the two suites the change touches — fm-spawn-dispatch-profile and fm-bootstrap — and both pass, then went past pass/fail to capture the actual command firstmate would run: driving real fm-spawn through the suite's fake tmux at each effort level shows codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="max"' ..., while the same driver against the base commit's fm-spawn.sh produces the same command with no effort flag at all, reproducing the silent downgrade the change fixes and confirming xhigh was unaffected. A live fm-bootstrap run confirms the config-facing half: a crew-dispatch rule with codex/max is now accepted silently, and grok/max still fails with invalid effort: grok:max, so the deliberate scope limit to codex holds. No UI surface is involved, so the reviewer-visible evidence is CLI transcripts rather than screenshots; the worktree is clean and all evidence lives in the temp evidence directory.

Evidence: codex launch command, base vs fixed, per effort level

=== BEFORE (base fa0d85d): max silently loses the effort flag === --effort xhigh -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="xhigh"' --dangerously-bypass-approvals-and-sandbox ... --effort max -> codex --model 'gpt-5.6-codex' --dangerously-bypass-approvals-and-sandbox ... === AFTER (c266166): every accepted level reaches codex === --effort low -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="low"' --dangerously-bypass-approvals-and-sandbox --effort medium -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="medium"' --dangerously-bypass-approvals-and-sandbox --effort high -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="high"' --dangerously-bypass-approvals-and-sandbox --effort xhigh -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="xhigh"' --dangerously-bypass-approvals-and-sandbox --effort max -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="max"' --dangerously-bypass-approvals-and-sandbox

=== BEFORE (base fa0d85d): max silently loses the effort flag ===
[base fa0d85d] fm-spawn ... --effort xhigh  -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="xhigh"' --dangerously-bypass-approvals-and-sandbox -c "notify=[\"bash\",\"-c\",\"touch ' "$('.../bin/fm-operational-input.sh' encode launch-brief < '
[base fa0d85d] fm-spawn ... --effort max    -> codex --model 'gpt-5.6-codex' --dangerously-bypass-approvals-and-sandbox -c "notify=[\"bash\",\"-c\",\"touch ' "$('.../bin/fm-operational-input.sh' encode launch-brief < '

=== AFTER (c266166): every accepted level reaches codex ===
[fixed c266166] fm-spawn ... --effort low    -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="low"' --dangerously-bypass-approvals-and-sandbox 
[fixed c266166] fm-spawn ... --effort medium -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="medium"' --dangerously-bypass-approvals-and-sandbox 
[fixed c266166] fm-spawn ... --effort high   -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="high"' --dangerously-bypass-approvals-and-sandbox 
[fixed c266166] fm-spawn ... --effort xhigh  -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="xhigh"' --dangerously-bypass-approvals-and-sandbox 
[fixed c266166] fm-spawn ... --effort max    -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="max"' --dangerously-bypass-approvals-and-sandbox 
Evidence: fm-bootstrap crew-dispatch validation: codex:max accepted, grok:max still rejected

--- codex effort=max --- (no CREW_DISPATCH complaint - config accepted) --- grok effort=max --- CREW_DISPATCH: invalid config/crew-dispatch.json - invalid effort: grok:max

--- codex effort=max ---
(no CREW_DISPATCH complaint - config accepted)
--- grok effort=max ---
CREW_DISPATCH: invalid config/crew-dispatch.json - invalid effort: grok:max
Evidence: Full per-level launch commands and recorded state meta (fixed code)
[fixed c266166] fm-spawn ... --effort low    -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="low"' --dangerously-bypass-approvals-and-sandbox -c "notify=[\"bash\",\"-c\",\"touch '/tmp/fm-codex-effort-evidence.gnc5zc/ev-low/home/state/ev-low.turn-ended'\"]" "$('/home/tds/.no-mistakes/worktrees/3605d2c32b02/01KYMW35QNBBD19PSX9KPJATPF/bin/fm-operational-input.sh' encode launch-brief < '/tmp/fm-codex-effort-evidence.gnc5zc/ev-low/home/data/ev-low/brief.md')"
            state meta        -> harness=codex model=gpt-5.6-codex effort=low 
[fixed c266166] fm-spawn ... --effort medium -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="medium"' --dangerously-bypass-approvals-and-sandbox -c "notify=[\"bash\",\"-c\",\"touch '/tmp/fm-codex-effort-evidence.gnc5zc/ev-medium/home/state/ev-medium.turn-ended'\"]" "$('/home/tds/.no-mistakes/worktrees/3605d2c32b02/01KYMW35QNBBD19PSX9KPJATPF/bin/fm-operational-input.sh' encode launch-brief < '/tmp/fm-codex-effort-evidence.gnc5zc/ev-medium/home/data/ev-medium/brief.md')"
            state meta        -> harness=codex model=gpt-5.6-codex effort=medium 
[fixed c266166] fm-spawn ... --effort high   -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="high"' --dangerously-bypass-approvals-and-sandbox -c "notify=[\"bash\",\"-c\",\"touch '/tmp/fm-codex-effort-evidence.gnc5zc/ev-high/home/state/ev-high.turn-ended'\"]" "$('/home/tds/.no-mistakes/worktrees/3605d2c32b02/01KYMW35QNBBD19PSX9KPJATPF/bin/fm-operational-input.sh' encode launch-brief < '/tmp/fm-codex-effort-evidence.gnc5zc/ev-high/home/data/ev-high/brief.md')"
            state meta        -> harness=codex model=gpt-5.6-codex effort=high 
[fixed c266166] fm-spawn ... --effort xhigh  -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="xhigh"' --dangerously-bypass-approvals-and-sandbox -c "notify=[\"bash\",\"-c\",\"touch '/tmp/fm-codex-effort-evidence.gnc5zc/ev-xhigh/home/state/ev-xhigh.turn-ended'\"]" "$('/home/tds/.no-mistakes/worktrees/3605d2c32b02/01KYMW35QNBBD19PSX9KPJATPF/bin/fm-operational-input.sh' encode launch-brief < '/tmp/fm-codex-effort-evidence.gnc5zc/ev-xhigh/home/data/ev-xhigh/brief.md')"
            state meta        -> harness=codex model=gpt-5.6-codex effort=xhigh 
[fixed c266166] fm-spawn ... --effort max    -> codex --model 'gpt-5.6-codex' -c 'model_reasoning_effort="max"' --dangerously-bypass-approvals-and-sandbox -c "notify=[\"bash\",\"-c\",\"touch '/tmp/fm-codex-effort-evidence.gnc5zc/ev-max/home/state/ev-max.turn-ended'\"]" "$('/home/tds/.no-mistakes/worktrees/3605d2c32b02/01KYMW35QNBBD19PSX9KPJATPF/bin/fm-operational-input.sh' encode launch-brief < '/tmp/fm-codex-effort-evidence.gnc5zc/ev-max/home/data/ev-max/brief.md')"
            state meta        -> harness=codex model=gpt-5.6-codex effort=max 
Evidence: Evidence driver script
#!/usr/bin/env bash
# Evidence driver: what codex launch command does firstmate actually build for
# each requested effort level? Uses the suite's fake tmux to capture the literal
# command fm-spawn would send.
set -u
ROOT=${FM_EVIDENCE_ROOT:?}
. "$ROOT/tests/lib.sh"
SPAWN="$ROOT/bin/fm-spawn.sh"
TMP_ROOT=$(fm_test_tmproot fm-codex-effort-evidence)

label=${1:-current}
for effort in low medium high xhigh max; do
  case_dir="$TMP_ROOT/ev-$effort"
  home="$case_dir/home"; proj="$case_dir/project"; wt="$case_dir/wt"
  launchlog="$case_dir/launch.log"
  fakebin=$(fm_fakebin "$case_dir/fake")
  cat > "$fakebin/tmux" <<'SH'
#!/usr/bin/env bash
set -u
case "$*" in *"#{pane_current_path}"*) printf '%s\n' "${FM_FAKE_PANE_PATH:-}"; exit 0 ;; esac
case "${1:-}" in
  display-message) printf 'firstmate\n'; exit 0 ;;
  list-windows) exit 0 ;;
  has-session|new-session|new-window|kill-window) exit 0 ;;
  send-keys)
    prev=
    for a in "$@"; do
      [ "$prev" = "-l" ] && printf '%s\n' "$a" >> "$FM_FAKE_LAUNCH_LOG"
      prev=$a
    done
    exit 0 ;;
esac
exit 0
SH
  chmod +x "$fakebin/tmux"
  fm_fake_exit0 "$fakebin" treehouse pi-signed
  mkdir -p "$home/data" "$home/projects" "$home/state" "$home/config"
  printf 'codex\n' > "$home/config/crew-harness"
  fm_git_worktree "$proj" "$wt" "wt-ev-$effort" >/dev/null 2>&1
  touch "$home/state/.last-watcher-beat"
  id="ev-$effort"
  mkdir -p "$home/data/$id"; printf 'brief\n' > "$home/data/$id/brief.md"
  : > "$launchlog"
  FM_ROOT_OVERRIDE='' FM_HOME="$home" \
    FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \
    FM_PROJECTS_OVERRIDE="$home/projects" FM_CONFIG_OVERRIDE="$home/config" \
    FM_SPAWN_NO_GUARD=1 FM_FAKE_PANE_PATH="$wt" TMUX="fake,1,0" \
    FM_FAKE_LAUNCH_LOG="$launchlog" PATH="$fakebin:$PATH" \
    "$SPAWN" "$id" "$proj" --model gpt-5.6-codex --effort "$effort" >/dev/null 2>&1
  printf '[%s] fm-spawn ... --effort %-6s -> %s\n' "$label" "$effort" "$(head -1 "$launchlog")"
  printf '            state meta        -> %s\n' "$(grep -E '^(harness|model|effort)=' "$home/state/$id.meta" | tr '\n' ' ')"
done

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed ✅
  • ⚠️ bin/fm-bootstrap.sh:722 - The crew-dispatch config validator still caps codex effort at xhigh, so the raised ceiling only applies to the direct spawn path. Concrete path: config/crew-dispatch.json containing {"rules":[{"when":"big feature","use":{"harness":"codex","model":"gpt-5","effort":"max"}}]} makes crew_dispatch_validate print 'CREW_DISPATCH: invalid config/crew-dispatch.json - invalid effort: codex:max', while the identical profile passed as fm-spawn --harness codex --effort max now launches with -c 'model_reasoning_effort="max"'. tests/fm-bootstrap.test.sh:780 and :797 assert that rejection, so the two owners of codex's effort vocabulary (effort_flag_for_harness and effort_ok) now disagree and the disagreement is test-locked. The intent's claim that no requested level can fall through unmatched holds for the launch command but not for the config-driven dispatch profile that feeds it. Earliest supported fix: add "max" to the codex list in effort_ok and update the two bootstrap test expectations, keeping grok's genuine low|medium|high ceiling untouched.
  • ℹ️ bin/fm-spawn.sh:602 - The codex case now matches the entire vocabulary that fm-spawn.sh:197 validates, so the branch can never fall through — it is functionally an unconditional printf. That is the intended structural property (the drop path is gone rather than guarded), and keeping the explicit list documents the verified vocabulary; noting only so the redundancy is not mistaken for a live guard.

🔧 Fix: accept codex max effort in crew-dispatch validator
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-spawn-dispatch-profile.test.sh — all pass, including the new test_codex_threads_every_accepted_effort_level looping low|medium|high|xhigh|max
  • bash tests/fm-bootstrap.test.sh — all pass, including the flipped codex max effort is accepted and array profile codex max effort is accepted crew-dispatch rows
  • Manual before/after driver /tmp/no-mistakes-evidence/01KYMW35QNBBD19PSX9KPJATPF/codex-effort-evidence.sh run against both git checkout fa0d85d -- bin/fm-spawn.sh and the target c266166 (file restored afterwards; worktree clean) to capture the literal codex launch command per effort level
  • Live bin/fm-bootstrap.sh run with a real config/crew-dispatch.json containing codex/effort:max and grok/effort:max to compare validator output
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

codex-cli 0.145.0 accepts the whole shared effort vocabulary through max,
so fm-spawn no longer omits the flag for max.
Omitting it left the worker at the ambient default effort with no error,
a silent downgrade of what the caller asked for.
The codex case now covers every level fm-spawn accepts, so no requested
level can be dropped; an unsupported value would fail visibly at the API.
ultra stays out of the vocabulary because its parallel-agent fan-out does
not belong inside firstmate supervision.
The portable serial lane tests the pull-request merge ref, and the merge base
carried a teardown conformance case that pinned tmux's old permissive
kill-window selector.
That case fails against main's exact-selector adapter, so the lane failed on
material this branch never touched.
Main already replaced the pinned assertion with a selector-form-agnostic one;
merging it makes the branch's merge state green and matches what CI runs.
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