feat(codex): run anywhere, declare a concurrency limit, resolve model aliases - #10
Merged
Merged
Conversation
codex refuses to start outside a git repository — "Not inside a trusted directory and --skip-git-repo-check was not specified" — and nothing in this library's surface let a caller lift that guard: no Request field, no option, no argv passthrough. The dialect asymmetry landed on the caller instead, where the same Request succeeds on claudecode and dies at spawn here. The check is a guardrail for an interactive human who may have opened codex in the wrong directory. `codex exec` is the scripted entry point: there is nobody to warn, and where a scripted child runs is the caller's deliberate choice. The flag now goes on every invocation, ahead of the positional prompt, the way claudecode opens every argv with --setting-sources "".
A caller that wants to run several agents at once had no way to learn that it must not. codex authenticates from auth.json under CODEX_HOME, a file it rewrites in place whose refresh tokens are effectively single-use; Claude Code authenticates from a static bearer token in an environment variable that nothing rewrites. So the same fan-out is correct on one provider and corrupts a credential on the other, and the only route left to a caller was a switch on the provider ID. ConcurrencyLimiter is that capability, discovered by type assertion like every other. codex answers 1; claudecode does not implement it, and absent means unconstrained. It carries a count rather than a boolean because a boolean states only what the assertion already states, and it lives on the provider rather than the Driver because a Codex profile holding a session outranks the token Isolated injects — a driver answering "isolated, therefore unbounded" would be describing a profile it cannot see. docs/adr/0005 records both, and why a config dir per run is not a way around the limit.
Request.Model meant two different things depending on the provider: a family on claudecode, an exact vendor string on codex, so a caller writing configuration had to know which dialect it was addressing to know what it could write. codex now implements ModelResolver over a table of OpenAI's own family names, and anything unrecognised — a concrete ID and an unknown family alike — is passed through, because the CLI is the authority on what it accepts and rejecting here would make a model shipped after this file unreachable through it. The vocabulary is deliberately not shared with claudecode. A "sonnet" that also meant something here would have this library assert that one vendor's model is the counterpart of another's, and a caller swapping providers in a manifest would silently get a model nobody chose.
The orchestrator gains an unconditional merge_group trigger: a queued pull request reports its required check from that event and no other, so without it a queue waits forever on a check nothing can report.
|
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.
Three codex changes that all remove a reason a caller has to know which dialect
it is addressing. They ship together as v0.6.0.
codex execalways skips the git repository checkcodex refuses to start outside a git repository — "Not inside a trusted
directory and --skip-git-repo-check was not specified" — and nothing in this
library's surface let a caller lift that guard: no Request field, no option, no
argv passthrough. The comment naming it a caller's responsibility named a
responsibility with no mechanism to discharge it, and the driver's own
integration suite worked around it by
git init-ing a temp directory.The check is a guardrail for an interactive human who may have opened codex in
the wrong directory.
codex execis the scripted entry point: there is nobodyto warn, and where a scripted child runs is the caller's deliberate choice.
claudecode has no equivalent check, so the same Request succeeded on one
provider and died at spawn on the other.
The flag is now on every invocation, ahead of the positional prompt, the way
claudecode opens every argv with
--setting-sources "". The integrationsuite's
workspacehelper is a plaint.TempDir()— that it reaches theprompt at all is the end-to-end proof.
A provider declares whether its runs can be concurrent
ConcurrencyLimiteris a new optional interface, discovered by type assertionlike every other capability. codex implements it and answers
1; claudecodedoes not implement it, and absent means unconstrained.
codex authenticates from
auth.jsonunderCODEX_HOME— a file it rewrites inplace, whose refresh tokens are effectively single-use. Claude Code
authenticates from a static bearer token in an environment variable that nothing
rewrites. So "run four agents in parallel" was correct on one provider and
corrupted a credential on the other, and the only route left to a caller was
if id == "codex".Three decisions, recorded in
ADR 0005:
Driver. A driver could account for thecredential mode it was built with, but the answer would be wrong where it
matters most: a
CODEX_HOMEprofile holding a session outranks the tokenIsolatedinjects, so "isolated, therefore unbounded" would be describing aprofile the driver cannot see.
Driver.MaxConcurrentRunsforwards theprovider's answer and adds nothing, the way
Driver.ResolveModeldoes.WithConfigDirdoes not lift the limit. Copies of one session are notindependent sessions — the same single-use refresh token is in every copy, so
the first refresh invalidates the rest and can invalidate the source profile.
Considered and rejected in the ADR, because it is the obvious wrong fix.
MustSerialise() boolstates exactly whatimplementing the interface already states, leaving a caller sizing a pool with
nothing to size it from.
Installer.SigningIdentityis the precedent: amethod carries a value.
codex resolves family aliases
Request.Modelmeant a family on claudecode and an exact vendor string oncodex, so a caller writing configuration had to know which dialect it was
addressing to know what it could write.
codex/model.gomirrorsclaudecode/model.go:astra,sol,terra,lunaandminiresolve toconcrete builds, and anything unrecognised passes through, because the CLI is
the authority on what it accepts.
The vocabulary is OpenAI's own rather than one shared across providers. A
shared one would let a manifest swap providers without rewriting models, and it
would buy that by having this library assert that some OpenAI model is "the
sonnet one" — an editorial claim it has no standing to make, failing in the
direction resolution exists to prevent.
Downstream
agentic-toolkit's
agtk code-reviewruns reviewers in a neutral workingdirectory that is not the reviewed code (its ADR 0007). Once it bumps to
v0.6.0 it can delete the
git initof that directory, and replace itsif name == "codex"serialisation withDriver.MaxConcurrentRuns().