feat(implement): default model route flips clodex → claude (C6) - #35
feat(implement): default model route flips clodex → claude (C6)#35SSFSKIM wants to merge 3 commits into
Conversation
implement-dispatch.sh resolved a label-less ticket to the clodex gateway route (GPT models via the local cliproxy). Opus 5-class models now beat that route for implementation work, so the fallback becomes `claude`. The gateway route itself is untouched — `engine:codex` on a ticket, or WORKER_ENGINE=codex in the environment, still spawns a gateway-routed worker with the same settings/effort plumbing. Only the default moved. Resume consistency needs no code: daemon-spawn writes the settings/effort meta keys only when the env vars are non-empty, and daemon-resume re-adds --settings/--effort only when those keys are non-empty, so a claude-route worker persists nothing and resumes plain. Prose that named the old default moves with it (issue-tracker dispatch ritual steps 2-3, sweep-setup knobs table). The knobs table now states both lane defaults, since WORKER_ENGINE is shared with the review/land dispatchers, which keep codex. Closes #33
![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: 12fad0ff0f
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| engine="${T_ENGINE_LABEL:-}" | ||
| [ -n "$engine" ] || engine="${WORKER_ENGINE:-codex}" | ||
| [ -n "$engine" ] || engine="${WORKER_ENGINE:-claude}" |
There was a problem hiding this comment.
On the documented Linux worker-host path, this fallback is never reached: infra/worker-host/README.md tells operators to copy env.example into ~/.env, and that file still exports WORKER_ENGINE=codex. For new or rebuilt worker hosts using the supported setup, label-less implement/spike tickets will therefore continue to route through the gateway instead of the new plain-Claude default, while review/land also remain forced to codex. Please update or remove the seeded WORKER_ENGINE value (or split per-lane knobs) so the intended default flip actually applies in that environment.
Useful? React with 👍 / 👎.
SSFSKIM
added
the
engine:claude
The label-less `claude` default entered the plain spawn branch with no env prefix, so a dispatcher running inside a gateway-routed daemon leaked its own DAEMON_CLAUDE_SETTINGS/DAEMON_CLAUDE_EFFORT into daemon-spawn.sh. That applied --settings/--effort AND persisted both into the registry meta, so daemon-resume.sh restored them on every later fork: the worker rode the gateway forever while the dispatch log said engine=claude. Clear both on the plain spawn, the same command-prefix shape the codex branch already uses. The test suite now exports an ambient gateway route for its whole run, which turns the three existing "no gateway env" assertions into regression tests (they fail on the pre-fix dispatcher).
The shipped worker-host env pinned WORKER_ENGINE=codex globally. That file
is sourced under `set -a` from ~/.bashrc and copied verbatim into the
Actions runner env (README §2.6), so on the documented host every sweep
tick and event dispatch reached implement-dispatch.sh with WORKER_ENGINE
already exported — the new claude default for a label-less implement/spike
ticket never took effect in production.
Removing the assignment is safe for the other lanes: review-dispatch.sh:265
and land-dispatch.sh:154 each carry their own ${WORKER_ENGINE:-codex}
fallback, so they stay on the gateway with the var unset. The comment now
states the per-lane defaults instead of the retired global one.
SSFSKIM
added
the
confident-ready
Review trail — PR #35 (ticket #33)Verdict: approve, HUMAN tier. Reviewed at head EngineEngine: native
Exit condition met: no new blocker at r3. The two P2/P3 items r3 raised are new Compliance auditVerdict: no PROTOCOL BLOCKER. One SPEC FINDING (found by me, independently
AUDIT NOTES (trail only, never merge blockers):
Closing-artifact cross-check (Validation Evidence)Every command-backed claim was re-run. Results:
Findings and routing
Routing departures, stated: r1-1 was ranked P2 by the engine (default LOG) but Fix wavesBoth waves: one item, dispatched to a general-purpose fixer, task tree quiesced Wave 1 — W1-1 Wave 2 — W2-1 Tier judgmentHUMAN tier. The self-merge tier was not satisfied — this is not
Auto-merge is off in any case. The clauses it did meet: final verdict approve; One thing to know before merging#39 is an operational follow-up, not a code one: if any worker host was Reviewed by the autonomous review worker ( |
1 participant
Closes #33
Roadmap child C6 of
docs/doperpowers/specs/2026-07-26-claude-review-stack-roadmap.md.implement-dispatch.shresolved a label-less ticket to the clodex gateway route(GPT models through the local cliproxy). Opus 5-class models now beat that route
for implementation work, so the final fallback becomes
claude. The gatewayroute is left entirely intact — retiring clodex is a separate, later unit.
What moved
skills/implementing-tickets/scripts/implement-dispatch.sh— the fallbackliteral, plus the
WORKER_ENGINEheader docs.skills/issue-tracker/SKILL.md— dispatch-ritual step 2 (the default) andstep 3 (the claude route now reads first, as the default, with the codex
gateway prefix second).
skills/issue-tracker/references/sweep-setup.md— the knobs table now namesboth lane defaults, because
WORKER_ENGINEis shared with the review/landdispatchers, which deliberately keep
codexuntil C5: review-loop worker default route clodex → claude + plumbing removal #32 (C5).X4 (route label semantics) is preserved.
engine:codexon a ticket stillroutes to the gateway;
engine:claudeis redundant now but still valid andstill wins over the env;
WORKER_ENGINE=codexstill overrides the default.Each of those four is a separate test case below.
Resume consistency needed no code.
daemon-spawn.shwrites thesettings/effortmeta keys only whenDAEMON_CLAUDE_SETTINGS/DAEMON_CLAUDE_EFFORTare non-empty, anddaemon-resume.shre-adds--settings/--effortonly when those meta keys are non-empty. A claude-routeworker therefore persists nothing and resumes plain — the invariant holds by
construction. Verified by reading both scripts (
daemon-spawn.sh:46-60,daemon-resume.sh:37-70), and asserted spawn-side by thesettings=;effort=cases.
Out of scope by the ticket's own constraint:
review-dispatch.shandland-dispatch.shkeep theircodexdefault (roadmap C5 / #32), and nogateway plumbing was removed.
Validation Evidence
Claim: the change is genuinely test-driven — the new assertions fail against
the old default. With only the one-line fallback reverted to
codexand everyother file at its new state,
bash tests/implementing-tickets/test-implement-dispatch.shreported exactly the three new/changed assertions as failures:
The revert was then undone (
grep -n "WORKER_ENGINE:-" …confirms${WORKER_ENGINE:-claude}on line 179;git statusshows no stray.bak).Claim: a label-less implement dispatch spawns a plain-Claude worker.
bash tests/implementing-tickets/test-implement-dispatch.sh→all tests passed,exit 0, 41 passing assertions. The success criteria are covered by these cases:
#1default engine claude passes no gateway env(settings=;effort=)#1claude route pins no model — it inherits unless IMPLEMENT_MODEL says otherwiseengine:codexticket#7(new seed row)engine:codex label opts back into the gateway routeengine:codexticket#7label-selected codex route pins the gateway model alias(model=fable)engine:claudeticket#5engine:claude label … still suppresses the gateway envWORKER_ENGINE=codex+ label-less#1WORKER_ENGINE=codex overrides the claude default on a label-less ticketWORKER_ENGINE=codex+engine:claude#5engine:claude label still wins over WORKER_ENGINE=codexClaim: the sweep path picks up the new default too (the ticket asked for this
path to be checked explicitly).
--sweepmode sharesdispatch_one, and the newsweep assertion pins it behaviorally: across a full sweep of the seeded board,
the gateway settings string appears exactly once in the spawn log — the
engine:codexticket — while every other worker rides the claude default.Claim: nothing else regressed. Each run separately, all exit 0:
bash tests/implementing-tickets/test-protocol-content.sh→all tests passedbash tests/issue-tracker/test-board-scripts.sh→all tests passedbash tests/issue-tracker/test-board-sweep.sh→all tests passedbash tests/reviewing-prs/test-review-dispatch.sh→all tests passed(thereview lane's
codexdefault is untouched and still asserted)bash tests/reviewing-prs/test-land-dispatch.sh→all tests passedbash tests/skill-links/test-cross-doc-refs.sh→all cross-doc references resolveClaim: the changed shell is lint-clean.
bash scripts/lint-shell.sh→ exit 0,and
shellcheck -S warningrun directly on both changed shell files → exit 0,no output. (See Confusions — the repo lint script alone does not cover them.)
Confusions
origin/main. The ticket citesdocs/doperpowers/specs/2026-07-26-claude-review-stack-roadmap.md, whichexists only on the local
main(commits7a93b3c,5ecdd5b,1f42bc5),unpushed from a parallel session. This branch is cut from
origin/main(
8a0cefe), so the file is absent from my worktree; I read the C6 section viagit show main:…to confirm the acceptance criteria. The unpushed commitstouch only that one doc, so there is no file overlap with this branch and no
rebase was warranted — but a reviewer following the ticket's spec link on
origin/mainwill not find the file until that session pushes.scripts/lint-shell.shreports "Linting 2 shell files" — it is a curatedbaseline, not a full sweep, and neither file I changed is in it. Its passing
is therefore not evidence about this diff, which is why I ran
shellcheckdirectly on both changed files as well.
Follow-ups
FOLLOW-UPS: none.
The one residual I considered —
WORKER_ENGINEis a single env var now carryingdifferent defaults per lane (claude for implement/spike, codex for review/land) —
is transient and already owned: roadmap C5 (#32) flips the review and land
dispatchers to the same default, at which point the asymmetry disappears. I
documented it in the knobs table rather than registering a duplicate ticket.