From a687de7b37c0f9f1683a6fa83468e56e1a3774f7 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Fri, 4 Sep 2026 14:09:10 +0900 Subject: [PATCH] ci(scheduler): reduce organization sweep polling Signed-off-by: Seongho Bae --- .../workflows/pr-review-merge-scheduler.yml | 60 ++++++++----------- ...ions_queue_saturation_scheduler_cadence.py | 42 +++++++++---- .../test_required_workflow_queue_contract.py | 26 ++++---- 3 files changed, 67 insertions(+), 61 deletions(-) diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 238b0a156e..9cc9033a91 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -8,7 +8,7 @@ on: pull_request_review: types: [submitted, dismissed] workflow_run: - workflows: ["Required OpenCode Review", "Strix Security Scan"] + workflows: ["Required OpenCode Review", "Strix Security Scan", "Security Scan", "SAST Semgrep"] types: [completed] workflow_call: inputs: @@ -73,32 +73,19 @@ on: default: "" type: string schedule: - # scan-pr-queue's own repository-local heartbeat. org-queue-sweep below + # Daily missed-event recovery for this repository. org-queue-sweep below # explicitly excludes ContextualWisdomLab/.github from its target list # (a PR in THIS repository, including one editing the governance # workflows themselves, is never covered by the org-wide sweep), so this - # is the only periodic fallback for this repository's own PR queue. It - # also plugs a real event-coverage gap shared by every repository: - # required checks such as Security Scan and SAST Semgrep have no - # workflow_run listener anywhere in this file, so a PR where either is - # the last required check to go green has no event-driven re-wake at - # all. Offset by 30 minutes from org-queue-sweep's "0 * * * *" tick so - # the two heartbeats do not collide. Lengthened from */30 to hourly for - # the same Actions-capacity reason, and by the same lever, as the - # org-queue-sweep hourly cadence below (see - # docs/doctoring/actions-queue-saturation-hourly-sweep.md, #1630) — do - # NOT remove it outright; that would leave this repository's own queue - # with zero fallback heartbeat. - - cron: "30 * * * *" - # Hourly org-wide sweep cadence for the org-queue-sweep job below. Target - # repositories only receive scheduler runs on PR events, review/security - # workflow completion, and protected-branch pushes; a PR whose approval or - # required checks land AFTER its last event has no later trigger and sits - # approved-but-unmerged until a human pushes something. The sweep closes - # that gap on a fixed heartbeat. Runs hourly so an approval or - # required check that lands after a PR's last event is auto-updated/merged - # within about an hour without adding quarter-hourly runner pressure. - - cron: "0 * * * *" + # remains the bounded fallback for a genuinely missed native event. Security + # Scan and SAST Semgrep completions now wake the scheduler directly above. + # Offset from the organization sweep so recovery jobs do not collide. + - cron: "47 3 * * *" + # Daily organization-wide missed-event recovery. Native PR, review, + # protected-branch push, OpenCode, Strix, Security Scan, and SAST Semgrep + # events handle the normal path; this lower-frequency sweep only recovers + # delivery gaps and stacked PRs that do not receive required workflows. + - cron: "17 3 * * *" repository_dispatch: types: [merge-scheduler] @@ -146,7 +133,7 @@ jobs: ) && ( github.event_name != 'schedule' || - github.event.schedule != '0 * * * *' + github.event.schedule != '17 3 * * *' ) && ( github.event_name != 'repository_dispatch' || @@ -589,13 +576,13 @@ jobs: python3 scripts/ci/pr_review_merge_scheduler.py "${args[@]}" org-queue-sweep: - # Organization-wide approved-PR fallback sweep. Event-driven scheduler runs + # Organization-wide approved-PR recovery sweep. Event-driven scheduler runs # in target repositories stop retrying once their triggering event is # consumed, so a PR that becomes mergeable AFTER its last event (approval # published after the scheduler pass, required merge-preview checks landing # late, a base-branch policy blocker clearing) stays approved-but-unmerged # with no later trigger. This job re-runs the same trusted scheduler against - # every organization repository on an hourly heartbeat so each such PR is + # repositories that can contain open PRs on a daily heartbeat so each such PR is # merged, branch-updated, or leaves a concrete per-PR blocker reason in this # log. It never bypasses policy: all mutations go through the same guarded # scheduler contract as the per-repository runs. Stacked PRs have no @@ -604,12 +591,12 @@ jobs: if: >- github.repository == 'ContextualWisdomLab/.github' && ( - (github.event_name == 'schedule' && github.event.schedule == '0 * * * *') || + (github.event_name == 'schedule' && github.event.schedule == '17 3 * * *') || (github.event_name == 'repository_dispatch' && github.event.client_payload.org_sweep == true) ) runs-on: ubuntu-24.04 # The complete organization walk exceeded the legacy 30-minute boundary in - # production. Keep one running and one latest pending hourly sweep through the + # production. Keep one running and one latest pending daily sweep through the # schedule-specific concurrency key above, while allowing the current walk # enough time to finish instead of cancelling before later repositories. timeout-minutes: 60 @@ -630,7 +617,9 @@ jobs: # this contract). The scheduler paginates, so 1000 keeps the practical # GitHub queue ceiling while avoiding an arbitrary per-repository sample. ORG_SWEEP_MAX_PRS: ${{ github.event.client_payload.max_prs || inputs.max_prs || vars.ORG_SWEEP_MAX_PRS || '1000' }} - ORG_SWEEP_REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '1' }} + # #1823 moved ordinary PR OpenCode dispatch into the required workflow. + # Keep only the separate stacked-PR fallback budget enabled by default. + ORG_SWEEP_REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '0' }} ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.stacked_review_dispatch_limit || vars.ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT || '1' }} ORG_SWEEP_BRANCH_UPDATE_LIMIT: ${{ github.event.client_payload.branch_update_limit || inputs.branch_update_limit || vars.ORG_SWEEP_BRANCH_UPDATE_LIMIT || '1' }} ORG_SWEEP_TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.trigger_reviews != false || inputs.trigger_reviews == true }} @@ -653,7 +642,7 @@ jobs: # ticks" guarantee a rotation is meant to provide. Wall-clock time alone # is also insufficient, since this single-flight/non-cancelling job can # run up to 60 minutes and a delayed real execution can let more than - # one hourly window elapse, occasionally repeating a modulo offset + # one daily window elapse, occasionally repeating a modulo offset # (ContextualWisdomLab/.github#1223 review finding). # A repository the sweep credential structurally cannot read (the OpenCode # app is not installed there / the PR_REVIEW_MERGE_TOKEN lacks it) returns @@ -867,10 +856,10 @@ jobs: # source: a persistent `ORG_SWEEP_ROTATION_COUNTER` repository # variable on this (.github) repository, incremented by exactly # one at the start of every actual org-queue-sweep execution. A - # wall-clock tick (one per hour) is *not* sufficient on its own: + # wall-clock tick (one per day) is *not* sufficient on its own: # this job is single-flight/non-cancelling with up to a 60-minute # timeout, so a delayed or backlogged execution can let more than - # one hourly window elapse between two real sweep runs, and if that + # one daily window elapse between two real sweep runs, and if that # gap happens to be an exact multiple of the repository count the # modulo offset repeats -- reintroducing the exact starvation # #1220 fixed (CodeRabbit review finding on #1223). A persistent @@ -934,7 +923,7 @@ jobs: ORG_SWEEP_ROTATION_INDEX="$counter_next" else echo "::warning::read ${counter_variable_name}=${counter_current} but could not PATCH it; falling back to a wall-clock rotation tick for this run only" - ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 3600 )) + ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 86400 )) fi elif gh api "repos/${GITHUB_REPOSITORY}/actions/variables" \ -X POST -f "name=${counter_variable_name}" -f "value=1" >/dev/null 2>&1; then @@ -947,7 +936,7 @@ jobs: ORG_SWEEP_ROTATION_INDEX=1 else echo "::warning::could not read/write ${counter_variable_name}; falling back to a wall-clock rotation tick for this run only" - ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 3600 )) + ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 86400 )) fi fi if ! [[ "$ORG_SWEEP_ROTATION_INDEX" =~ ^[0-9]+$ ]]; then @@ -964,6 +953,7 @@ jobs: jq -r ' .[] | select(.archived == false and .disabled == false) + | select((.open_issues_count // 1) > 0) | select(.full_name != "ContextualWisdomLab/.github") | "\(.full_name)\t\(.default_branch)" ' <<<"$repositories_json" diff --git a/tests/test_actions_queue_saturation_scheduler_cadence.py b/tests/test_actions_queue_saturation_scheduler_cadence.py index fa5ce36370..836ddc8ed7 100644 --- a/tests/test_actions_queue_saturation_scheduler_cadence.py +++ b/tests/test_actions_queue_saturation_scheduler_cadence.py @@ -7,17 +7,19 @@ WORKFLOW = ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" -def test_org_queue_sweep_is_hourly_not_quarter_hourly() -> None: - """The expensive org sweep must not self-amplify a saturated Actions queue.""" +def test_org_queue_sweep_is_daily_recovery_not_hourly_polling() -> None: + """Native events own normal progress; the expensive sweep only recovers gaps.""" workflow = WORKFLOW.read_text(encoding="utf-8") - assert '- cron: "0 * * * *"' in workflow or "- cron: '0 * * * *'" in workflow + assert '- cron: "17 3 * * *"' in workflow + assert '- cron: "0 * * * *"' not in workflow assert '*/15 * * * *' not in workflow -def test_org_queue_sweep_wall_clock_fallback_matches_hourly_cadence() -> None: - """Fallback rotation and its maintenance comments must match hourly cadence.""" +def test_org_queue_sweep_wall_clock_fallback_matches_daily_cadence() -> None: + """Fallback rotation and its maintenance comments must match daily cadence.""" workflow = WORKFLOW.read_text(encoding="utf-8") - assert workflow.count("$(date -u +%s) / 3600") == 2 + assert workflow.count("$(date -u +%s) / 86400") == 2 + assert "$(date -u +%s) / 3600" not in workflow assert "$(date -u +%s) / 900" not in workflow assert "900s window" not in workflow assert "900s)" not in workflow @@ -33,21 +35,35 @@ def test_repository_scheduler_keeps_event_driven_wakes() -> None: assert "repository_dispatch:" in workflow -def test_scan_pr_queue_heartbeat_is_hourly_and_offset_not_removed() -> None: +def test_scan_pr_queue_keeps_offset_daily_missed_event_recovery() -> None: """scan-pr-queue's own repository-local heartbeat must not be dropped. org-queue-sweep excludes ContextualWisdomLab/.github from its target list by name, so scan-pr-queue's own cron is the sole periodic fallback - for this repository's PR queue (and for any required check, such as - Security Scan or SAST Semgrep, with no workflow_run listener anywhere in - this file). It must be lengthened to hourly for the same capacity reason - as org-queue-sweep, not deleted, and offset from org-queue-sweep's - "0 * * * *" tick so the two heartbeats do not collide. + for this repository's PR queue after a genuinely missed native event. + Keep one low-frequency fallback offset from the organization recovery. """ workflow = WORKFLOW.read_text(encoding="utf-8") - assert '- cron: "30 * * * *"' in workflow + assert '- cron: "47 3 * * *"' in workflow + assert '- cron: "30 * * * *"' not in workflow assert '*/30 * * * *' not in workflow schedule_block = workflow.split(" schedule:", 1)[1].split( " repository_dispatch:", 1 )[0] assert schedule_block.count('- cron:') == 2 + + +def test_required_check_completions_wake_the_scheduler_natively() -> None: + """The last security gate should not wait for the daily recovery sweep.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + workflow_run = workflow.split(" workflow_run:\n", 1)[1].split( + " workflow_call:\n", 1 + )[0] + + for name in ( + "Required OpenCode Review", + "Strix Security Scan", + "Security Scan", + "SAST Semgrep", + ): + assert f'"{name}"' in workflow_run diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index f139c58419..a93c3e4855 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1083,7 +1083,7 @@ def test_scan_pr_queue_has_a_bounded_runtime() -> None: assert scan_timeout < 60 -def test_org_queue_sweep_covers_target_repositories_on_a_heartbeat() -> None: +def test_org_queue_sweep_covers_target_repositories_as_daily_recovery() -> None: """Guard the org-wide approved-PR fallback sweep contract. Target repositories only receive scheduler runs on PR events, so a PR that @@ -1091,18 +1091,17 @@ def test_org_queue_sweep_covers_target_repositories_on_a_heartbeat() -> None: The sweep job must exist, run only from the central repository on its own cron, use a cross-repository mutation credential (never the repository github.token silently), skip the central repository itself, and fail with a - visible reason when it cannot mutate sibling repositories. The sweep runs - hourly so an approval that lands after a PR's last event is - auto-updated/merged promptly instead of idling indefinitely. Its cron has a + visible reason when it cannot mutate sibling repositories. Native events + handle the normal path; the daily sweep recovers missed events. Its cron has a distinct concurrency key from the separate scan-pr-queue heartbeat, and the job has enough runtime headroom to finish a complete organization walk. """ workflow = workflow_text("pr-review-merge-scheduler.yml") assert "org-queue-sweep:" in workflow - assert '- cron: "0 * * * *"' in workflow + assert '- cron: "17 3 * * *"' in workflow assert "github.repository == 'ContextualWisdomLab/.github'" in workflow - assert "github.event.schedule == '0 * * * *'" in workflow + assert "github.event.schedule == '17 3 * * *'" in workflow assert "github.event.client_payload.org_sweep == true" in workflow assert ( "github.event_name == 'schedule' && format('schedule-{0}', " @@ -1119,7 +1118,7 @@ def test_org_queue_sweep_covers_target_repositories_on_a_heartbeat() -> None: ): assert f"{setting}: ${{{{ github.event_name == 'schedule' ||" in workflow # The single-repository scan must not double-run on the sweep cron. - assert "github.event.schedule != '0 * * * *'" in workflow + assert "github.event.schedule != '17 3 * * *'" in workflow assert "github.event.client_payload.org_sweep != true" in workflow # The sweep must never silently no-op with the repository-scoped token. assert ( @@ -1128,6 +1127,7 @@ def test_org_queue_sweep_covers_target_repositories_on_a_heartbeat() -> None: ) assert 'select(.full_name != "ContextualWisdomLab/.github")' in workflow assert "select(.archived == false and .disabled == false)" in workflow + assert "select((.open_issues_count // 1) > 0)" in workflow # The sweep must not silently truncate large/old queues or skip a repository # whose only open work is a stacked/non-default-base PR. assert "vars.ORG_SWEEP_MAX_PRS || '1000'" in workflow @@ -1420,7 +1420,7 @@ def test_org_queue_sweep_rotation_index_falls_back_to_wall_clock(tmp_path: Path) assert result.returncode == 0, result.stderr stdout_lines = result.stdout.strip().splitlines() computed_tick = int(stdout_lines[-1]) # last line: the printed value; earlier: the warning - expected_tick = int(time.time()) // 3600 + expected_tick = int(time.time()) // 86400 assert abs(computed_tick - expected_tick) <= 1 # tolerate a tick boundary race assert "could not read/write" in result.stdout # a `::warning::` workflow command @@ -1446,7 +1446,7 @@ def test_org_queue_sweep_rotation_index_transient_read_failure_does_not_reset_co assert result.returncode == 0, result.stderr stdout_lines = result.stdout.strip().splitlines() computed_tick = int(stdout_lines[-1]) - expected_tick = int(time.time()) // 3600 + expected_tick = int(time.time()) // 86400 assert abs(computed_tick - expected_tick) <= 1 # Critically: never "1" -- that would mean the failed read was treated # as a fresh-start reset rather than an unreadable existing value. @@ -1469,7 +1469,7 @@ def test_org_queue_sweep_rotation_index_successful_read_but_failed_patch_falls_b assert result.returncode == 0, result.stderr stdout_lines = result.stdout.strip().splitlines() computed_tick = int(stdout_lines[-1]) - expected_tick = int(time.time()) // 3600 + expected_tick = int(time.time()) // 86400 assert abs(computed_tick - expected_tick) <= 1 assert "read ORG_SWEEP_ROTATION_COUNTER=41 but could not PATCH it" in result.stdout @@ -1514,7 +1514,7 @@ def test_org_queue_sweep_documents_rotation_leverage_and_validates_input() -> No assert "ContextualWisdomLab/.github#1219" in workflow assert ( - 'ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 3600 ))' + 'ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 86400 ))' ) in workflow assert ( 'if ! [[ "$ORG_SWEEP_ROTATION_INDEX" =~ ^[0-9]+$ ]]; then' @@ -1529,7 +1529,7 @@ def test_org_queue_sweep_documents_rotation_leverage_and_validates_input() -> No assert "ORG_SWEEP_ROTATION_INDEX: ${{ github.run_number }}" not in workflow # Keep ordinary and stacked review budgets independently configurable so # ordinary work cannot starve the only review path for stacked PRs. - assert "vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '1'" in workflow + assert "vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '0'" in workflow assert "vars.ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT || '1'" in workflow assert "Stacked PRs have no" in workflow @@ -1540,7 +1540,7 @@ def test_org_queue_sweep_manual_cadence_inputs_reach_the_sweep_job() -> None: assert ( "ORG_SWEEP_REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || " - "vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '1' }}" + "vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '0' }}" ) in workflow assert ( "ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.stacked_review_dispatch_limit || "