feat: rig edition — tier routing, typed envelopes, quota failover, fanout/council/cloud verbs#488
Open
nategarelik wants to merge 5 commits into
Open
feat: rig edition — tier routing, typed envelopes, quota failover, fanout/council/cloud verbs#488nategarelik wants to merge 5 commits into
nategarelik wants to merge 5 commits into
Conversation
openai#236 reports the app-server broker hanging indefinitely at "Initializing..." on Windows with no error surfaced. The direct spawn path (used by the broker itself to launch `codex app-server`, and by runCommand for utilities like taskkill) wraps the child process with process.env.SHELL on win32, trusting whatever shell the user happens to have configured (git-bash, a WSL bash.exe shim, an unresolvable literal POSIX path, etc.) to correctly proxy a JSON-RPC stdio pipe. That is unpredictable: it can silently swallow the handshake, fail with ENOENT, or otherwise misbehave depending on what SHELL resolves to, and the broker's own cleanup logic already assumed the spawned child was cmd.exe (see the terminateProcessTree comment in app-server.mjs), which breaks when SHELL points elsewhere. process.env.SHELL was added in openai#178 to fix openai#138, but that fix targeted the wrong layer: the outer spawn wrapper only resolves the codex executable itself (npm installs ship a .cmd shim that CreateProcess cannot exec directly) and has no bearing on what shell Codex uses internally for its own command-execution tool, since SHELL is already passed straight through via the `env` option regardless of this wrapper. Reverting to a deterministic cmd.exe wrapper on Windows fixes the hang without reopening openai#138. Verified: baseline test suite has 6 pre-existing failures including "cancel sends turn interrupt to the shared app-server before killing a brokered task" (a taskkill/terminateProcessTree test whose process tree assumptions broke exactly the way described above); with this fix that test passes and no other test regresses (5 failures remain, all pre-existing and unrelated: platform-mismatch path assertions, npm/setup env assumptions, Claude-session-transfer path assumptions). A live headless round trip against `codex app-server` (broker start, initialize, thread/start, turn/start, turn/completed, clean shutdown) also passes end to end.
Cross-model adversarial review of 8b49e99 found a remaining gap: on Windows, `shell: true` doesn't invoke cmd.exe directly -- Node falls back to process.env.ComSpec (defaulting to cmd.exe only when ComSpec is unset). A nonstandard ComSpec (pointed at PowerShell, a shell shim, etc.) can therefore reproduce the exact openai#236 hang through ComSpec that 8b49e99 already fixed for SHELL. Add resolveWindowsShell() in process.mjs, which joins SystemRoot (set by the OS itself, not a dev-tooling convention like SHELL/ComSpec) with System32\cmd.exe, and use it at both spawn sites instead of the plain `shell: true` boolean. Neither SHELL nor ComSpec can redirect the wrapper now. runCommand's explicit `options.shell` override is preserved. Verified: full test suite matches the previous fix's pass/fail split (the two extra failures seen during one run were load-induced flakes under heavy concurrent-agent CPU contention on this machine -- 300+ node.exe processes at the time -- confirmed by re-running both in isolation, where they pass cleanly). Live headless round trip against codex app-server (thread/start, turn/start, turn/completed, clean shutdown) passes end to end with this build.
…ilover Adds an additive lib/rig-edition.mjs layer composing the existing lib/codex.mjs turn path: sol/terra/luna model-tier resolution with per-verb defaults, envelope parse/validate helpers matching the DONE|DONE_WITH_CONCERNS|NEEDS_CONTEXT|BLOCKED contract, and one-step quota failover (sol -> terra -> luna, then BLOCKED/QUOTA_EXHAUSTED). codex-companion.mjs gains an optional --tier flag on `task` that resolves model/effort through the new tier table; omitting --tier keeps the prior --model/--effort behavior byte-for-byte.
Adds three additive orchestration verbs on top of the rig-edition tier routing layer: - fanout: N brief specs -> one disjoint git worktree per worker -> concurrent tiered Codex turns (hard cap 5, extra briefs queue behind it), aggregated orchestrator-side into results/failed/quota_exhausted. Worktree cleanup only runs when --cleanup is passed, and a cleanup failure never discards a worker's real outcome (a throw inside finally would otherwise replace it) -- it only annotates the entry with cleanupWarning, since a live smoke showed the Codex app-server process can keep a worktree directory locked past turn completion. - council: M seats (default 3) debate a topic in two rounds -- round-1 advocate/counter seats run concurrently and in isolation from each other, round-2's decider seat sees both transcripts and produces the verdict. Falls back to the raw reply when a seat's turn has no parseable envelope, since council seats are prompted to debate, not to emit structured envelopes. - cloud: envelope-wrapping passthrough over the verified `codex cloud` CLI surface (exec/status/list/apply/diff; EXPERIMENTAL upstream). Wires all three into codex-companion.mjs as new subcommands with minimal, additive routing, matching the existing task/review command shape.
A createWorktree throw (branch collision, existing path, held lock) was outside runFanoutWorker's error handling, so it rejected the Promise.all concurrency lane and discarded every other worker's result plus the queued briefs. Wrap it to emit a failed-bucket entry instead. Found via cross-model adversarial review (gpt-5.6-sol).
There was a problem hiding this comment.
Pull request overview
Adds an additive “rig edition” layer on top of the existing Codex broker/app-server integration, introducing tier-aware routing, typed envelopes, quota failover, and new higher-level verbs (fanout/council/cloud) wired into codex-companion.
Changes:
- Introduces
rig-edition.mjsfor model-tier aliasing, per-verb defaults, envelope parsing/validation, and quota step-down behavior. - Adds new verb modules:
fanout(worktree-based parallelism),council(two-round debate + decider), andcloud(argv builder + envelope wrapper aroundcodex cloud). - Updates companion wiring/rendering and hardens Windows spawning by using a deterministic
cmd.exeshell wrapper.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rig-edition.test.mjs | Adds unit tests for tier routing, envelope parsing, and quota failover. |
| tests/fanout.test.mjs | Adds tests for fanout parsing, concurrency cap, bucketing, and cleanup behavior. |
| tests/council.test.mjs | Adds tests for council seat planning, prompt isolation, and agreement notes. |
| tests/cloud.test.mjs | Adds tests for cloud argv mapping and envelope wrapping. |
| plugins/codex/scripts/lib/rig-edition.mjs | New tier routing + envelope parsing + quota failover entry points. |
| plugins/codex/scripts/lib/fanout.mjs | New fanout verb: worktree workers + concurrency-limited orchestration. |
| plugins/codex/scripts/lib/council.mjs | New council verb: round-1 isolation + decider round. |
| plugins/codex/scripts/lib/cloud.mjs | New cloud verb: codex cloud argv builder + envelope wrapper. |
| plugins/codex/scripts/lib/render.mjs | Adds renderers for fanout/council/cloud results. |
| plugins/codex/scripts/lib/process.mjs | Adds deterministic Windows shell resolver; uses it in runCommand. |
| plugins/codex/scripts/lib/app-server.mjs | Uses deterministic Windows shell for app-server spawn. |
| plugins/codex/scripts/codex-companion.mjs | Wires new verbs + adds --tier support to task. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+58
to
+63
| if (!isNonEmptyString(raw.id)) { | ||
| throw new Error(`Brief at index ${index} is missing a non-empty string "id".`); | ||
| } | ||
| if (seenIds.has(raw.id)) { | ||
| throw new Error(`Duplicate brief id "${raw.id}".`); | ||
| } |
| * from here. | ||
| * | ||
| * @typedef {"sol" | "terra" | "luna"} ModelTier | ||
| * @typedef {"low" | "medium" | "high" | "xhigh" | "max" | "ultra"} ReasoningEffort |
Comment on lines
+35
to
+36
| const VALID_EFFORTS = new Set(["low", "medium", "high", "xhigh", "max", "ultra"]); | ||
| const SOL_ONLY_EFFORTS = new Set(["max", "ultra"]); |
Comment on lines
+239
to
+241
| if (isValidEnvelopeShape(parsed)) { | ||
| return Object.freeze({ ...parsed }); | ||
| } |
Comment on lines
+278
to
+282
| export async function runFanout(repoRoot, worktreeRoot, briefs, options = {}) { | ||
| if (!Array.isArray(briefs) || briefs.length === 0) { | ||
| throw new Error("runFanout requires a non-empty array of brief specs."); | ||
| } | ||
|
|
![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: 94026da0d0
ℹ️ 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".
| } | ||
|
|
||
| try { | ||
| return await fn({ tier: startTier.tier, modelId: startTier.modelId }); |
There was a problem hiding this comment.
When the app-server reports quota during a turn, runAppServerTurn records it in the resolved result's error/status fields rather than necessarily throwing. This unconditional return treats a 429-shaped result as success, so fanout/council callers never step down tiers or receive a QUOTA_EXHAUSTED envelope in that path; classify the returned outcome before returning.
Useful? React with 👍 / 👎.
| export function runCloudCommand(subcommand, options = {}) { | ||
| const runner = options.runCommand ?? runCommand; | ||
| const args = buildCloudArgs(subcommand, options); | ||
| const result = runner("codex", args, { cwd: options.cwd, shell: false }); |
There was a problem hiding this comment.
This forces runCommand to bypass its Windows default shell. For npm-installed Codex on Windows the executable is the codex.cmd shim (the adjacent app-server spawn was changed for this), so codex cloud ... fails before the CLI starts whenever only that shim is on PATH; let runCommand choose its default shell or use the resolved Windows shell on win32.
Useful? React with 👍 / 👎.
| runAppServerTurn(cwd, { | ||
| ...turnOptions, | ||
| model: modelId, | ||
| effort: resolvedDefaults.effort |
There was a problem hiding this comment.
When a caller starts on sol with effort: "max" or "ultra" and the first attempt hits quota, the retry switches only the model to terra while keeping the sol-only effort. That makes the fallback request invalid for the lower tier, so quota failover turns into a non-quota failure instead of producing the intended fallback or QUOTA_EXHAUSTED envelope.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Additive "rig edition" layer on top of the app-server broker, built as separate files for cheap upstream rebases. Companion to #484 (the Windows broker fix this branch builds on).
What it adds
DONE | DONE_WITH_CONCERNS | NEEDS_CONTEXT | BLOCKEDwith defensive, prose-tolerant parsing (never throws on garbage).QUOTA_EXHAUSTEDenvelope so the caller can fall back.fanout(parallel worktree workers, cap 5, pre-flight file-overlap rejection, worktree-failure containment, divergence-preserving aggregation),council(advocate/counter/decider with round-1 isolation),cloud(wraps the realcodex cloudsubcommands — verified 1:1 against 0.144.1--help).Testing
148→ tests total (27 new for the rig-edition layer); the pre-existing env-dependent failures are unchanged and unrelated. Verified on Windows 11, codex-cli 0.144.1.
codex cloudarg mapping checked against the live CLI (zero deltas).Notes
Everything is additive (
lib/rig-edition.mjs+ verb modules + minimal companion wiring); no existing signatures changed, so callers that do not opt into a tier alias are byte-for-byte unaffected.