Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ jobs:
- name: Hook state-machine tests (dash)
run: HOOK_SH=dash dash plugins/dev-workflow/hooks/codex-gate.test.sh

# Invariants 5 and 6 plus two prompt-conformance checks, mechanically, and BOTH
# Invariants 5 and 6 plus three prompt-conformance checks, mechanically, and BOTH
# checkers' regression suites. The
# invariant-12 checker itself is not here — it needs a PR base and runs in the
# step below, so naming this step "version bump" would show a green version-bump
Expand Down
8 changes: 5 additions & 3 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,12 @@ reader can judge whether it still holds.
11. **Prompt changes pass `docs/prompt-standards.md`** — all 12 checklist items, for
any skill, command, agent definition, hook message, or scaffolded template. The
prompts are the product, and **no comprehensive mechanical checker exists for them**:
review is the gate. Two narrow checks in `scripts/check-invariants.sh` cover one
review is the gate. Three narrow checks in `scripts/check-invariants.sh` cover one
spelling each — a `Target model:` line naming exactly one recognized model in files
claiming conformance, and a prose checklist-count claim matching the checklist — and
they are a floor, not coverage. Every other item is judged by a reader.
claiming conformance, a prose checklist-count claim matching the checklist, and the
finding-severity vocabulary stated as a closed set in both prompt copies (in the
scaffolded template's own section, in the command file) — and they are a floor, not
coverage. Every other item is judged by a reader.

## Don'ts

Expand Down
77 changes: 76 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ Append to the gate prompt:
> `gate-a-spec-pass-<p>`, `gate-a-plan-pass-<p>`, or `gate-b-<spec|quality>-pass-<p>`.
>
> One finding per line in the format above; escape a literal pipe inside a field as
> `\|`. Every line before the terminator is exactly one finding line — no blank lines,
> `\|`.
> Severity is one of exactly: BLOCKER | MAJOR | MINOR | NIT — no other token.
> Every line before the terminator is exactly one finding line — no blank lines,
> headings, prose or wrapped continuations. End the file with a final line reading
> exactly `END OF FINDINGS (<n> total)`, `<n>` being the number of finding lines. A
> clean pass is the single body line `NO FINDINGS` with `END OF FINDINGS (0 total)`.
Expand Down Expand Up @@ -149,6 +151,19 @@ file, an `INCOMPLETE` reply — is an **INCOMPLETE pass**, which is not a review
act on the partial list, don't count it toward the 3-pass floor, and don't read "no
Blocker/Major visible" as clean.

**Reader:** the severity field is taken by splitting the line on **unescaped** pipes and
trimming the ASCII whitespace the finding format puts either side of each separator; a field
that is empty or all whitespace is a **structural** failure, so the line is INCOMPLETE and is
never normalized. Otherwise the field is matched **case-insensitively** against the four tokens
first — `Minor`, `minor` and `MINOR` are all `MINOR`, because `CLAUDE.md` Mechanics
legitimately spells them in Title case and a model copying that spelling is doing as it was
told, not drifting. A field that matches no token case-insensitively, and is non-empty, is
read as `MAJOR`. Every **structural** failure stays INCOMPLETE — a malformed
line, a wrong field count, an empty severity field, a bad terminator, a count mismatch. Only
the severity token is tolerated, and only when everything else about the line is right.
(PR #23's Gate-B pass 3 returned all four findings at `IMPORTANT`; discarding that pass over a
token would have thrown away four real findings.)

**Recovery: one attempt per pass**, shared across timeout, an `INCOMPLETE` reply and
failed validation — the Mechanics timeout-retry rule widened, not a second budget beside
it, since two budgets let a pass alternate between them indefinitely. The attempt is a
Expand Down Expand Up @@ -415,6 +430,66 @@ like the rest of §5; the detection is a reader comparing the pass against the s
amend replaces the WIP message wholesale, so an entry written only into the WIP body is
destroyed exactly when the cycle closes. The final commit body is the durable record;
a PR shows commit messages, so there is no second home to keep in sync.

**On squash-merge, copy every evidence entry and every human-exception record in the squash range into the squash body — the squash commit is the only body the merge carries into `main`'s history, so anything left behind is unreachable from it.**

**Recording a human exception.** Where a human decides that something **no applicable rule
required** was nonetheless worth skipping — an optional check this environment cannot run, a
review someone asked for and then stood down, a courtesy step — that decision goes in the
closing commit body:

```
Human exception: <handle> · <date>
Not done: <what was skipped, specifically>
Accepted because: <one line>
```

**Which commit:** an ungated change records it in that commit; a Gate-A cycle in the spec or
plan commit; a Gate-B cycle in the WIP commit, restated by the closing amend. Several records
accumulate; order means nothing.

**A decision made after its commit closed** — during PR review, say — goes in whichever of
these exists: the next commit on the branch, the squash body, or a follow-up commit after the
merge. If none does — the branch is closed, unmerged, and heading for an ordinary or rebase
merge — **add a commit for it.** An empty commit carrying only the record is a legitimate
destination: it changes no content, so it raises no review obligation. A record with nowhere
to go would otherwise be a record that does not exist.

**Do not expect silence from the gate hook, and do not read a reminder as a gate
reopening.** It is advisory, so it never blocks the commit attempt. What is exempt is the
**empty diff**, which `git show --stat` confirms — never a reminder that merely looks the
same on a commit carrying content.

Copy every record into the squash body alongside the evidence entry (Mechanics,
squash-merge carry). **Nothing performs that carry and nothing checks afterwards that it
happened** — it is on whoever prepares the merge. If two copies of one record disagree, that
is a copying error: stop and fix it rather than picking one.

**Scope, and it is narrow. This form supplies no permission.** It records a decision that
was already the human's to make about something genuinely optional. It is **never** the answer to a
below-floor pass, an unclean final pass, a `STOP and surface`, a Gate-A or Gate-B
obligation, or a profile-derived evidence requirement — and more generally **it authorizes
nothing that any mandatory rule in this file or in `AGENTS.md` requires.** Those have their
own terminal actions and this paragraph changes none of them: on a STOP you still stop, and
neither a human's assent nor this record lets an agent close or continue a cycle.

**"Mandatory" is not limited to this file.** A rule in `AGENTS.md`, a project doc, CI, a
branch policy or the platform is equally out of reach — under **Wait for**,
`docs/pr-review-bots.md` requires a bot review unless an explicit recorded human decision
permits proceeding without it, and this form is not that decision. If you are reaching for it to get past something mandatory, the answer
is no — take the operational route or stop.

**Nor is it for things that were simply never owed.** An absent review from a bot routed
**opportunistically** blocks nothing and needs no exception and no record;
`docs/pr-review-bots.md` says so deliberately, and writing one anyway would rebuild the
per-quiet-bot ceremony that routing removed. Record a decision, not a non-event.

**What the record is worth.** It is an **unverified assertion**, and reads as one: nothing
checks that the handle belongs to whoever decided, that a human was asked, or that the
reason is honest. A reader of history learns that *the commit claims* a human chose, what
it says was skipped, and why — no more. It supports no claim of authorization or review,
and satisfies no evidence obligation. It exists because an exception nobody wrote down is
invisible, not because writing it down makes it sound.
- **Timeout / abort:** a codex call that dies at the MCP tool-call timeout is retried
once before surfacing to the user, and that retry *is* the single shared recovery
attempt above — not a second one. An abort is an incomplete pass, so treat it as one:
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ honest gap ([reasoning](docs/coding-workflow.md#adapting-it-to-another-project))
CI ([`.github/workflows/ci.yml`](.github/workflows/ci.yml)) runs four checks on every
PR and push to main: `shellcheck --shell=sh` over all three executables and their test
files, the hook's test suite,
[`scripts/check-invariants.sh`](scripts/check-invariants.sh) (invariants 5 and 6, plus two prompt-conformance checks) plus
[`scripts/check-invariants.sh`](scripts/check-invariants.sh) (invariants 5 and 6, plus three prompt-conformance checks) plus
both checkers' regression suites, and `claude plugin validate . --strict`.

A fifth check runs **on pull requests only**:
Expand Down
104 changes: 104 additions & 0 deletions docs/coding-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,110 @@ real check is noise, and noise trains people to ignore the gate that will eventu
matter. Being explicitly gateless is a known gap you can close; being implicitly
self-reviewed is an unknown one you cannot.

**Choosing which model reviews — and switching when one runs dry.** The invariant names a
model **family**, not a vendor: a pass satisfies a gate when the reviewer is a different family
from the implementer. That leaves the vendor free, which matters because the common failure is
not a bad review, it is **no review** — a quota limit hit mid-cycle, with work blocked and the
gate unsatisfiable. An alternative reviewer is the operational answer, and it is worth wiring
up *before* you need it.

**This section describes the mechanism, not a choice of model.** It names no models and no
recommended default deliberately: model availability, pricing and quality move faster than a
document does, and a list here would be stale before it was useful. A gateway such as
OpenRouter publishes a live catalog — read that for what exists. What follows is how the
plumbing works, so that picking a model is a one-string edit rather than a research project.

**Adding a gateway** to the Codex CLI is one provider block naming the base URL and the
environment variable holding the key:

```toml
[model_providers."<id>"]
name = "<display name>"
base_url = "<gateway base URL>"
env_key = "<ENV VAR HOLDING THE KEY>"
wire_api = "responses"
```

(The table key is quoted because `<id>` is a placeholder: TOML bare keys allow only
`A-Za-z0-9_-`, so the block would not parse with the angle brackets unquoted. Substitute a bare
id and the quotes become optional.)

Adding it changes nothing by itself; `model_provider` still decides who answers. Check
`wire_api` against your CLI version, and check it with `codex doctor` rather than at the first
call. Measured on **codex-cli 0.147.0**: `wire_api = "chat"` makes the whole config fail to
load — `codex doctor` reports `config could not be loaded` — while `"responses"` loads clean.
An arbitrary value fails identically, so `"chat"` is not specially diagnosed, it is simply no
longer accepted. That is the good failure, surfacing at load rather than silently; the version
is named because it is the one this was run against, not because earlier or later ones are
known to differ.

**Four switch surfaces, each a one-string edit**, in the order `mcp-codex-dev` resolves them
(later overrides earlier):

| Surface | Scope | Use it when |
|---|---|---|
| The config the CLI reads (`~/.codex/config.toml`) — its `model` and `model_provider` | every call, all repos | you are changing the standing default |
| `~/.mcp/mcp-codex-dev/config.json` | every repo, this MCP server only | the gate calls need a different model from what the CLI uses by hand |
| `<repo>/.mcp/mcp-codex-dev.config.json` | one repository | a project needs a different reviewer from your default |
| `CODEX_DEV_MODEL` / `CODEX_DEV_REVIEW_MODEL` | current environment — all tools / **Gate B only** | switching per-shell; the `REVIEW` variant changes the code reviewer without touching Gate A |

**One catch worth knowing before you reach for a profile:** `mcp-codex-dev` passes `--model`
and **never `--profile`**, so a CLI profile does not reach the gate calls at all. Because only
the model name is passed, `model_provider` has to be active in the config the CLI reads — a
profile cannot carry the switch. Profiles remain useful for driving the CLI by hand.

**One config, both providers.** Keep the native provider and the gateway entry in the same
config the CLI reads: the top level names no `model_provider`, so the native default answers,
and the appended gateway block is inert until a top-level `model_provider = "<id>"` line
selects it. The switch is that one line — inserted in the top-level block, since a key placed
after any `[table]` header belongs to that table — and the revert is deleting it; the default
returns to the native provider at the next call. Do not point `CODEX_HOME` at a second config
directory to get isolation: the CLI's login state lives beside the config it reads, and a
redirected directory strands the existing login.

Three timing facts decide where an edit lands and when it takes effect. The CLI is spawned
per call and reads its config at start, so the provider switch needs no restart of anything.
`mcp-codex-dev` resolves its *model* chain once per resolved project root and caches it until
the server restarts — the launch root at startup, any other root on its first call — so a model
edit must be in place before the root is first loaded, or be made under a project root the
server has not seen yet. And the key named by `env_key` must be present in the
environment the MCP server was launched with — the CLI inherits it from the server, the
server from its parent at spawn — so an export made after launch reaches nothing until that
parent restarts.

Provider selection cannot travel per-repo: the `mcp-codex-dev` config schema has no
provider key and strips unknown keys, so the per-repo file picks a *model* while
the *provider* stays global to the config the CLI reads.

**Record which model took each pass.** The gate's value comes from independence, so a pass is
only interpretable if you know who gave it. Put the model the pass *ran under* in the pass record
beside the finding count, never one recalled from memory or copied from a document. That is not
always what the config says now: per the timing facts above the model chain is resolved once per
project root and cached until the server restarts — the launch root at startup, any other root on
its first call — so a model edit landed after a root was loaded leaves the configured value and
the running one disagreeing until restart, and the configured value is the wrong one. A root the
server has not loaded yet is the exception: there the edit does take effect. Where they
can disagree, confirm by probing: call `mcp__codex__health` with the same `workingDirectory` you pass to the
gate call. It reports the server's cached per-root resolution, which is what the gate call for
that root uses — the point being that reading the config file yourself is exactly the thing that
can disagree. **Read the per-tool field, not the top-level one:** the server resolves a gate's
model as `tools.<tool>.model ?? model`, so Gate B is `checks.config.effective.tools.review.model`
falling back to `checks.config.effective.model`, and Gate A the same with `tools.exec.model`. The
top-level field alone is the wrong answer precisely where the override documented above is in
use, since `CODEX_DEV_REVIEW_MODEL` is stored at `tools.review.model`. If neither level names a
model the probe establishes nothing — the CLI then picks its own default, and the only honest
record is to set an explicit model or record the model as undetermined. Record the result beside
the finding count in the pass record: the commit body's evidence entry, or the slot's
dispositions file. This is
bookkeeping, not enforcement: nothing checks it, and a wrong entry looks exactly like a right
one.

**The one permanent rule here is family-level.** No model from the **implementer's own family**
satisfies a gate — whatever the vendor, whatever the gateway, whatever the transport. Routing
an Anthropic model through a third-party gateway while Claude is implementing does not make it
independent; it is the same family behind a different bill. Everything else in this section is
configuration and will change. That sentence will not.

### The self-hardening ledger

The system learns from its own findings through an **append-only ledger**. Every
Expand Down
Loading