Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
11 changes: 11 additions & 0 deletions .github/workflows/pr-review-merge-scheduler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,17 @@ jobs:
(
github.event_name != 'repository_dispatch' ||
github.event.client_payload.org_sweep != true
) &&
(
github.event_name != 'pull_request_review' ||
github.event.action == 'dismissed' ||
(
github.event.action == 'submitted' &&
(
github.event.review.state == 'approved' ||
github.event.review.state == 'changes_requested'
)
)
Comment thread
seonghobae marked this conversation as resolved.
)
runs-on: ubuntu-24.04
# Bound scan-pr-queue to a wall-clock ceiling well short of GitHub's
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@
- Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up.

## [Unreleased]
- Stop `pull_request_review: submitted` events with state `commented` at the
merge scheduler's job-admission boundary, before a hosted runner is
requested. `approved`, `changes_requested`, and `dismissed` review
transitions retain their existing exact-PR scheduler path and permissions.
- **Fix current-main contract drift that blocked the unscoped
`agent-review-runtime-quality-ci.yml` "Verify scheduler and
contextual-orchestrator review-repair contracts" step (which discovers and
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
# Doctoring record: pr-review-merge-scheduler.yml's "fires at every step" pattern is by-design, not a bug (2026-09-03)

> **2026-09-05 correction.** The broad claim below that every submitted review
> is an actionable approval-state change was incomplete. GitHub emits
> `pull_request_review: submitted` for `COMMENTED` reviews, which do not create
> an `APPROVED` or `CHANGES_REQUESTED` state. On PR #1885, CodeRabbit submitted
> `COMMENTED` reviews at 03:08:52Z, 04:27:31Z, and 05:30:37Z; the central
> scheduler admitted runner-backed runs 33941045179, 33944606701, and
> 33947394894 within seconds. The scheduler still needs the review trigger for
> `APPROVED`, `CHANGES_REQUESTED`, and `dismissed`, but `COMMENTED` is now
> rejected by the `scan-pr-queue` job-level `if` before runner acquisition.
> The executable truth-table contract is
> `tests/test_merge_scheduler_review_event_admission.py`. This correction does
> not reinterpret a bot comment as formal review evidence and does not alter
> exact-PR concurrency, review semantics, or scheduler permissions.

- **Date:** 2026-09-03
- **Subject:** the user directly observed the scheduler workflow firing repeatedly ("왜 각 모든 단계마다 Trigger
되고 있죠?") after live evidence surfaced today of severe org-wide Actions thrashing (near-zero completion
Expand Down
100 changes: 100 additions & 0 deletions tests/test_merge_scheduler_review_event_admission.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
"""Executable admission contract for merge-scheduler review events."""

from __future__ import annotations

import ast
import re
from pathlib import Path

import pytest


ROOT = Path(__file__).resolve().parents[1]
WORKFLOW = ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml"


def scan_job_condition() -> str:
"""Return the normalized pre-runner condition for ``scan-pr-queue``."""
workflow = WORKFLOW.read_text(encoding="utf-8")
scan_job = workflow.split("\n scan-pr-queue:\n", 1)[1]
condition = scan_job.split("\n runs-on:", 1)[0].split("\n if: >-\n", 1)[1]
return " ".join(line.strip() for line in condition.splitlines())


def admits_review_event(*, action: str, state: str) -> bool:
"""Evaluate the workflow's review-event condition for one trusted fixture."""
expression = scan_job_condition().replace("&&", " and ").replace("||", " or ")
expression = re.sub(r"\btrue\b", "True", expression)
values = {
"github.event_name": "pull_request_review",
"github.event.action": action,
"github.event.review.state": state,
"github.event.client_payload.org_sweep": False,
}

def evaluate(node: ast.AST) -> object:
"""Interpret only the boolean/comparison subset used by the job guard."""
if isinstance(node, ast.Expression):
return evaluate(node.body)
if isinstance(node, ast.BoolOp):
operands = [bool(evaluate(value)) for value in node.values]
return all(operands) if isinstance(node.op, ast.And) else any(operands)
if isinstance(node, ast.Compare) and len(node.ops) == len(node.comparators) == 1:
left = evaluate(node.left)
right = evaluate(node.comparators[0])
if isinstance(node.ops[0], ast.Eq):
return left == right
if isinstance(node.ops[0], ast.NotEq):
return left != right
if isinstance(node, ast.Constant):
return node.value
if isinstance(node, (ast.Attribute, ast.Name)):
key = ast.unparse(node)
if key in values:
return values[key]
raise AssertionError(f"unsupported scheduler expression node: {ast.dump(node)}")

return bool(evaluate(ast.parse(expression, mode="eval")))


@pytest.mark.parametrize(
("action", "state", "expected"),
[
("submitted", "commented", False),
("submitted", "approved", True),
("submitted", "changes_requested", True),
("dismissed", "commented", True),
],
)
def test_review_event_admission_truth_table(
action: str, state: str, expected: bool
) -> None:
"""Admit only review transitions that can change merge eligibility."""
assert admits_review_event(action=action, state=state) is expected


def test_review_filter_preserves_exact_pr_group_and_least_privilege() -> None:
"""Filtering COMMENTED reviews must not weaken scheduler trust boundaries."""
workflow = WORKFLOW.read_text(encoding="utf-8")
assert (
"github.event_name == 'pull_request_review' && "
"format('pr-{0}', github.event.pull_request.number)" in workflow
)
assert (
"cancel-in-progress: ${{ github.event_name == 'pull_request_target' || "
"github.event_name == 'pull_request_review' || "
"github.event_name == 'repository_dispatch' }}" in workflow
)
assert "permissions:\n contents: read" in workflow

scan_header = workflow.split("\n scan-pr-queue:\n", 1)[1].split(
"\n env:\n", 1
)[0]
for permission in (
"actions: write",
"checks: read",
"contents: write",
"id-token: write",
"pull-requests: write",
):
assert permission in scan_header
Loading