diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index b3deb32eef..a15cdf36e1 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -74,15 +74,15 @@ on: type: string schedule: - cron: "*/30 * * * *" - # Every-15-minutes org-wide sweep cadence for the org-queue-sweep job below. Target + # 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 every 15 minutes so an approval or + # 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 ~15 minutes instead of sitting idle for up to an hour. - - cron: "*/15 * * * *" + # within about an hour without adding quarter-hourly runner pressure. + - cron: "0 * * * *" repository_dispatch: types: [merge-scheduler] @@ -136,7 +136,7 @@ jobs: ) && ( github.event_name != 'schedule' || - github.event.schedule != '*/15 * * * *' + github.event.schedule != '0 * * * *' ) && ( github.event_name != 'repository_dispatch' || @@ -587,12 +587,12 @@ jobs: if: >- github.repository == 'ContextualWisdomLab/.github' && ( - (github.event_name == 'schedule' && github.event.schedule == '*/15 * * * *') || + (github.event_name == 'schedule' && github.event.schedule == '0 * * * *') || (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 */15 sweep through the + # production. Keep one running and one latest pending hourly 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 @@ -636,7 +636,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 900s window elapse, occasionally repeating a modulo offset + # one hourly 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 @@ -850,10 +850,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 900s) is *not* sufficient on its own: + # wall-clock tick (one per hour) 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 900s window elapse between two real sweep runs, and if that + # one hourly 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 @@ -917,7 +917,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) / 900 )) + ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 3600 )) fi elif gh api "repos/${GITHUB_REPOSITORY}/actions/variables" \ -X POST -f "name=${counter_variable_name}" -f "value=1" >/dev/null 2>&1; then @@ -930,7 +930,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) / 900 )) + ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 3600 )) fi fi if ! [[ "$ORG_SWEEP_ROTATION_INDEX" =~ ^[0-9]+$ ]]; then diff --git a/docs/doctoring/actions-queue-saturation-hourly-sweep.md b/docs/doctoring/actions-queue-saturation-hourly-sweep.md new file mode 100644 index 0000000000..a0d3122290 --- /dev/null +++ b/docs/doctoring/actions-queue-saturation-hourly-sweep.md @@ -0,0 +1,36 @@ +# Actions queue saturation: hourly organization sweep + +**Status:** active repair evidence +**Owning repository:** `ContextualWisdomLab/.github` +**Canonical repair PR:** `#1630` +**Protected baseline:** `main@4ae90e18b03a3a455e13e501628010cabc5c37a8` + +## Root cause + +The central PR review/merge scheduler has two periodic entry points in addition to event-driven wakes. The repository-local queue scan runs every 30 minutes, while the expensive `org-queue-sweep` has been admitted every 15 minutes. Under the observed organization-wide hosted-runner saturation, the full organization walk can remain queued or run long enough that quarter-hourly admission adds more pending work before prior evidence drains. That is a control-plane pressure amplifier: required current-head evidence for leaf repositories queues behind recurring control-plane work that exists to unblock those same repositories. + +The repair is deliberately bounded. Keep the 30-minute repository scan and all event-driven `pull_request_target`, `pull_request_review`, `workflow_run`, and `repository_dispatch` wakes. Change only the organization sweep heartbeat to hourly (`0 * * * *`). The wall-clock fallback used by the persisted sweep rotation counter must advance on the same hourly cadence (`epoch_seconds / 3600`) rather than the old 15-minute cadence (`epoch_seconds / 900`), otherwise a fallback run would skip four repository offsets for each real scheduled sweep. + +## TDD and executable contract + +`tests/test_actions_queue_saturation_scheduler_cadence.py` is the RED-first contract. It requires the live workflow to contain the hourly cron, rejects the quarter-hourly cron, preserves event-driven wakes, and binds both wall-clock fallback expressions to hourly rotation. The older assertions in `tests/test_required_workflow_queue_contract.py` must be updated with the production workflow rather than retained as a stale policy test. + +The production change must also update `docs/org-required-workflow-rollout.md` so operator guidance states that the heartbeat can be up to one hour old. Historical doctoring that describes the old quarter-hour schedule remains historical evidence and must not be rewritten as though it never existed. + +## Safety boundary + +This repair does not mark queued checks successful, cancel the sole current-head evidence, weaken required workflows, relax approval requirements, or synthesize review state. Queue hygiene remains fail-closed. Cross-repository mutation credentials, exact-head validation, stale-head cancellation rules, unavailable-repository thresholds, scheduler concurrency groups, and merge guards remain unchanged. + +No organization-owned identifier introduced by this repair uses an ambiguous single-word domain name. GitHub event fields and cron syntax are externally mandated contract terms and remain unchanged except for the cadence value. + +## Verification + +After the production commit lands on the canonical branch: + +1. run the focused cadence and required-workflow queue contract tests; +2. verify the scheduler workflow contains exactly the intended 30-minute repository scan and hourly organization sweep; +3. confirm event-driven wakes remain present; +4. inspect fresh exact-head required checks and review evidence; +5. observe queue depth after the change rather than treating the configuration diff itself as proof that saturation has cleared. + +Merge remains subject to ordinary protected-branch requirements and exact-current-head evidence. diff --git a/docs/org-required-workflow-rollout.md b/docs/org-required-workflow-rollout.md index 36edcd29dd..7c55c6fbab 100644 --- a/docs/org-required-workflow-rollout.md +++ b/docs/org-required-workflow-rollout.md @@ -156,9 +156,9 @@ The central `.github/workflows/pr-review-merge-scheduler.yml` is now part of the Do not centralize the scheduler by running a `.github` scheduled job against other repositories with the `.github` repository token. That would either fail permission checks or use the wrong mutation actor. The central path is a required workflow executed in each target repository context. -- Heartbeat fallback posture: event-driven target-repository runs stop retrying once their triggering event is consumed, so a PR that becomes mergeable AFTER its last event (approval published after the scheduler pass, merge-preview checks landing late, a temporary base-branch policy blocker clearing) has no later trigger and sits approved-but-unmerged. The `org-queue-sweep` job in the central scheduler workflow closes this gap: it runs every 15 minutes (`*/15 * * * *`) only in `ContextualWisdomLab/.github`, re-runs the same trusted scheduler script against every non-archived organization repository, and merges/updates through the identical guarded contract. Stacked PRs, which do not receive injected required workflows, use a separate bounded OpenCode dispatch budget so ordinary default-branch traffic cannot leave them at `OpenCode review absent`. It never uses the `.github` repository `github.token` for sibling mutations — it requires `PR_REVIEW_MERGE_TOKEN`, `OPENCODE_APPROVE_TOKEN`, or the exchanged OpenCode app token, and fails with a visible `::error` reason when no cross-repository mutation credential is available instead of silently no-opping. Every swept repository prints its per-PR decision log, so an unmerged PR always has a concrete logged reason at most 15 minutes old. +- Heartbeat fallback posture: event-driven target-repository runs stop retrying once their triggering event is consumed, so a PR that becomes mergeable AFTER its last event (approval published after the scheduler pass, merge-preview checks landing late, a temporary base-branch policy blocker clearing) has no later trigger and sits approved-but-unmerged. The `org-queue-sweep` job in the central scheduler workflow closes this gap: it runs hourly (`0 * * * *`) only in `ContextualWisdomLab/.github`, re-runs the same trusted scheduler script against every non-archived organization repository, and merges/updates through the identical guarded contract. Stacked PRs, which do not receive injected required workflows, use a separate bounded OpenCode dispatch budget so ordinary default-branch traffic cannot leave them at `OpenCode review absent`. It never uses the `.github` repository `github.token` for sibling mutations — it requires `PR_REVIEW_MERGE_TOKEN`, `OPENCODE_APPROVE_TOKEN`, or the exchanged OpenCode app token, and fails with a visible `::error` reason when no cross-repository mutation credential is available instead of silently no-opping. Every swept repository prints its per-PR decision log, so an unmerged PR always has a concrete logged reason at most one hour old. - Queue hygiene posture: during the sweep, workflow runs still `queued` after `ORG_SWEEP_STALE_QUEUE_HOURS` (default 24h) are cancelled with their run id, workflow name, head branch, and age logged. A run queued that long belongs to a head that PR events will never revisit (closed PR, force-pushed branch, or a previous runner outage), and leaving it keeps the Actions queue holding non-current-head work. -- Inaccessible-repository posture: a sibling repository the sweep credential structurally cannot read — the OpenCode app is not installed there, or `PR_REVIEW_MERGE_TOKEN` does not cover it — returns HTTP 403 `Resource not accessible by integration` on every read. That is an access-grant fact the automation can never resolve, so the sweep classifies it as a skipped, non-fatal **unavailable** repository (a `::warning` naming the repository and the remediation) instead of a hard failure. Without this, a handful of un-enrolled repositories keeps the scheduled sweep heartbeat (the org sweep's `*/15 * * * *` cron) permanently red and masks a genuinely new repository that starts failing. Fail-closed is preserved on both sides: any non-403 scheduler failure still fails the sweep with its per-PR reason, and if more than `ORG_SWEEP_MAX_UNAVAILABLE` (default 5) repositories become unreachable in one pass — a credential-scope regression rather than a few un-enrolled repos — the job fails loudly. Remediation for a listed repository is to install the OpenCode app on it or grant `PR_REVIEW_MERGE_TOKEN` access. +- Inaccessible-repository posture: a sibling repository the sweep credential structurally cannot read — the OpenCode app is not installed there, or `PR_REVIEW_MERGE_TOKEN` does not cover it — returns HTTP 403 `Resource not accessible by integration` on every read. That is an access-grant fact the automation can never resolve, so the sweep classifies it as a skipped, non-fatal **unavailable** repository (a `::warning` naming the repository and the remediation) instead of a hard failure. Without this, a handful of un-enrolled repositories keeps the scheduled sweep heartbeat (the org sweep's `0 * * * *` cron) permanently red and masks a genuinely new repository that starts failing. Fail-closed is preserved on both sides: any non-403 scheduler failure still fails the sweep with its per-PR reason, and if more than `ORG_SWEEP_MAX_UNAVAILABLE` (default 5) repositories become unreachable in one pass — a credential-scope regression rather than a few un-enrolled repos — the job fails loudly. Remediation for a listed repository is to install the OpenCode app on it or grant `PR_REVIEW_MERGE_TOKEN` access. ## Second-reviewer (Noema) posture diff --git a/tests/test_actions_queue_saturation_scheduler_cadence.py b/tests/test_actions_queue_saturation_scheduler_cadence.py new file mode 100644 index 0000000000..fbf8f45547 --- /dev/null +++ b/tests/test_actions_queue_saturation_scheduler_cadence.py @@ -0,0 +1,33 @@ +"""Regression contract for the organization scheduler queue-saturation repair.""" + +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +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.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + assert '- cron: "0 * * * *"' in workflow or "- cron: '0 * * * *'" 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.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + assert workflow.count("$(date -u +%s) / 3600") == 2 + assert "$(date -u +%s) / 900" not in workflow + assert "900s window" not in workflow + assert "900s)" not in workflow + assert "pending */15 sweep" not in workflow + + +def test_repository_scheduler_keeps_event_driven_wakes() -> None: + """Capacity repair must preserve event-driven admission rather than polling only.""" + workflow = WORKFLOW.read_text(encoding="utf-8") + assert "pull_request_target:" in workflow + assert "pull_request_review:" in workflow + assert "workflow_run:" in workflow + assert "repository_dispatch:" in workflow diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 9823c417c1..e18e021fae 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -967,7 +967,7 @@ def test_org_queue_sweep_covers_target_repositories_on_a_heartbeat() -> None: 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 - every 15 minutes so an approval that lands after a PR's last event is + 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 distinct concurrency key from the separate 30-minute scan, and the job has enough runtime headroom to finish a complete organization walk. @@ -975,9 +975,9 @@ def test_org_queue_sweep_covers_target_repositories_on_a_heartbeat() -> None: workflow = workflow_text("pr-review-merge-scheduler.yml") assert "org-queue-sweep:" in workflow - assert '- cron: "*/15 * * * *"' in workflow + assert '- cron: "0 * * * *"' in workflow assert "github.repository == 'ContextualWisdomLab/.github'" in workflow - assert "github.event.schedule == '*/15 * * * *'" in workflow + assert "github.event.schedule == '0 * * * *'" in workflow assert "github.event.client_payload.org_sweep == true" in workflow assert ( "github.event_name == 'schedule' && format('schedule-{0}', " @@ -994,7 +994,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 != '*/15 * * * *'" in workflow + assert "github.event.schedule != '0 * * * *'" 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 ( @@ -1494,7 +1494,7 @@ def test_org_queue_sweep_treats_inaccessible_repositories_as_non_fatal() -> None automation can never resolve, so those repositories are reported as skipped, non-fatal "unavailable" repositories rather than hard failures — otherwise a handful of un-enrolled repositories keeps the scheduled sweep (the - ``*/15 * * * *`` cron) permanently red and masks a genuinely new repository + ``0 * * * *`` cron) permanently red and masks a genuinely new repository that starts failing. The sweep stays fail-closed two ways: any non-403 scheduler failure still