From 1a108402b71414befb25ae281551ba54ba38ff7e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 13 Jul 2026 10:00:24 +0900 Subject: [PATCH] fix(governance): bound current-head branch update cadence --- .../workflows/pr-review-merge-scheduler.yml | 17 ++++++++++ scripts/ci/noema_review_gate.py | 19 +++++++++-- scripts/ci/pr_review_merge_scheduler.py | 22 +++++++++++++ scripts/ci/test_strix_quick_gate.sh | 4 +++ tests/test_noema_review_gate.py | 29 ++++++++++++++++ tests/test_opencode_agent_contract.py | 5 +++ tests/test_pr_review_merge_scheduler.py | 33 +++++++++++++++++-- 7 files changed, 125 insertions(+), 4 deletions(-) diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 779203829..0d92f0453 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -35,6 +35,11 @@ on: required: false default: "1" type: string + branch_update_limit: + description: Branch update budget per scheduler run (-1 updates every eligible outdated branch) + required: false + default: "1" + type: string enable_auto_merge: description: Enable auto-merge for current-head approved PRs required: false @@ -105,6 +110,10 @@ on: description: OpenCode/Strix review dispatch budget per scheduler run (-1 dispatches every eligible current-head review) required: false default: "1" + branch_update_limit: + description: Branch update budget per scheduler run (-1 updates every eligible outdated branch) + required: false + default: "1" enable_auto_merge: description: Enable auto-merge for current-head approved PRs required: false @@ -192,6 +201,7 @@ jobs: PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number || inputs.pr_number || '' }} TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_run' || github.event_name == 'push' || github.event_name == 'pull_request_target' || inputs.trigger_reviews == true }} REVIEW_DISPATCH_LIMIT_INPUT: ${{ inputs.review_dispatch_limit || vars.REVIEW_DISPATCH_LIMIT || '1' }} + BRANCH_UPDATE_LIMIT_INPUT: ${{ inputs.branch_update_limit || vars.BRANCH_UPDATE_LIMIT || '1' }} ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || inputs.enable_auto_merge == true }} MERGE_MODE: ${{ inputs.merge_mode || vars.PR_MERGE_MODE || 'direct_or_auto' }} UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'push' || github.event_name == 'pull_request_target' || github.event_name == 'workflow_run' || inputs.update_branches == true }} @@ -356,6 +366,10 @@ jobs: if [ -z "$review_dispatch_limit" ]; then review_dispatch_limit="-1" fi + branch_update_limit="$BRANCH_UPDATE_LIMIT_INPUT" + if [ -z "$branch_update_limit" ]; then + branch_update_limit="1" + fi args=( --repo "$GITHUB_REPOSITORY" --base-branch "$DEFAULT_BRANCH" @@ -363,6 +377,7 @@ jobs: --project-flow "$project_flow" --review-workflow "Required OpenCode Review" --review-dispatch-limit "$review_dispatch_limit" + --branch-update-limit "$branch_update_limit" --stale-opencode-minutes "$STALE_OPENCODE_MINUTES" ) if [ -n "$PULL_REQUEST_NUMBER" ]; then @@ -426,6 +441,7 @@ jobs: # GitHub queue ceiling while avoiding an arbitrary per-repository sample. ORG_SWEEP_MAX_PRS: ${{ inputs.max_prs || vars.ORG_SWEEP_MAX_PRS || '1000' }} ORG_SWEEP_REVIEW_DISPATCH_LIMIT: ${{ inputs.review_dispatch_limit || vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '1' }} + ORG_SWEEP_BRANCH_UPDATE_LIMIT: ${{ inputs.branch_update_limit || vars.ORG_SWEEP_BRANCH_UPDATE_LIMIT || '1' }} ORG_SWEEP_TRIGGER_REVIEWS: ${{ inputs.trigger_reviews == true }} ORG_SWEEP_ENABLE_AUTO_MERGE: ${{ inputs.enable_auto_merge == true }} ORG_SWEEP_MERGE_MODE: ${{ inputs.merge_mode || 'direct_or_auto' }} @@ -658,6 +674,7 @@ jobs: --max-prs "$ORG_SWEEP_MAX_PRS" --review-workflow "Required OpenCode Review" --review-dispatch-limit "$ORG_SWEEP_REVIEW_DISPATCH_LIMIT" + --branch-update-limit "$ORG_SWEEP_BRANCH_UPDATE_LIMIT" --stale-opencode-minutes "$STALE_OPENCODE_MINUTES" --merge-mode "$ORG_SWEEP_MERGE_MODE" ) diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 821050064..54f27e563 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -29,6 +29,7 @@ "Result: APPROVE", "opencode-review-control-v1", ) +REVIEW_BODY_HEAD_SHA_RE = re.compile(r"Head SHA:\s*`([0-9a-fA-F]{40})`") IGNORED_RUNNING_CHECKS = { "approve-after-primary-review", "noema-review", @@ -180,12 +181,26 @@ def review_commit(review: dict[str, Any]) -> str: return ((review.get("commit") or {}).get("oid") or "").strip() +def review_body_head_sha(review: dict[str, Any]) -> str | None: + """Return the last explicit current-head SHA recorded in a review body.""" + matches = REVIEW_BODY_HEAD_SHA_RE.findall(str(review.get("body") or "")) + return matches[-1] if matches else None + + +def review_matches_current_head(review: dict[str, Any], head_sha: str) -> bool: + """Return whether commit and explicit review-body evidence match the live head.""" + if not head_sha or review_commit(review) != head_sha: + return False + body_head = review_body_head_sha(review) + return body_head is None or body_head.lower() == head_sha.lower() + + def current_primary_approval(pr: dict[str, Any]) -> dict[str, Any] | None: """Return the current-head OpenCode approval when it matches the contract.""" head_sha = str(pr.get("headRefOid") or "") reviews = (((pr.get("reviews") or {}).get("nodes")) or []) for review in reversed(reviews): - if review_commit(review) != head_sha: + if not review_matches_current_head(review, head_sha): continue if str(review.get("state") or "").upper() != "APPROVED": continue @@ -201,7 +216,7 @@ def has_current_changes_requested(pr: dict[str, Any]) -> bool: head_sha = str(pr.get("headRefOid") or "") reviews = (((pr.get("reviews") or {}).get("nodes")) or []) for review in reversed(reviews): - if review_commit(review) == head_sha and str(review.get("state") or "").upper() == "CHANGES_REQUESTED": + if review_matches_current_head(review, head_sha) and str(review.get("state") or "").upper() == "CHANGES_REQUESTED": return True return False diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 01380f800..232d6412e 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1777,6 +1777,8 @@ def inspect_pr( dry_run: bool, trigger_reviews: bool, review_dispatch_allowed: bool = True, + branch_update_allowed: bool = True, + branch_update_limit: int = 1, enable_auto_merge_flag: bool, update_branches: bool, workflow: str, @@ -1851,6 +1853,12 @@ def decide(action: str, reason: str) -> Decision: def request_branch_update(freshness_reason: str, *, suffix: str = "") -> Decision: """Request update-branch and attach any same-head evidence follow-up.""" + if not branch_update_allowed: + return decide( + "wait", + f"branch update limit reached ({branch_update_limit} update/run); " + "defer outdated branch to the next scheduler run", + ) update_branch(repo, pr, dry_run=dry_run) followup_note = post_update_branch_followup( repo, @@ -3026,6 +3034,12 @@ def parse_args(argv: list[str]) -> argparse.Namespace: default=int(os.environ.get("REVIEW_DISPATCH_LIMIT", "1")), help="Maximum OpenCode/Strix review dispatch actions per scheduler run; -1 means unlimited", ) + parser.add_argument( + "--branch-update-limit", + type=int, + default=int(os.environ.get("BRANCH_UPDATE_LIMIT", "1")), + help="Maximum update-branch mutations per scheduler run; -1 means unlimited", + ) parser.add_argument("--enable-auto-merge", action=argparse.BooleanOptionalAction, default=True) parser.add_argument( "--merge-mode", @@ -3060,13 +3074,17 @@ def main(argv: list[str]) -> int: raise SystemExit("--pr-number must not be negative") if args.review_dispatch_limit < -1: raise SystemExit("--review-dispatch-limit must be -1 or greater") + if args.branch_update_limit < -1: + raise SystemExit("--branch-update-limit must be -1 or greater") prs = fetch_pr(args.repo, args.pr_number) if args.pr_number else fetch_open_prs(args.repo, args.max_prs) decisions = [] review_dispatches_used = 0 + branch_updates_used = 0 for pr in prs: review_dispatch_allowed = ( args.review_dispatch_limit < 0 or review_dispatches_used < args.review_dispatch_limit ) + branch_update_allowed = args.branch_update_limit < 0 or branch_updates_used < args.branch_update_limit try: decision = inspect_pr( args.repo, @@ -3074,6 +3092,8 @@ def main(argv: list[str]) -> int: dry_run=args.dry_run, trigger_reviews=args.trigger_reviews, review_dispatch_allowed=review_dispatch_allowed, + branch_update_allowed=branch_update_allowed, + branch_update_limit=args.branch_update_limit, enable_auto_merge_flag=args.enable_auto_merge, merge_mode=args.merge_mode, update_branches=args.update_branches, @@ -3091,6 +3111,8 @@ def main(argv: list[str]) -> int: decisions.append(decision) if decision.action in {"review_dispatch", "security_dispatch"}: review_dispatches_used += 1 + if decision.action == "update_branch": + branch_updates_used += 1 print_summary( decisions, dry_run=args.dry_run, diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 2e01e874e..5af38cc52 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -1241,6 +1241,10 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { assert_file_contains "$workflow_file" 'review_dispatch_limit="-1"' "scheduler dispatches every eligible same-head review or Strix evidence job immediately unless an explicit budget overrides it" assert_file_not_contains "$workflow_file" 'review_dispatch_limit="0"' "scheduler must not silently suppress eligible review dispatches on base-branch push events" assert_file_contains "$workflow_file" "--review-dispatch-limit" "scheduler passes the dispatch budget to the canonical script" + assert_file_contains "$workflow_file" "branch_update_limit:" "scheduler exposes a bounded branch-update budget" + assert_file_contains "$workflow_file" "BRANCH_UPDATE_LIMIT_INPUT" "scheduler forwards the branch-update budget to the canonical script" + assert_file_contains "$workflow_file" "ORG_SWEEP_BRANCH_UPDATE_LIMIT" "organization sweeps bound branch updates per repository" + assert_file_contains "$workflow_file" "--branch-update-limit" "scheduler passes the branch-update budget to the canonical script" assert_file_contains "$workflow_file" 'GH_TOKEN: ${{ github.token }}' "scheduler uses the caller workflow token so mutations are attributed to GitHub Actions in the target repository" assert_file_not_contains "$workflow_file" "INPUT_CANONICAL_REF" "scheduler trusted source checkout must not be controlled by workflow input" assert_file_not_contains "$workflow_file" "inputs.canonical_ref" "scheduler no longer accepts checkout-ref override input" diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index f09581c8b..890e4bd65 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -111,6 +111,35 @@ def test_review_state_helpers_cover_current_head_logic(): assert not noema.has_unresolved_threads(make_pr(reviewThreads={"nodes": [{"isResolved": False, "isOutdated": True}]})) +def test_review_state_helpers_reject_explicit_previous_head_evidence(): + current_head = "a" * 40 + previous_head = "b" * 40 + approval_marker = "Result: APPROVE" + stale_approval = review( + commit=current_head, + body=f"{approval_marker}\n\n- Head SHA: `{previous_head}`", + ) + exact_approval = review( + commit=current_head, + body=f"{approval_marker}\n\n- Head SHA: `{current_head}`", + ) + stale_change_request = review( + "CHANGES_REQUESTED", + commit=current_head, + body=f"Result: REQUEST_CHANGES\n\n- Head SHA: `{previous_head}`", + ) + + assert noema.current_primary_approval( + make_pr(headRefOid=current_head, reviews={"nodes": [stale_approval]}) + ) is None + assert noema.current_primary_approval( + make_pr(headRefOid=current_head, reviews={"nodes": [exact_approval]}) + ) == exact_approval + assert not noema.has_current_changes_requested( + make_pr(headRefOid=current_head, reviews={"nodes": [stale_change_request]}) + ) + + def test_check_helpers_and_existing_noema_review(): status_context = {"__typename": "StatusContext", "context": "ci", "state": "FAILURE"} check_run = { diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index dc08c881b..05667293d 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -859,6 +859,11 @@ def test_merge_scheduler_uses_escalating_mutation_credentials(): assert "SCHEDULER_MUTATION_TOKEN_SOURCE" in workflow assert 'default: "1"' in workflow assert 'review_dispatch_limit="-1"' in workflow + assert "branch_update_limit:" in workflow + assert "BRANCH_UPDATE_LIMIT_INPUT" in workflow + assert "ORG_SWEEP_BRANCH_UPDATE_LIMIT" in workflow + assert '--branch-update-limit "$branch_update_limit"' in workflow + assert '--branch-update-limit "$ORG_SWEEP_BRANCH_UPDATE_LIMIT"' in workflow def test_opencode_runs_merge_scheduler_after_review_without_repo_local_dispatch(): diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 3d80474d9..95c035ceb 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -3263,7 +3263,7 @@ def test_direct_merge_block_detail_keeps_generic_refusal_tail(): assert sched.direct_merge_block_detail(error) == "first diagnostic line last diagnostic line" -def test_main_limits_review_dispatches_without_blocking_branch_updates(monkeypatch, capsys): +def test_main_limits_review_dispatches_and_branch_updates(monkeypatch, capsys): prs = [ make_pr( number=1, @@ -3280,6 +3280,13 @@ def test_main_limits_review_dispatches_without_blocking_branch_updates(monkeypat compareBehindBy=2, autoMergeRequest={"enabledAt": "now"}, ), + make_pr( + number=4, + mergeStateStatus="BLOCKED", + restMergeableState="BLOCKED", + compareBehindBy=3, + autoMergeRequest={"enabledAt": "now"}, + ), ] dispatched = [] updated = [] @@ -3309,6 +3316,8 @@ def test_main_limits_review_dispatches_without_blocking_branch_updates(monkeypat "github-flow", "--review-dispatch-limit", "1", + "--branch-update-limit", + "1", ] ) == 0 @@ -3318,12 +3327,16 @@ def test_main_limits_review_dispatches_without_blocking_branch_updates(monkeypat payload = json.loads(output.strip().splitlines()[-1]) assert dispatched == [1] assert updated == [3] - assert payload["counts"] == {"review_dispatch": 1, "update_branch": 1, "wait": 1} + assert payload["counts"] == {"review_dispatch": 1, "update_branch": 1, "wait": 2} assert ( payload["decisions"][1]["reason"] == "current head has completed Strix evidence; review dispatch limit reached" ) assert payload["decisions"][2]["contract_decision"] == "UPDATE_BRANCH" + assert payload["decisions"][3]["contract_decision"] == "WAIT" + assert payload["decisions"][3]["reason"] == ( + "branch update limit reached (1 update/run); defer outdated branch to the next scheduler run" + ) def test_main_rejects_invalid_review_dispatch_limit(): @@ -3342,6 +3355,22 @@ def test_main_rejects_invalid_review_dispatch_limit(): ) +def test_main_rejects_invalid_branch_update_limit(): + with pytest.raises(SystemExit, match="--branch-update-limit must be -1 or greater"): + sched.main( + [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github-flow", + "--branch-update-limit", + "-2", + ] + ) + + def test_print_summary_self_test_parse_args_and_main(monkeypatch, capsys): sched.print_summary( [sched.Decision(1, "wait", "ready"), sched.Decision(2, "wait", "queued")],