diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 99bd6a89b..123f5c90a 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -611,6 +611,17 @@ jobs: ORG_SWEEP_MERGE_MODE: ${{ github.event.client_payload.merge_mode || inputs.merge_mode || 'direct_or_auto' }} ORG_SWEEP_UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false || inputs.update_branches == true }} ORG_SWEEP_STALE_QUEUE_HOURS: ${{ vars.ORG_SWEEP_STALE_QUEUE_HOURS || '24' }} + # The review-dispatch and branch-update budgets above are organization-wide + # per sweep tick (sized to bound LLM review-provider cost/rate exposure, not + # per-repository). Without rotation, `sweep_targets` is walked in a fixed + # order every tick (the org repos API response order), so the same early + # repositories always exhaust the shared budget and every later repository + # starves indefinitely even with zero-open-thread, all-green PRs + # (ContextualWisdomLab/.github#1219). `github.run_number` increments on + # every run of this workflow, so rotating the walk order by it spreads the + # same fixed total budget across repositories over successive ticks instead + # of raising it. + ORG_SWEEP_ROTATION_INDEX: ${{ github.run_number }} # A repository the sweep credential structurally cannot read (the OpenCode # app is not installed there / the PR_REVIEW_MERGE_TOKEN lacks it) returns # HTTP 403 "Resource not accessible by integration". That is an access-grant @@ -815,6 +826,10 @@ jobs: echo "::error::ORG_SWEEP_BRANCH_UPDATE_LIMIT must be -1 or a non-negative integer; got '${ORG_SWEEP_BRANCH_UPDATE_LIMIT}'. Fix the ORG_SWEEP_BRANCH_UPDATE_LIMIT repository variable." exit 1 fi + if ! [[ "$ORG_SWEEP_ROTATION_INDEX" =~ ^[0-9]+$ ]]; then + echo "::error::ORG_SWEEP_ROTATION_INDEX must be a non-negative integer; got '${ORG_SWEEP_ROTATION_INDEX}'. This is derived from github.run_number and should never be malformed." + exit 1 + fi repositories_json="$( gh api \ @@ -829,7 +844,23 @@ jobs: | "\(.full_name)\t\(.default_branch)" ' <<<"$repositories_json" ) - echo "Sweeping ${#sweep_targets[@]} repositories." + sweep_target_count=${#sweep_targets[@]} + # Rotate the fixed walk order by the run number so the same + # organization-wide review-dispatch/branch-update budget lands on a + # different starting repository each tick instead of always exhausting + # on the same early repositories (#1219). Total dispatches per tick are + # unchanged; only which repositories receive them rotates over time. + rotation_offset=0 + if [ "$sweep_target_count" -gt 0 ]; then + rotation_offset=$(( ORG_SWEEP_ROTATION_INDEX % sweep_target_count )) + if [ "$rotation_offset" -gt 0 ]; then + sweep_targets=( + "${sweep_targets[@]:rotation_offset}" + "${sweep_targets[@]:0:rotation_offset}" + ) + fi + fi + echo "Sweeping ${sweep_target_count} repositories starting at rotation offset ${rotation_offset} (run number ${ORG_SWEEP_ROTATION_INDEX})." failures=0 unavailable=0 diff --git a/CHANGELOG.md b/CHANGELOG.md index e1f19bc01..22ba60e64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Rotated `org-queue-sweep`'s repository walk order by the workflow's own run number before applying the shared organization-wide review-dispatch/branch-update budget, so a fixed early repository in the unsorted `gh api /orgs/{org}/repos` walk order can no longer permanently starve every later repository's ready, all-green, zero-open-thread pull requests of the single per-tick dispatch (`ContextualWisdomLab/.github#1219`). The total per-tick budget is unchanged; only which repository consumes it rotates. - Forward `trigger_reviews=true` explicitly from the trusted OpenCode mention wrapper to the authoritative scheduler while retaining GitHub's ten-key dispatch limit. Source-comment identity remains bound in the verified invocation claim and durable ledger instead of occupying an unused scheduler field, so a successfully routed `@opencode-agent` request now dispatches review work rather than entering queue maintenance with reviews disabled. - Allowed an allowlisted base repository's open fork-head PR to enter the central exact-head OpenCode review path. The scheduler and privileged reviewer still re-read the live PR, bind base/head refs and SHAs, reject malformed repository identities, keep fork source as untrusted data, preserve the existing maintainer-writable update rule, and reserve the final external-head merge for a maintainer. - Confined OSV base and head repository checkouts to the same `source/` child directory, so a cross-fork head checkout can replace that repository without deleting the base-scan JSON held at the workspace root. Both scans retain identical source paths and the required base/head vulnerability comparison remains fail-closed. diff --git a/docs/doctoring/org-queue-sweep-rotation.md b/docs/doctoring/org-queue-sweep-rotation.md new file mode 100644 index 000000000..e6240879e --- /dev/null +++ b/docs/doctoring/org-queue-sweep-rotation.md @@ -0,0 +1,71 @@ +# Org-queue-sweep review-dispatch rotation + +## Problem + +`org-queue-sweep` in `pr-review-merge-scheduler.yml` walks every organization +repository once per 15-minute tick and consumes one shared, organization-wide +review-dispatch budget (`ORG_SWEEP_REVIEW_DISPATCH_LIMIT`, default `1`) across +that entire walk. The walk order came from a single `gh api +/orgs/{org}/repos` call with no explicit sort, so it was effectively fixed +across ticks. A repository early in that fixed order always consumed the +single available dispatch, so every later repository's ready, all-green, +zero-open-thread pull requests never reached the OpenCode review dispatch +through the sweep fallback path — indefinitely, not just for one tick. + +`ContextualWisdomLab/.github#1219` recorded this with direct evidence from a +RankWeave sweep run: PRs #36, #40, and #41 all reported `review dispatch limit +reached` in the same run where the org-wide budget was already `1/1` before +RankWeave's own turn. + +## Decision + +Rotate the sweep's repository walk order by `github.run_number` (a value +GitHub increments on every run of this workflow) before applying the +unchanged organization-wide budget. `rotation_offset = run_number % +repository_count`; the walk starts at that offset and wraps. This spreads the +exact same total per-tick dispatch budget across repositories over successive +ticks instead of raising it. + +The budget-sizing question in #1219 (is `1` a deliberate LLM-provider +cost/rate ceiling, or an unconsidered default?) is explicitly **not** +resolved here. Raising the shared number without that context risks the +exact provider budget/rate-limit incident already documented in +`PR_GOVERNANCE_AUDIT.md` (2026-07-13 KST GitHub Models org budget cap +starvation). Rotation fixes starvation-by-fixed-order without touching that +open cost question; whoever has organization Billing/Budgets visibility can +still raise `vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT` independently later if the +ceiling turns out to be conservative. + +## Consequences + +- Every repository with ready work eventually reaches the front of the walk + order and receives the shared dispatch, bounded by `repository_count` + ticks in the worst case, instead of never. +- Total review dispatches per tick, and therefore LLM-provider call volume + per tick, are unchanged. +- `rotation_offset` is logged (`Sweeping N repositories starting at rotation + offset O (run number R).`) so a specific tick's walk order is reconstructable + from the run log alone. +- `ORG_SWEEP_ROTATION_INDEX` follows the same fail-closed numeric-validation + pattern as the sibling `ORG_SWEEP_*_LIMIT` variables (reject non-digit + input before it reaches arithmetic context, where an unguarded `set -e` + would not trap the error). + +## Verification + +- `tests/test_required_workflow_queue_contract.py::test_org_queue_sweep_rotation_offset_is_deterministic_and_reorders_targets` + executes the extracted rotation snippet directly through `bash -euo + pipefail` for several rotation indices and asserts the resulting order is a + full permutation of the input, not a subset. +- `test_org_queue_sweep_rotation_offset_is_safe_with_no_targets` covers the + zero-repository edge case. +- `test_org_queue_sweep_documents_rotation_leverage_and_validates_input` + locks the `#1219` cross-reference and confirms the shared budget constant + itself is untouched. +- `actionlint` (with `shellcheck` on `PATH`) reports no findings against the + modified workflow. + +## References + +`ContextualWisdomLab/.github#1219` — original starvation report with sweep +run evidence. diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 49115b3e2..caca1a4b0 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -784,6 +784,87 @@ def test_org_queue_sweep_superseded_run_log_filter_executes() -> None: assert "current_head=closed-or-no-open-pr" in result.stdout +def _extract_org_sweep_rotation_snippet(workflow: str) -> str: + """Return only the rotation-offset bash block, without the surrounding + `gh api`/dispatch logic that would require live network credentials.""" + + start_marker = " sweep_target_count=${#sweep_targets[@]}\n" + end_marker = 'run number ${ORG_SWEEP_ROTATION_INDEX})."\n' + start = workflow.index(start_marker) + end = workflow.index(end_marker, start) + len(end_marker) + return textwrap.dedent(workflow[start:end]) + + +def test_org_queue_sweep_rotation_offset_is_deterministic_and_reorders_targets() -> None: + """Rotating the sweep walk order must preserve every target and only reorder them.""" + workflow = workflow_text("pr-review-merge-scheduler.yml") + snippet = _extract_org_sweep_rotation_snippet(workflow) + + for rotation_index, expected_first in ( + ("0", "repo-a"), + ("1", "repo-b"), + ("2", "repo-c"), + ("5", "repo-a"), # 5 % 5 == 0: wraps back to unrotated order + ("7", "repo-c"), # 7 % 5 == 2 + ): + script = ( + "sweep_targets=($'repo-a\\tmain' $'repo-b\\tmain' $'repo-c\\tmain' " + "$'repo-d\\tmain' $'repo-e\\tmain')\n" + + snippet + + '\nprintf "%s\\n" "${sweep_targets[@]}"\n' + ) + result = subprocess.run( + ["bash", "-euo", "pipefail", "-c", script], + env={**os.environ, "ORG_SWEEP_ROTATION_INDEX": rotation_index}, + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr + rotated = [ + line.split("\t")[0] + for line in result.stdout.strip().splitlines() + if "\t" in line + ] + assert len(rotated) == 5 + assert set(rotated) == {"repo-a", "repo-b", "repo-c", "repo-d", "repo-e"} + assert rotated[0] == expected_first, (rotation_index, result.stdout) + + +def test_org_queue_sweep_rotation_offset_is_safe_with_no_targets() -> None: + """An org with no sweepable repositories must not crash the rotation arithmetic.""" + workflow = workflow_text("pr-review-merge-scheduler.yml") + snippet = _extract_org_sweep_rotation_snippet(workflow) + script = "sweep_targets=()\n" + snippet + result = subprocess.run( + ["bash", "-euo", "pipefail", "-c", script], + env={**os.environ, "ORG_SWEEP_ROTATION_INDEX": "3"}, + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stderr + assert "starting at rotation offset 0" in result.stdout + + +def test_org_queue_sweep_documents_rotation_leverage_and_validates_input() -> None: + """Record why rotation exists and keep the new input on the same fail-closed contract.""" + workflow = workflow_text("pr-review-merge-scheduler.yml") + + assert ( + "ORG_SWEEP_ROTATION_INDEX: ${{ github.run_number }}" + ) in workflow + assert "ContextualWisdomLab/.github#1219" in workflow + assert ( + 'if ! [[ "$ORG_SWEEP_ROTATION_INDEX" =~ ^[0-9]+$ ]]; then' + ) in workflow + assert ( + "rotation_offset=$(( ORG_SWEEP_ROTATION_INDEX % sweep_target_count ))" + ) in workflow + # The fix must not change the org-wide budget itself, only which + # repositories consume it — otherwise it reintroduces the exact + # cost/rate-limit risk #1219 explicitly declined to guess at. + assert "vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '1'" in workflow + + def test_org_queue_sweep_manual_cadence_inputs_reach_the_sweep_job() -> None: """Manual full-sweep cadence must override repository variables and defaults.""" workflow = workflow_text("pr-review-merge-scheduler.yml")