Skip to content

review-pr on codex workers is a silent no-op: sandbox blocks gh review POSTs; verdicts adjudicated without any posted review — quality gate bypassed #4204

Description

@Trecek

Problem

Under a codex orchestrator, review_pr steps are routed to codex workers (the review-pr skill declares uses_capabilities: [agent_model, cross_skill_ref, run_skill] — nothing that triggers the R0 claude-code auto-route), and the codex workspace-write sandbox blocks the network writes the skill performs via gh (batch POST /pulls/{N}/reviews). Result: the review quality gate is a silent no-op — the sessions run, compute findings, emit verdict structured-output that the orchestrator adjudicates and routes on, but nothing is ever posted to GitHub.

Evidence (codex-impl-loop attempt 5, 2026-07-07, dispatch 0ef8f012, 0.10.851)

  • Three review_pr children ran on backend=codex (sessions at 15:23Z ok, 15:39Z ok, 15:57Z adjudicated_failure); two resolve_review children ran on claude-code (15:30Z, 15:44Z) — against empty comment sets.
  • PR compose-pr: retry gh pr create with backoff and guard resilience #4203 after merge: GET /pulls/4203/reviews0, /pulls/4203/comments0, /issues/4203/comments0. Zero review artifacts despite 3 review sessions and 2 resolve rounds.
  • The pipeline proceeded to CI → merge queue → merged 16:11Z. CI and merge queue still gated, but the LLM review layer contributed nothing.
  • Contrast: resolve-review declares git_metadata_write and was correctly routed to claude-code workers.

Expected behavior / fix shapes

  1. review-pr (and any skill whose contract requires GitHub API writes) must declare a capability that routes it to a backend that can perform them — either reuse the existing backend-requirements mechanism (R0 auto-route, Rectify: Capability-Driven Backend Auto-Route (R0) — Codex Orchestrators Dispatch git_metadata_write Skills to Claude Code Workers #4176) or introduce a github_api_write capability consumed the same way.
  2. Truth-telling contract: review-pr should verify its batch POST landed (e.g., re-read the review count) before emitting verdict = ... — a verdict without a posted review should be an explicit failure, not an adjudicated success. (Note HTTP 200 is the success signal per project rules; the point is the POST never happened at all in the sandbox.)
  3. Optionally: codex sandbox network policy for skills that declare network needs.

Related

Operator clarification (priority framing)

There are two distinct defects, and the fix priority differs from the original fix-shape ordering above:

  1. review-pr does not actually run its GitHub side on codex. Review on codex workers is desired — there are deliberately no model/provider overrides for review steps, and routing review to codex is correct. The defect is that the codex sandbox blocks the skill's gh review POSTs. The primary fix is to make review-pr actually work on codex (sandbox network policy for skills that need GitHub API access, or an equivalent server-side posting path — e.g. the skill returns findings and an MCP tool performs the batch POST server-side, outside the sandbox). Forcing claude-code routing is a fallback, not the goal.
  2. The pipeline continued without any posted reviews. Independent of backend: the review gate must verify its own effect (posted review exists on the PR) before a verdict is accepted, and the recipe must not proceed to enqueue/merge when the review step produced zero review artifacts. An adjudicated verdict with no posted review should be a hard failure of the step.

Route: remediation with investigation. Full evidence in this issue body; the run's ledger is at the loop driver's .autoskillit/temp/codex-loop/ledger.json (attempt 5, dispatch 0ef8f012).

Prior Remediation Attempt (round 1, 2026-07-07)

Dispatch fb62e674 / session dbc4741e ran 2h06m and failed at the test-fix stage — context limit in resolve-failures — but produced substantial preserved work on branch impl-4204-20260707-111500 (pushed to origin):

  • 80946ef93 — "feat: add github_api_write capability with network_access wiring (Part A)" — the primary fix direction per the operator clarification (make review work on codex via sandbox network policy).
  • a4f049f43 — "fix: resolve test failures from github_api_write capability implementation" — 44 of 46 test failures fixed.

Remaining known failures (2): related to _has_routing_capability() matching over-broadly for open_kitchen — the routing predicate needs narrowing so open_kitchen is not treated as requiring the new capability route.

Guidance for the next round: build on the pushed branch rather than reimplementing Part A — fetch it, fix the 2 remaining _has_routing_capability() failures, and continue with Part B (the merge-without-reviews gate verification defect), tests, and PR.

Prior Remediation Attempt (round 2, 2026-07-07)

Dispatch 045d509c ran 1h30m and was killed by the stale-suppression bound (max_suppression_seconds=1800 exceeded at 2402s while legitimately blocked on its implement child — same defect family as #4202, claude-code variant; evidence appended there). Substantial work preserved on pushed branch impl-4204-20260707-130220 (~8 commits, supersedes round 1's branch):

  • Part A re-implemented: github_api_write capability + network_access wiring (a04268aac), test fixes (2906d3324)
  • worker_routable discriminator on SkillCapabilityDef + routing predicate updates (3a02b509d) — the proper fix for the over-broad _has_routing_capability() open_kitchen failures
  • Multi-line gh api line-continuation normalizer in _detect_capabilities (1cb6d8c89)
  • uses_capabilities declarations for three skills (0ed6fd102)
  • Part B: degradation contract for gh network failure in review-pr (3bcfb703e)
  • Routing negative/positive tests + admission negative test (41e247f4b), stale-test updates (9900edd58)

Guidance for round 3: the implementation appears near-complete on that branch — fetch it, run the full test suite, fix any residual failures, verify Part B actually blocks merge-with-zero-reviews, then PR/review/merge. Do not reimplement.

Prior Remediation Attempt (round 3, 2026-07-07)

Dispatch 2046994e ran 2h22m; watchdog mitigations held (no infra kill). Part B implemented: check_review_posted effect-verification gate (bd249e238) plus contract/guard/test corrections (fe8931e8f) on pushed branch impl-4204-20260707-151055. The assess step then reduced test failures 313 → 70 before hitting its context limit.

The remaining 70 failures are mechanical and tightly scoped: check_review_posted step configuration wiring across the 4 bundled recipes (recipe-contract/graph tests cascade when a step is added).

Guidance for round 4 — SCOPE STRICTLY: fetch branch impl-4204-20260707-151055 (Part B) and branch impl-4204-20260707-130220 (Part A + worker_routable, from round 2 — verify both parts are present in the final diff; merge/rebase as needed). Do NOT reimplement either part. Fix ONLY the check_review_posted wiring in the 4 bundled recipe YAMLs consistently (same step shape, routing, and skip guards in each), update the corresponding contract expectations, re-run tests, then PR/review/merge. Keep the fix session's scope to the recipe wiring — everything else is done.

Post-fix comparative evidence (2026-07-08): defect 1 is backend-independent

Review-post counts across recent PRs: #4198 (interactive claude session review): 2 reviews + 5 inline comments posted. Pipeline-run review_pr children: #4201 (claude-code) 0/0, #4203 (codex) 0/0, #4205 (claude-code, this fix's own PR) 0/0. Headless pipeline review-pr children post nothing regardless of backend — the codex sandbox is only one trigger. The merged check_review_posted gate (Part B, PR #4205) will hard-fail this on the next pipeline run once the fixed version is live, and its degradation contract should surface the true posting failure for the claude-headless case. Left open (staged) for promotion; if the first post-fix pipeline run trips the new gate on claude-code workers, that occurrence should be investigated as its own root cause.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugExisting behavior is brokenrecipe:remediationRoute: investigate/decompose before implementationstagedImplementation staged and waiting for promotion to main

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions