Problem
PR #334 ran 9 full review cycles in ~90 minutes despite every merge-decision verdict being GO-CLEAN. The MAX_REVIEW_CYCLES=2 guard never engaged.
Root cause
The cycle cap in .github/workflows/codex-code-review.yml (around line 2406) only governs the dispatch-triggered re-review path used by GO-WITH-RESERVATIONS. GO-CLEAN is assumed terminal.
But the merge-decision agent applies "small mechanical fixes" and pushes them to the PR branch before declaring GO-CLEAN (see GO-CLEAN-with-fixes branch around line 2364). Each push fires a normal pull_request: synchronize event, which re-runs the entire review workflow as a brand-new invocation. The new run has no memory of the prior cycle count — the cap never engages.
Reviewers then find a different tiny nit in the just-pushed doc edit, the merge-decision agent fixes it, pushes again, GO-CLEAN, and the loop repeats indefinitely.
Evidence from PR #334
9 merge-decision comments, all GO-CLEAN, all citing rewordings of docs/agentic-pipeline-learnings.md (sections 1.1, 1.5, 1.9, 2.1, 2.4, 2.5, 2.8, 2.9, 3.5) plus AGENTS.md and docs/openclaw-integration.md. Classic reviewer churn the cap was supposed to prevent.
Proposed fix: close the synchronize back-door
Make the cycle cap survive across workflow runs by counting cycles on the PR itself, not per run.
Option A — author-based skip (simplest): In the early get-context job, if the workflow was triggered by synchronize and the head commit's author/committer is the merge-decision bot (or the commit message contains a sentinel like [merge-decision-autofix] that the agent is instructed to include), short-circuit the entire pipeline and publish success statuses for the new HEAD. The push by the merge-decision agent is by definition a fix it already approved as GO-CLEAN.
Option B — PR-scoped cycle counter: Use a PR label (e.g. review-cycle-N) incremented at the start of each review run. If N > MAX_REVIEW_CYCLES, skip and post an explanatory comment. This survives across both dispatch and synchronize triggers.
Option A is narrower and lower-risk; Option B is more general. They are not mutually exclusive.
Acceptance
- A PR where merge-decision pushes a GO-CLEAN autofix does NOT trigger another full review cycle.
- The cap applies regardless of which event (
workflow_dispatch or synchronize) initiated the run.
- Manual
/review comments still force a fresh review (existing behavior at line 385).
Related
Problem
PR #334 ran 9 full review cycles in ~90 minutes despite every merge-decision verdict being GO-CLEAN. The
MAX_REVIEW_CYCLES=2guard never engaged.Root cause
The cycle cap in
.github/workflows/codex-code-review.yml(around line 2406) only governs the dispatch-triggered re-review path used byGO-WITH-RESERVATIONS.GO-CLEANis assumed terminal.But the merge-decision agent applies "small mechanical fixes" and pushes them to the PR branch before declaring GO-CLEAN (see GO-CLEAN-with-fixes branch around line 2364). Each push fires a normal
pull_request: synchronizeevent, which re-runs the entire review workflow as a brand-new invocation. The new run has no memory of the prior cycle count — the cap never engages.Reviewers then find a different tiny nit in the just-pushed doc edit, the merge-decision agent fixes it, pushes again, GO-CLEAN, and the loop repeats indefinitely.
Evidence from PR #334
9 merge-decision comments, all GO-CLEAN, all citing rewordings of
docs/agentic-pipeline-learnings.md(sections 1.1, 1.5, 1.9, 2.1, 2.4, 2.5, 2.8, 2.9, 3.5) plusAGENTS.mdanddocs/openclaw-integration.md. Classic reviewer churn the cap was supposed to prevent.Proposed fix: close the synchronize back-door
Make the cycle cap survive across workflow runs by counting cycles on the PR itself, not per run.
Option A — author-based skip (simplest): In the early
get-contextjob, if the workflow was triggered bysynchronizeand the head commit's author/committer is the merge-decision bot (or the commit message contains a sentinel like[merge-decision-autofix]that the agent is instructed to include), short-circuit the entire pipeline and publish success statuses for the new HEAD. The push by the merge-decision agent is by definition a fix it already approved as GO-CLEAN.Option B — PR-scoped cycle counter: Use a PR label (e.g.
review-cycle-N) incremented at the start of each review run. IfN > MAX_REVIEW_CYCLES, skip and post an explanatory comment. This survives across both dispatch and synchronize triggers.Option A is narrower and lower-risk; Option B is more general. They are not mutually exclusive.
Acceptance
workflow_dispatchorsynchronize) initiated the run./reviewcomments still force a fresh review (existing behavior at line 385).Related