fix: repair codex invocation and restore model role routing - #7
fix: repair codex invocation and restore model role routing#7metaphorics wants to merge 11 commits into
Conversation
a744dac retiered every preset in the OMP surface, moving the Stop review, precompact metacognition, and both risk-escalated code reviews off the frontier role and pushing base review and bash-failure diagnostics onto fast. FRONTIER_MODEL was left declared but unreachable, collapsing the three-role partition AGENTS.md mandates, and the sibling Python surface still routed those presets to frontier. Seven tests pinning the exact (model, effort) pairs went red and shipped that way. Revert the preset block to the documented role->preset map. The commit carried no rationale and nothing argued against the doc, so the code was the side that drifted. Add ROUTED_MODELS, a derived array of every preset's model, and assert its distinct-role count is three. The per-preset tests own role identity but each pins one preset, so none of them catches a sweep that retiers everything and updates its own expectation in the same edit. Cardinality rather than literal slugs, because AGENTS.md expects the role constants to be re-pinned on a lineup rename; asserting against the constants instead would dedupe alongside an aliased role and pass on the very collapse it guards. Verified both ways: the guard fails on the collapse and survives a rename of all three slugs.
codex-cli 0.147.0 rejects --full-auto outright: `codex exec ... --full-auto` exits with "unexpected argument '--full-auto' found" before doing any work. invokeCodex treats a non-zero exit as a fail-open and returns "", so every caller hit its `if (!raw) return undefined` path. The bash pre-guard let every command through, the Stop gate settled without ever blocking, and code-change review was dropped. The reflector was a complete no-op on both surfaces, not a degraded one. --sandbox read-only is the read-only guarantee, as AGENTS.md already said, so the flag bought nothing even on a codex that accepted it. The suite could not see this: it stubs codex with a fake that ignores argv, so all 141 tests passed against an argv the real CLI refuses. Move the OMP argv into an exported codexExecArgs and check every long flag against the installed `codex exec --help`, which is local and needs no auth. That catches any rejected flag, not just this one, and skips when codex is absent. Verified against the real binary: the old argv fails at parse, the new argv launches Codex v0.147.0 and reaches the API. A full round trip is unconfirmed - the account is over its usage limit until Aug 8 - so the proof covers argument acceptance, which is the layer this fixes.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe change updates reviewer routing, centralizes read-only Codex arguments, removes ChangesCodex routing and release updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant OMPTests
participant codexExecArgs
participant codexReflector
participant CodexCLI
OMPTests->>codexExecArgs: Generate Codex flags
codexReflector->>codexExecArgs: Build invocation arguments
codexReflector->>CodexCLI: Execute read-only command
OMPTests->>CodexCLI: Validate flags against --help
CodexCLI-->>OMPTests: Return validation and smoke-test results
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@omp/codex-reflector.test.ts`:
- Around line 250-259: Update the `codex exec --help` validation in the test so
it skips only when `spawnSync` reports an `ENOENT` error, indicating Codex is
not installed. Fail the test for other spawn errors and for non-zero exit
statuses, while retaining the existing flag checks when help succeeds.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 53980b88-94b2-4acd-8cf0-8ec285c71fe4
📒 Files selected for processing (9)
.claude-plugin/marketplace.json.claude-plugin/plugin.json.cursor-plugin/marketplace.json.cursor-plugin/plugin.jsonAGENTS.mdomp/codex-reflector.test.tsomp/codex-reflector.tspackage.jsonscripts/codex-reflector.py
💤 Files with no reviewable changes (1)
- scripts/codex-reflector.py
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 9 files
Architecture diagram
sequenceDiagram
participant Client as Agent (Claude/Cursor)
participant OMP as OMP Hook (invokeCodex)
participant Python as Python Hook (invoke_codex)
participant Args as codexExecArgs Builder
participant CLI as codex exec CLI
participant TempFile as Temp Output File
participant Gate as Review Gate (bash/stop)
Note over Client,Gate: NEW: Codex invocation flow with validated flags
alt OMP Surface
Client->>OMP: review request (category, content)
OMP->>Args: codexExecArgs(effort, model, outPath)
Note over Args: NEW: Centralized argv builder, exported for testing
Args-->>OMP: [exec, --sandbox, read-only, --skip-git-repo-check, --ephemeral, -c, -m, -o, -]
else Python Surface
Client->>Python: review request
Python->>Python: Build inline argv
Note over Python: --full-auto removed
end
Note over OMP,Python: Both surfaces now use same flag set (no --full-auto)
OMP->>CLI: spawn("codex", args) with stdin prompt
Python->>CLI: subprocess(["codex", ...]) with stdin prompt
alt CLI accepts flags
CLI->>CLI: Parse accepted flags
CLI->>CLI: Run model with sandbox read-only
CLI->>TempFile: Write review output
CLI-->>OMP: Exit 0
OMP->>TempFile: Read output
TempFile-->>OMP: Review text
OMP-->>Client: Verdict (PASS/FAIL)
else CLI rejects flag
Note over CLI: --full-auto would cause this
CLI-->>OMP: Exit non-zero
OMP->>OMP: invokeCodex returns ""
OMP-->>Client: undefined (fail-open)
Note over Client,Gate: Gate settles, never blocks (silent no-op)
end
Note over Client,Gate: Model routing flow (restored)
Client->>OMP: stopReview() / precompact()
OMP->>OMP: Lookup preset in ROUTED_MODELS
Note over OMP: NEW: stopReview → FRONTIER_MODEL, precompact → FRONTIER_MODEL
alt Stop Review
OMP->>OMP: STOP_REVIEW: model=FRONTIER_MODEL, effort=medium
Note over OMP: Previously was DEFAULT_MODEL
else Precompact metacognition
OMP->>OMP: PRECOMPACT: model=FRONTIER_MODEL, effort=low
Note over OMP: Previously was DEFAULT_MODEL
end
Note over OMP: Guards (test-time only)
Test->>Args: codexExecArgs(medium, gpt-5.6-sol, /tmp/out)
Args-->>Test: flags array
Test->>Test: Assert --sandbox read-only present
Test->>Test: Assert --skip-git-repo-check present
Test->>Test: Assert --full-auto absent
alt codex CLI available locally
Test->>CLI: spawnSync("codex", ["exec", "--help"])
CLI-->>Test: help text
Test->>Test: Verify every --flag is in help text
else codex not installed
Test->>Test: Skip flag validation
end
Test->>Test: Check ROUTED_MODELS cardinality
Note over Test: Assert exactly 3 distinct models reachable
Note over Test: Catches role collapse (e.g. FRONTIER_MODEL = DEFAULT_MODEL)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
@coderabbitai ultrareview |
|
|
|
@coderabbitai ultrareview |
|
✅ Action performedFull review finished. |
|
@codex review |
|
@coderabbitai ultrareview |
|
✅ Action performedFull review finished. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
- BASH_FAILURE: default@medium -> default@low to match Python surface and reduce post-hoc diagnostic cost (thread PRRT_kwDORKAmpc6XQIdu). - Flag test: skip only on ENOENT (codex not installed), fail on any other spawn error or non-zero exit; preserves validation in CI while catching broken installs (threads PRRT_kwDORKAmpc6XQId6, CodeRabbit body).
d2601db to
61373b9
Compare
Addressed in 61373b9:
All three covered inline: BASH_FAILURE fixed (thread PRRT_kwDORKAmpc6XQIdu), flag guard fixed (PRRT_kwDORKAmpc6XQId6), hard/complex declined with handler-budget harm cited (PRRT_kwDORKAmpc6XQIdz). |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
omp/codex-reflector.test.ts (1)
220-235: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAssert the complete model-routing contract.
The distinct-role check can pass with an incorrect model or effort assignment. Assert the exact model/effort pair for every preset, including
Stopasfrontier@medium. Add a test that preserves an explicit effort override. Clear and restoreCODEX_REFLECTOR_MODELaround preset assertions so ambient state cannot mask the defaults.As per coding guidelines, model-routing tests must assert exact model/effort pairs, cover Stop frontier@medium and override effort preservation, and isolate ambient
CODEX_REFLECTOR_MODELduring preset assertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@omp/codex-reflector.test.ts` around lines 220 - 235, Extend the ROUTED_MODELS tests to assert each preset’s exact model/effort pair, including Stop using frontier@medium, rather than only checking distinct-role cardinality. Add coverage proving an explicit effort override is preserved, and clear CODEX_REFLECTOR_MODEL before preset assertions, restoring the original ambient value afterward so defaults are tested reliably.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@omp/codex-reflector.test.ts`:
- Around line 220-235: Extend the ROUTED_MODELS tests to assert each preset’s
exact model/effort pair, including Stop using frontier@medium, rather than only
checking distinct-role cardinality. Add coverage proving an explicit effort
override is preserved, and clear CODEX_REFLECTOR_MODEL before preset assertions,
restoring the original ambient value afterward so defaults are tested reliably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 42f5ac67-e3aa-40c8-bf0a-a5c32ba1264e
📒 Files selected for processing (2)
omp/codex-reflector.test.tsomp/codex-reflector.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- omp/codex-reflector.ts
Restore OMP hard/complex presets to match Python surface _ME_CODE_REVIEW_HARD=frontier@high and _ME_CODE_REVIEW_COMPLEX=frontier@xhigh per AGENTS parity; update gateModelEffort tests accordingly. Revisits thread PRRT_kwDORKAmpc6XQIdz (previously declined on handler-budget grounds — aligning per explicit parity requirement; follow-up will verify handler budget in live review). OMP 1.3.10 / Claude 1.2.14.
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Fixes a broken CLI flag that disabled the entire reflector; restores documented model routing presets that had silently drifted. All changes revert to previously agreed-upon and tested behavior, with new guards that confirm the fix. No new product or operational tradeoffs introduced.
Re-trigger cubic
Verifies CODE_REVIEW_COMPLEX=xhigh stays within HANDLER_BUDGET_MS=25s: measured 2026-08-09 xhigh with 5.5k payload ~18.3s direct, ~20.1s via codexExecArgs smoke test, both <25s. Test is opt-in (CODEX_SMOKE=1) to avoid 6-18s cost on every run; spawn timeout 60s > budget so assertion can fail on overrun, bun timeout 65s > spawn so full round-trip is observed. Documents that xhigh is viable within OMP 25s budget vs Python ~100s guard.
![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: 14e5f0fa2e
ℹ️ 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".
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Dismissed because Cubic found issues in a newer review.
…dget Rename prior live smoke to 'xhigh direct Codex invocation latency' — it measures codexExecArgs->codex only, not handlerDeadline, compaction, prompt construction, invokeCodex, or cleanup at tool_result:1140-1160. Add complementary handler integration smoke that drives the registered tool_result handler with a 5.5k CODE_REVIEW_COMPLEX payload through the full path and asserts settlement < HANDLER_BUDGET_MS=25s. Live: direct ~10.6s, handler ~18.2s, both <25s (CODEX_SMOKE=1).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@omp/codex-reflector.test.ts`:
- Around line 267-269: Correct the date in the measurement comment associated
with the live Codex latency smoke test, replacing the future August 9, 2026 date
with the verified measurement date; if no verified date exists, remove the
measurement-date claim while preserving the latency context.
- Around line 1158-1171: Update the xhigh handler integration test around
codexReflector and its tool_result event to use a path with an xhigh file
heuristic, such as .env.local, instead of src/complex.ts. Add an assertion that
the selected model and effort pair matches the xhigh route, while preserving the
existing opt-in smoke-test behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ac759597-5561-42a9-9d35-df199e7145d2
📒 Files selected for processing (5)
.claude-plugin/marketplace.json.claude-plugin/plugin.jsonomp/codex-reflector.test.tsomp/codex-reflector.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (3)
- .claude-plugin/marketplace.json
- .claude-plugin/plugin.json
- omp/codex-reflector.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 088397286c
ℹ️ 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".
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Codex timeout 25s killed xhigh handler reviews at budget (measured 19-25s via handler, direct ~10s). Handler budget 25s equaled codex timeout, so deadline and child SIGKILL raced and smoke hid the expiry as undefined. Raise to coherent hierarchy: codex 26s < handler 28s < harness 30s (2s margins each). Keeps 2s under oh-my-pi's hard 30s cap while giving xhigh headroom.
…andler Direct smoke comment still said 25s after handler budget moved to 28s; update to 26s codex / 28s handler. Handler smoke previously skipped on any undefined (hiding deadline expiry at 25s codex timeout); now asserts elapsed < HANDLER_BUDGET_MS before skip and only skips fast unavailability (<5s), failing on deadline/budget expiry. Also fix missing 65s test closing and use xhigh-triggering path src/auth/credentials.test.ts for true xhigh coverage.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7eb9cde7e3
ℹ️ 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".
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Assert exact category/model/effort triples for positive classify routes, including bash-failure default@low. Match generated long flags against complete codex exec --help option tokens rather than substrings. Exercise CODEX_REFLECTOR_MODEL effort preservation on the distinctive frontier@high route.
Retier OMP complex reviews from frontier@xhigh to frontier@high after a live xhigh call took 37.588s and the full handler failed open at the 26s child timeout under the host's fixed 30s cap. Frontier@high returned a defined review in 11.025s under the same load. Keep Python xhigh under its ~100s guard, pin both complex gate disjuncts, and retain direct plus full-handler live smokes. Release as OMP 1.3.12 / Claude 1.2.16.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude-plugin/plugin.json:
- Line 3: Update the paired marketplace manifest’s metadata.lastUpdated to a
non-future release date matching the plugin version 1.2.16: use 2026-08-09 for a
release shipping today, or defer the metadata update until 2026-08-10 if
shipping tomorrow.
In `@omp/codex-reflector.test.ts`:
- Around line 1196-1199: Strengthen the assertion after the undefined check in
the handler integration test to require a non-empty reviewer response. Validate
that result.content contains reviewer text, while preserving the existing
elapsed-time budget assertion and undefined-result rejection.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c8942f3-9c66-4181-ba0f-c2524b6c22df
📒 Files selected for processing (6)
.claude-plugin/marketplace.json.claude-plugin/plugin.jsonAGENTS.mdomp/codex-reflector.test.tsomp/codex-reflector.tspackage.json
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- .claude-plugin/marketplace.json
- omp/codex-reflector.ts
Strengthen the live tool_result smoke to match the Codex verdict header and require a non-empty opinion body before checking latency. This rejects defined-but-empty content overrides; mutation with an empty codeReviewResponse body fails at the intended assertion.
Addressed: positive classify routes now pin exact category/model/effort triples, including bash-failure at gpt-5.6-terra@low. Gate tests pin both complex disjuncts. Existing argv captures pin Stop at frontier@medium, precompact at frontier@low, and bash guard at fast@low. The model-override test now uses a distinctive frontier@high route, clears/restores ambient CODEX_REFLECTOR_MODEL, and proves the effort survives an explicit model override.
Addressed: the help check skips only missing Codex, throws other spawn failures, requires exit 0, and matches generated long flags against complete option-name tokens. The live direct smoke also throws on every installed-Codex non-zero exit.
Addressed in 2c32762: the full handler smoke matches the Codex Review verdict header and target path, then requires a non-empty trimmed opinion body before checking latency. An empty-body mutation fails at the new assertion. The paired marketplace date remains 2026-08-10 because that is the authoritative session and release date. The inline date thread has the full disposition. |
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Confidence score: 5/5
- In
omp/codex-reflector.test.ts, theif (!handler) return;branch is unreachable afterexpect(handler).toBeDefined(), which can hide test intent and create misleading dead-path logic without affecting runtime behavior—remove the redundant guard (or replace the assertion/flow) so the test has a single, clear failure path.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="omp/codex-reflector.test.ts">
<violation number="1" location="omp/codex-reflector.test.ts:1183">
P3: The `if (!handler) return;` guard is unreachable: `expect(handler).toBeDefined()` on the line above already throws when the handler is undefined, so `!handler` is always false when execution reaches that branch. If the contract being defended is that `tool_result` must be registered, the `expect` already enforces it; consider dropping the dead guard so a missing registration fails the smoke test loudly instead of silently skipping.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| codexReflector(pi); | ||
| const handler = handlers.get("tool_result"); | ||
| expect(handler).toBeDefined(); | ||
| if (!handler) return; |
There was a problem hiding this comment.
P3: The if (!handler) return; guard is unreachable: expect(handler).toBeDefined() on the line above already throws when the handler is undefined, so !handler is always false when execution reaches that branch. If the contract being defended is that tool_result must be registered, the expect already enforces it; consider dropping the dead guard so a missing registration fails the smoke test loudly instead of silently skipping.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At omp/codex-reflector.test.ts, line 1183:
<comment>The `if (!handler) return;` guard is unreachable: `expect(handler).toBeDefined()` on the line above already throws when the handler is undefined, so `!handler` is always false when execution reaches that branch. If the contract being defended is that `tool_result` must be registered, the `expect` already enforces it; consider dropping the dead guard so a missing registration fails the smoke test loudly instead of silently skipping.</comment>
<file context>
@@ -1155,34 +1156,57 @@ exit 0
+ codexReflector(pi);
+ const handler = handlers.get("tool_result");
+ expect(handler).toBeDefined();
+ if (!handler) return;
+ const event: Record<string, unknown> = {
+ type: "tool_result",
</file context>
1 participant
Summary
The reflector has not been reviewing anything. Every
codex execcall passed--full-auto, a flagcodex-cli0.147.0 rejects during argument parsing, so the child died before doing any work.invokeCodextreats a non-zero exit as fail-open and returns"", and every caller reads that as "no verdict" and settles. The bash pre-guard waved through every command it was asked to gate, the Stop gate never blocked, and code-change review was dropped. Not degraded — off, on both the OMP and Claude/Cursor surfaces.Nothing caught it because the suite stubs
codexwith a fake that ignores argv. All 141 tests passed against an argv the real CLI refuses.Separately, the OMP surface had drifted off its documented model routing: the Stop gate and precompact metacognition ran on the everyday model instead of frontier,
FRONTIER_MODELwas declared but unreachable, and seven tests pinning exact(model, effort)pairs were red onmain.Both are fixed, and both now have a guard that fails if they come back.
Why every gate went quiet
--sandbox read-onlyis what actually provides the read-only guarantee, asAGENTS.mdalready said, so the flag bought nothing even on a codex version that accepted it. Removed from both surfaces.Model routing restored
a744dac("update") retiered every preset and carried no rationale. Reverted to the documented role-to-preset map:mainCODE_REVIEWluna@highterra@mediumCODE_REVIEW_HARDterra@mediumsol@mediumCODE_REVIEW_COMPLEXterra@highsol@highBASH_FAILUREluna@highterra@mediumBASH_GUARDluna@mediumluna@lowSTOP_REVIEWterra@mediumsol@mediumPRECOMPACTterra@lowsol@lowReverting rather than updating the doc, because four independent things agreed the doc was right:
AGENTS.mdstates load-bearing product reasons for each rule, the sibling Python surface still followed them, seven tests pinned the exact pairs, and the commit made no argument for the change. A doc is only stale when something argues against it.The practical cost of the drift:
STOP_REVIEWis the only post-hoc gate that can block, and it was running on the everyday model to save cost.Guards added
Role cardinality.
ROUTED_MODELSderives every preset's model, and a test asserts three distinct roles remain reachable. The per-preset tests own role identity, but each pins one preset, so none of them catches a sweep that retiers everything and updates its own expectation in the same edit — which is exactly what landed.Cardinality rather than literal slugs, because
AGENTS.mdexpects the role constants to be re-pinned when OpenAI renames the lineup, and a slug list would fail that legitimate edit. Asserting against the constants instead would be worse: aliasingFRONTIER_MODELtoDEFAULT_MODELis the collapse, and both sides would dedupe together and pass.Real-CLI flag acceptance. The OMP argv moved into an exported
codexExecArgs, and a test checks every long flag against the installedcodex exec --help— local, no auth, skipped when codex is absent. This catches any rejected flag, not just this one, and closes the hole that let a fake-codex suite bless a broken argv.Validation
bun test omp/codex-reflector.test.tspython3 scripts/codex-reflector.py --test-parseruff check scripts/Both guards were mutation-verified rather than assumed:
DEFAULT_MODELfails the cardinality test; renaming all three slugs passes it.--full-autofails bothcodexExecArgstests.Against the real binary: the old argv fails at parse with
error: unexpected argument '--full-auto' found; the new argv launchesOpenAI Codex v0.147.0and reaches the API.A full review round trip is unconfirmed — the Codex account is over its usage limit until Aug 8. The proof here covers argument acceptance, which is the layer this PR fixes. Worth one live review once quota resets.
Commit
9a29ca0was verified green in isolation before22755b7was layered on, so either can be bisected to independently.Release manifests
Each commit carries its own paired bump per
AGENTS.md: OMP1.3.6→1.3.8, Claude1.2.10→1.2.12across all three fields withlastUpdatedrefreshed. Cursor1.2.5→1.2.6, bumped only in the commit that touches the shared Python hook.Known gap, not addressed here
Nothing in this repo runs the suite before a push, which is how a commit with seven failing tests reached
main. These guards catch these two regressions; a pre-push hook would catch the class. Left out deliberately to keep this PR scoped.Summary by cubic
Fixes a no-op Codex invocation by removing
--full-auto, restores model role routing, and retiers OMP complex reviews to frontier@high to stay under the 30s host cap with a 26s/28s timeout hierarchy.Bug Fixes
--full-autofrom both OMP/Pythoncodex exec; exportedcodexExecArgsand added a--helpvalidation that checks every long flag (skips only whencodexis missing; fails on non‑zero exit or other spawn errors).default@medium; hardfrontier@high; complex nowfrontier@highon OMP (Python staysfrontier@xhigh); Stop review and precompact on frontier;BASH_FAILUREtodefault@low;BASH_GUARDtofast@low. AddedROUTED_MODELScardinality test and pinned classify routes to exact category/model/effort triples; ensuredCODEX_REFLECTOR_MODELoverrides model only. UpdatedAGENTS.mdwith the frontier‑high rule for OMP.CODEX_TIMEOUT_MS/HANDLER_BUDGET_MS.Dependencies
codex-reflector-ompto1.3.12, Claude plugin to1.2.16, Cursor plugin to1.2.6.Written for commit 2c32762. Summary will update on new commits.
Summary by CodeRabbit
Improvements
Bug Fixes
Chores