Raise the plan-review codex ceiling 1200s -> 2400s

[igerber]

Summary

Raises CODEX_TIMEOUT_S in the plan-review skill from 1200s to 2400s, updates the one line of SKILL.md that states the ceiling to the reader, and strengthens the contract test so the two cannot drift apart again.

Why. The 1200s cap was calibrated against Campaign 1's plan-review runtimes (up to ~430s), but those plans were far smaller than production ones. A dense ~500-line plan whose reviewer must verify claims against 15+ repo files runs 3–4× that: codex timed out on 6 of 9 rounds of a single v4 plan, every failure a genuine

codex exec timed out after 1200.0s (model=gpt-5.6-sol, effort=xhigh)

— never an auth or availability error. Plan review is simply the slow surface: its reviewer reads the plan and cross-checks every claim against the live tree.

Why not retry instead. Retrying was tried first and is strictly worse: a failed attempt burns the full ceiling before the retry starts, so a round that retried once timed out twice — 40 minutes for no review. Retrying a too-low ceiling just pays it twice.

Why this needs no engine re-validation. The "do not change without re-validation" note binds CODEX_MODEL and CODEX_EFFORT — what Campaign 1 actually graded. The ceiling is an operational knob graded by nothing, and the campaign itself ran unattended at CODEX_TIMEOUT_S=3600, so 2400 is still below the value the graded configuration used. The detection prompts, model, and effort are untouched.

Doc/code consistency. SKILL.md tells the reader codex_review.py "caps at Ns and exits 3", so bumping the constant alone would leave the skill's own instructions describing behavior the code no longer has. The existing contract test asserted only the code constant and could not catch that. It now derives both values and compares them rather than hard-coding the number twice — verified to bite by injecting the old value and confirming the test fails.

Trade-off, stated rather than buried: 2400s is a 40-minute interactive ceiling. It is a cap, not a wait — reviews that finish in five minutes still finish in five — but a genuinely hung codex now takes twice as long to reach the loud single-Claude fallback. Accepted because every observed failure was a clean timeout on real verification work, not a hang. The opposite reading is defensible: a plan needing >20 minutes of verification may be a plan that should be split.

Methodology references (required if estimator / math changes)

• Method name(s): N/A — no estimator, math, or library changes. This touches .claude/skills/plan-review/ tooling and its contract test only.
• Paper / source link(s): N/A
• Any intentional deviations from the source (and why): N/A

Validation

• Tests added/updated: tests/test_plan_review_skill.py — test_reviewer_invocations_are_pinned extended to derive the ceiling from both codex_review.py and SKILL.md and assert they agree.
• Full plan-review contract suite passes (26 tests).
• Negative check: re-injecting the stale 1200s value into SKILL.md makes the new assertion fail, confirming the pin is not vacuous.
• black --check and ruff clean on both changed files.
• No library code touched, so no estimator suites are affected.

Security / privacy

• Confirm no secrets/PII in this PR: Yes

[github-actions]

Overall Assessment

✅ Looks good — no unmitigated P0 or P1 findings.

Executive Summary

• Changes only the plan-review operational timeout from 1200s to 2400s.
• No causal estimator, identification, weighting, variance, SE, or inference behavior is affected.
• Code, documentation, and contract tests consistently use the new ceiling.
• The longer worst-case fallback delay is intentional, documented, and bounded.
• No security or correctness concerns found.

Methodology

No findings. No registered causal method or methodological assumption is affected.

• Severity: None
• Impact: None
• Concrete fix: None

Code Quality

No findings. The timeout remains a named constant and its rationale is documented.

• Severity: None
• Impact: None
• Concrete fix: None

Performance

The maximum wait before fallback increases by 20 minutes.

• Severity: P3 — Informational
• Impact: A genuinely hung Codex process may delay fallback for up to 2400 seconds.
• Concrete fix: None required; this is an explicit, bounded operational trade-off documented in .claude/skills/plan-review/codex_review.py.

Maintainability

No findings. The new test compares the documented ceiling with the runtime constant, preventing drift.

• Severity: None
• Impact: None
• Concrete fix: None

Tech Debt

No untracked technical debt introduced.

• Severity: None
• Impact: None
• Concrete fix: None

Security

No secrets, unsafe command construction, or permission changes are present.

• Severity: None
• Impact: None
• Concrete fix: None

Documentation/Tests

Documentation and both timeout assertions were updated consistently. The synchronization assertion covers future code/documentation drift.

• Severity: None
• Impact: None
• Concrete fix: None