diff --git a/.github/workflows/agent-review-runtime-quality-ci.yml b/.github/workflows/agent-review-runtime-quality-ci.yml index 8bef3fa85f..8cf413a4ec 100644 --- a/.github/workflows/agent-review-runtime-quality-ci.yml +++ b/.github/workflows/agent-review-runtime-quality-ci.yml @@ -34,7 +34,6 @@ on: - ".github/workflows/pr-review-merge-scheduler.yml" - ".github/workflows/current-head-run-coalescer.yml" - "scripts/ci/current_head_run_coalescer.py" - - "tests/test_org_sweep_queue_hygiene_owner.py" - ".github/workflows/pr-review-fix-scheduler.yml" - "scripts/ci/pr_review_fix_scheduler.py" - ".github/workflows/pr-review-autofix.yml" @@ -207,8 +206,7 @@ jobs: ;; .github/workflows/pr-review-merge-scheduler.yml|\ .github/workflows/current-head-run-coalescer.yml|\ - scripts/ci/current_head_run_coalescer.py|\ - tests/test_org_sweep_queue_hygiene_owner.py) + scripts/ci/current_head_run_coalescer.py) queue_suite=true ;; .github/workflows/pr-review-fix-scheduler.yml|\ @@ -369,8 +367,8 @@ jobs: if: steps.affected_suites.outputs.queue == 'true' run: | set -euo pipefail - python -m pytest -q tests/test_org_sweep_queue_hygiene_owner.py - python -m compileall -q tests/test_org_sweep_queue_hygiene_owner.py + python -m pytest -q tests/test_current_head_coalescer_self_cancellation.py + python -m compileall -q tests/test_current_head_coalescer_self_cancellation.py - name: Verify scheduler and contextual-orchestrator review-repair contracts if: steps.affected_suites.outputs.review_repair == 'true' diff --git a/.github/workflows/current-head-run-coalescer.yml b/.github/workflows/current-head-run-coalescer.yml index 84a3560421..187aefe4cc 100644 --- a/.github/workflows/current-head-run-coalescer.yml +++ b/.github/workflows/current-head-run-coalescer.yml @@ -10,11 +10,11 @@ permissions: pull-requests: read concurrency: - # Head identity keeps a delayed stale event from cancelling the live-head - # cleanup while still collapsing duplicate events for the same PR head. + # Keep one admitted run per repository and PR at workflow admission. Exact + # HEAD identity is revalidated by the first job step before any mutation. group: >- current-head-run-coalescer-${{ github.repository }}-${{ - github.event.pull_request.number }}-${{ github.event.pull_request.head.sha }} + github.event.pull_request.number }} cancel-in-progress: true jobs: diff --git a/.github/workflows/noema-review.yml b/.github/workflows/noema-review.yml index f43368ea20..21ea967201 100644 --- a/.github/workflows/noema-review.yml +++ b/.github/workflows/noema-review.yml @@ -14,6 +14,17 @@ on: repository_dispatch: types: [noema-review] +concurrency: + # Workflow-level admission is required: a queued run cannot reach a job-level + # cancellation guard while the organization is at its Actions job ceiling. + group: >- + required-noema-review-${{ + github.event.pull_request.base.repo.full_name || + github.event.client_payload.target_repository || github.repository }}-${{ + github.event.pull_request.number || + github.event.client_payload.pr_number || github.run_id }} + cancel-in-progress: true + permissions: contents: read pull-requests: read @@ -247,17 +258,6 @@ jobs: name: noema-review needs: [admit-current-head] runs-on: ubuntu-24.04 - # Only an exact live head reaches this group. Keeping concurrency at the - # job level prevents a delayed stale event from cancelling the current - # workflow before admit-current-head can reject it. - concurrency: - group: >- - required-noema-review-${{ - github.event.pull_request.base.repo.full_name || - github.event.client_payload.target_repository || github.repository }}-${{ - github.event.pull_request.number || - github.event.client_payload.pr_number || github.run_id }} - cancel-in-progress: true # No job-level timeout-minutes here, deliberately. This job's "Prepare # Noema model verdict" step calls two_phase.py's call_llm synchronously # via the contextual-orchestrator gateway and blocks on the model's own diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 939087bff5..09942e3ca4 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -84,7 +84,6 @@ concurrency: github.event_name == 'workflow_call' && inputs.pr_number != '' && format('pr-{0}', inputs.pr_number) || github.event_name == 'workflow_call' && inputs.base_branch != '' && format('call-{0}', inputs.base_branch) || github.event_name == 'schedule' && format('schedule-{0}', github.event.schedule) || - github.event_name == 'repository_dispatch' && github.event.client_payload.org_sweep == true && format('org-sweep-{0}', github.repository) || github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository != '' && github.event.client_payload.pr_number != '' && format('target-{0}-pr-{1}', github.event.client_payload.target_repository, github.event.client_payload.pr_number) || github.event_name == 'repository_dispatch' && github.event.client_payload.pr_number != '' && format('pr-{0}', github.event.client_payload.pr_number) || github.event_name == 'repository_dispatch' && format('repo-dispatch-{0}', github.repository) || @@ -102,8 +101,6 @@ jobs: scan-pr-queue: # repository_dispatch review runs do not reliably carry pull_requests metadata. # Without this guard, one completed central review can wake a repo-wide scan. - # Explicit org_sweep dispatches are handled by org-queue-sweep below; - # skipping them here avoids a duplicate same-repository scan. if: >- ( github.event_name != 'pull_request_target' || @@ -117,9 +114,7 @@ jobs: # Bound scan-pr-queue to a wall-clock ceiling well short of GitHub's # 360-minute platform default. This is a single-repository queue scan # (paginated GraphQL reads plus at most one review dispatch and one - # branch update per run) -- much lighter than org-queue-sweep's full - # organization walk below, so it gets a shorter bound than that job's - # timeout-minutes: 60. + # branch update per run), so it stays well below GitHub's platform default. timeout-minutes: 30 permissions: actions: write @@ -552,575 +547,3 @@ jobs: args+=(--no-update-branches) fi python3 scripts/ci/pr_review_merge_scheduler.py "${args[@]}" - - org-queue-sweep: - # 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 - # 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 - # injected required workflow, so they receive a separate bounded OpenCode - # dispatch budget and cannot be starved by the ordinary queue. - if: >- - github.repository == 'ContextualWisdomLab/.github' && - ( - 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 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 - permissions: - actions: write - checks: read - contents: write - id-token: write - pull-requests: write - env: - FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - GH_TOKEN: ${{ github.token }} - DRY_RUN: ${{ github.event.client_payload.dry_run == true || inputs.dry_run == true }} - ORG_SWEEP_OWNER: ContextualWisdomLab - # Inspect the complete practical queue for every repository. The previous - # default of 30 silently omitted older PRs whenever a repository had a - # larger queue (BandScope had 34 during the incident that established - # 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' }} - # #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' }} - REVIEW_ADMISSION_DISPATCH_BUDGET: ${{ vars.REVIEW_ADMISSION_DISPATCH_BUDGET || '1' }} - 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 }} - ORG_SWEEP_ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.enable_auto_merge != false || inputs.enable_auto_merge == true }} - 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 }} - # The review-dispatch, stacked-review, 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 a queue's budget and every later repository - # starves indefinitely even with zero-open-thread, all-green PRs - # (ContextualWisdomLab/.github#1219). Left unset here so the sweep step - # below derives it from a persistent per-execution counter (or, as a - # fallback, wall-clock time) instead of `github.run_number`: run_number - # increments on every trigger of this workflow (push, - # pull_request_target, pull_request_review), not only the - # sweep schedule, so it cannot give the "bounded by repository_count - # 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 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 - # HTTP 403 "Resource not accessible by integration". That is an access-grant - # fact the automation can never resolve, so it is reported as a skipped, - # non-fatal "unavailable" repository rather than a hard sweep failure. This - # ceiling keeps the sweep fail-closed against a credential-scope regression: - # if MORE than this many repositories become unreachable at once, the whole - # credential likely broke and the job fails loudly. - ORG_SWEEP_MAX_UNAVAILABLE: ${{ vars.ORG_SWEEP_MAX_UNAVAILABLE || '5' }} - STALE_OPENCODE_MINUTES: ${{ github.event.client_payload.stale_opencode_minutes || inputs.stale_opencode_minutes || vars.STALE_OPENCODE_MINUTES || '90' }} - steps: - - name: Exchange OpenCode app token for sweep mutations - id: sweep_app_token - env: - OIDC_AUDIENCE: opencode-github-action - OPENCODE_API_BASE_URL: https://api.opencode.ai - run: | - set -euo pipefail - - mark_unavailable() { - echo "available=false" >>"$GITHUB_OUTPUT" - } - - if [ -z "${ACTIONS_ID_TOKEN_REQUEST_TOKEN:-}" ] || [ -z "${ACTIONS_ID_TOKEN_REQUEST_URL:-}" ]; then - echo "OpenCode app token exchange unavailable: OIDC request environment is missing." - mark_unavailable - exit 0 - fi - - request_url="${ACTIONS_ID_TOKEN_REQUEST_URL}" - separator="&" - case "$request_url" in - *\?*) ;; - *) separator="?" ;; - esac - - if ! oidc_response="$( - curl -fsS \ - -H "Authorization: Bearer ${ACTIONS_ID_TOKEN_REQUEST_TOKEN}" \ - "${request_url}${separator}audience=${OIDC_AUDIENCE}" - )"; then - echo "OpenCode app token exchange unavailable: OIDC token request did not complete." - mark_unavailable - exit 0 - fi - - oidc_token="$(jq -r '.value // empty' <<<"$oidc_response")" - if [ -z "$oidc_token" ]; then - echo "OpenCode app token exchange unavailable: OIDC token response was empty." - mark_unavailable - exit 0 - fi - - if ! token_response="$( - curl -fsS \ - -X POST \ - -H "Authorization: Bearer ${oidc_token}" \ - "${OPENCODE_API_BASE_URL}/exchange_github_app_token" - )"; then - echo "OpenCode app token exchange unavailable: app token request did not complete." - mark_unavailable - exit 0 - fi - - app_token="$(jq -r '.token // empty' <<<"$token_response")" - if [ -z "$app_token" ]; then - echo "OpenCode app token exchange unavailable: app token response was empty." - mark_unavailable - exit 0 - fi - - echo "::add-mask::$app_token" - { - echo "available=true" - echo "token=$app_token" - } >>"$GITHUB_OUTPUT" - - - name: Resolve trusted scheduler source ref - id: trusted_source - env: - JOB_CONTEXT_JSON: ${{ toJSON(job) }} - GITHUB_CONTEXT_JSON: ${{ toJSON(github) }} - run: | - set -euo pipefail - python3 <<'PY' >>"$GITHUB_OUTPUT" - import json - import os - import re - import sys - - try: - job_context = json.loads(os.environ.get("JOB_CONTEXT_JSON") or "{}") - github_context = json.loads(os.environ.get("GITHUB_CONTEXT_JSON") or "{}") - except json.JSONDecodeError as exc: - print(f"::error::Could not parse GitHub workflow context JSON: {exc}", file=sys.stderr) - raise SystemExit(1) - - trusted_repository = str( - job_context.get("workflow_repository") or "ContextualWisdomLab/.github" - ).strip() - trusted_ref = str( - job_context.get("workflow_sha") or github_context.get("workflow_sha") or "" - ).strip() - workflow_ref = str( - job_context.get("workflow_ref") or github_context.get("workflow_ref") or "" - ).strip() - - if not trusted_ref: - trusted_ref = "main" - prefix = "ContextualWisdomLab/.github/.github/workflows/pr-review-merge-scheduler.yml@" - if workflow_ref.startswith(prefix): - trusted_ref = workflow_ref.split("@", 1)[1] - - if trusted_repository != "ContextualWisdomLab/.github": - print("::error::Trusted scheduler workflow repository resolved outside ContextualWisdomLab/.github.", file=sys.stderr) - raise SystemExit(1) - if not re.fullmatch(r"[0-9a-fA-F]{40}|refs/[^\s]+|[A-Za-z0-9._/-]+", trusted_ref): - print("::error::Trusted scheduler workflow ref resolved to an invalid value.", file=sys.stderr) - raise SystemExit(1) - - print(f"repository={trusted_repository}") - print(f"ref={trusted_ref}") - PY - - - name: Materialize trusted scheduler - env: - GH_TOKEN: ${{ github.token }} - TRUSTED_SOURCE_REF: ${{ steps.trusted_source.outputs.ref }} - run: | - set -euo pipefail - if [[ ! "$TRUSTED_SOURCE_REF" =~ ^[0-9a-fA-F]{40}$ ]]; then - echo "::error::Trusted scheduler source ref must resolve to the immutable workflow commit SHA before archive materialization." - exit 1 - fi - trusted_archive="${RUNNER_TEMP}/trusted-scheduler-source.tar.gz" - api_url="${GITHUB_API_URL:-https://api.github.com}" - curl -fsSL \ - -H "Authorization: Bearer ${GH_TOKEN}" \ - -H "Accept: application/vnd.github+json" \ - -o "$trusted_archive" \ - "${api_url}/repos/ContextualWisdomLab/.github/tarball/${TRUSTED_SOURCE_REF}" - tar -xzf "$trusted_archive" -C "$GITHUB_WORKSPACE" --strip-components=1 - test -f scripts/ci/pr_review_merge_scheduler.py - - - name: Self-test scheduler - run: python3 scripts/ci/pr_review_merge_scheduler.py --self-test - - - name: Sweep organization repository queues - env: - GH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.sweep_app_token.outputs.token || github.token }} - SCHEDULER_ACTIONS_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || steps.sweep_app_token.outputs.token || github.token }} - # The sweep executes inside ContextualWisdomLab/.github, which is exactly - # where the central required workflows are dispatched, so the runner's own - # github.token (contents: write) is a sufficient dispatch credential even - # though the OpenCode app token has no Actions permission. Without this the - # sweep deadlocks every PR that needs current-head review evidence with - # "no cross-repository repository-dispatch credential". - SCHEDULER_DISPATCH_TOKEN: ${{ github.token }} - SCHEDULER_MUTATION_TOKEN_SOURCE: ${{ secrets.PR_REVIEW_MERGE_TOKEN != '' && 'PR_REVIEW_MERGE_TOKEN' || secrets.OPENCODE_APPROVE_TOKEN != '' && 'OPENCODE_APPROVE_TOKEN' || steps.sweep_app_token.outputs.available == 'true' && 'opencode-app' || 'github-token' }} - SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY: ContextualWisdomLab/.github - SCHEDULER_ALLOW_CROSS_REPO_REPOSITORY_DISPATCH: ${{ (secrets.PR_REVIEW_MERGE_TOKEN != '' || secrets.OPENCODE_APPROVE_TOKEN != '') && 'true' || 'false' }} - run: | - set -euo pipefail - case "$STALE_OPENCODE_MINUTES" in - ''|*[!0-9]*) - echo "::error::STALE_OPENCODE_MINUTES must contain only decimal digits" - exit 1 - ;; - esac - if [ "${#STALE_OPENCODE_MINUTES}" -gt 4 ]; then - echo "::error::STALE_OPENCODE_MINUTES must be between 1 and 1440" - exit 1 - fi - stale_opencode_minutes=$((10#$STALE_OPENCODE_MINUTES)) - if [ "$stale_opencode_minutes" -lt 1 ] || [ "$stale_opencode_minutes" -gt 1440 ]; then - echo "::error::STALE_OPENCODE_MINUTES must be between 1 and 1440" - exit 1 - fi - STALE_OPENCODE_MINUTES="$stale_opencode_minutes" - if [ "$SCHEDULER_MUTATION_TOKEN_SOURCE" = "github-token" ]; then - # github.token is repository-scoped to .github and cannot mutate - # sibling repositories; a sweep with it would silently do nothing. - echo "::error::Organization queue sweep has no cross-repository mutation credential. Configure the PR_REVIEW_MERGE_TOKEN or OPENCODE_APPROVE_TOKEN secret (or keep the OpenCode app token exchange available) so approved PRs in target repositories can be merged or updated." - exit 1 - fi - echo "Sweep mutation token source: $SCHEDULER_MUTATION_TOKEN_SOURCE" - - # Validate the fail-closed ceiling before it is used in a numeric test. - # A non-integer would make "[ ... -gt ... ]" error out inside an if - # condition, which set -e does not trap, silently skipping the - # regression guard. Fail loudly instead so a misconfigured - # ORG_SWEEP_MAX_UNAVAILABLE can never quietly disable fail-closed. - if ! [[ "$ORG_SWEEP_MAX_UNAVAILABLE" =~ ^[0-9]+$ ]]; then - echo "::error::ORG_SWEEP_MAX_UNAVAILABLE must be a non-negative integer; got '${ORG_SWEEP_MAX_UNAVAILABLE}'. Fix the ORG_SWEEP_MAX_UNAVAILABLE repository variable." - exit 1 - fi - if ! [[ "$ORG_SWEEP_REVIEW_DISPATCH_LIMIT" =~ ^(-1|[0-9]+)$ ]]; then - echo "::error::ORG_SWEEP_REVIEW_DISPATCH_LIMIT must be -1 or a non-negative integer; got '${ORG_SWEEP_REVIEW_DISPATCH_LIMIT}'. Fix the ORG_SWEEP_REVIEW_DISPATCH_LIMIT repository variable." - exit 1 - fi - if ! [[ "$ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT" =~ ^(-1|[0-9]+)$ ]]; then - echo "::error::ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT must be -1 or a non-negative integer; got '${ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT}'. Fix the ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT repository variable." - exit 1 - fi - if ! [[ "$ORG_SWEEP_BRANCH_UPDATE_LIMIT" =~ ^(-1|[0-9]+)$ ]]; then - 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 - # Unset in production (see the env-block comment above). Primary - # 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 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 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 - # per-execution counter advances by exactly one every time the - # sweep body actually runs, regardless of how much wall-clock time - # a slow prior run consumed. Falls back to the wall-clock tick, - # which still strictly improves on the pre-#1220 fixed order, only - # if the counter read/write itself is unavailable (permissions, - # transient API failure) -- a fairness mechanism must never fail - # the sweep's much more important review-dispatch/merge work. - # Tests inject ORG_SWEEP_ROTATION_INDEX directly for determinism, - # which this only fills in when absent. - # - # Two known, accepted limitations of this counter (Devin review on - # #1223), neither of which is fixed here: - # - Read-modify-write is not atomic. A schedule-triggered run and a - # manual `repository_dispatch` org_sweep run use different - # concurrency groups and can therefore execute concurrently, in - # which case both could read the same counter value and pick the - # same rotation offset for that one pair of runs. The REST - # Variables API has no compare-and-swap primitive to close this - # without a broader concurrency-group redesign shared across - # every trigger type this workflow serves; the consequence is - # bounded and self-correcting (one occasionally-repeated offset, - # not a stuck one), so it is accepted rather than redesigned. - # - Whether the PATCH/POST below ever succeeds in production - # depends on the resolved token actually holding repository - # Variables-write scope, which is not independently verifiable - # from inside this workflow. If it does not, every run silently - # but safely degrades to the wall-clock fallback below (logged - # via ::warning:: each time), which is still strictly better - # than the pre-#1220 fixed order -- never a hard failure, and - # observable in the run log for whoever holds that token. - if [ -z "${ORG_SWEEP_ROTATION_INDEX:-}" ]; then - counter_variable_name="ORG_SWEEP_ROTATION_COUNTER" - # Distinguish a *successful* read (the variable exists; its - # value, valid or not, is authoritative) from a *failed* read - # (transient error, permissions, or the variable genuinely - # doesn't exist yet -- indistinguishable from here). Only a - # successful read may PATCH: a transient failure that silently - # became "treat as 0" would let the PATCH below clobber an - # already-accumulated counter value back down to 1, restarting - # the rotation sequence instead of degrading to the wall-clock - # fallback the design intends (Devin review finding on #1223). - if counter_current="$( - gh api "repos/${GITHUB_REPOSITORY}/actions/variables/${counter_variable_name}" \ - --jq '.value' 2>/dev/null - )"; then - if ! [[ "$counter_current" =~ ^[0-9]+$ ]]; then - counter_current=0 - fi - # Force base-10: a manually-seeded value with a leading zero - # (e.g. "08") passes the digit-only check above but bash's - # unprefixed arithmetic parses a leading-zero literal as - # octal, and "08"/"09" are not valid octal digits -- errors - # under set -e. $((10#...)) is the same guard already used - # elsewhere in this file (STALE_OPENCODE_MINUTES). - counter_next=$(( 10#$counter_current + 1 )) - if gh api "repos/${GITHUB_REPOSITORY}/actions/variables/${counter_variable_name}" \ - -X PATCH -f "value=${counter_next}" >/dev/null 2>&1; then - 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) / 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 - # The read failed, so this is only safe as a first-run - # create: POST fails on its own if the variable actually - # already exists (a real read outage rather than a genuinely - # missing variable), which correctly falls through to the - # wall-clock branch below instead of resetting a value this - # run could not see. - 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) / 86400 )) - fi - 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}'." - exit 1 - fi - - repositories_json="$( - gh api \ - -H "Accept: application/vnd.github+json" \ - "/orgs/${ORG_SWEEP_OWNER}/repos?per_page=100&type=all" --paginate - )" - mapfile -t sweep_targets < <( - 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" - ) - sweep_target_count=${#sweep_targets[@]} - # Rotate the fixed walk order by ORG_SWEEP_ROTATION_INDEX (see - # above: a persistent per-execution counter, falling back to a - # wall-clock tick) so the same organization-wide review-dispatch - # /branch-update budgets land on a different starting repository each - # execution instead of always exhausting on the same early - # repositories (#1219). The ordinary and stacked review budgets are - # tracked independently so the latter cannot be starved by the former. - 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} (rotation tick ${ORG_SWEEP_ROTATION_INDEX})." - - failures=0 - unavailable=0 - unavailable_repos=() - rate_limited=0 - rate_limited_repos=() - # These are organization-wide budgets. They must be consumed across - # the repository loop, not reset for every target repository; resetting - # them here can enqueue hundreds of long-running review jobs per sweep. - org_review_dispatches_used=0 - org_stacked_review_dispatches_used=0 - org_branch_updates_used=0 - for target in "${sweep_targets[@]}"; do - repo_full_name="${target%%$'\t'*}" - default_branch="${target##*$'\t'}" - echo "::group::Sweep ${repo_full_name} (base ${default_branch})" - - open_pr_count="$( - gh api \ - -H "Accept: application/vnd.github+json" \ - "/repos/${repo_full_name}/pulls?state=open&per_page=1" \ - --jq 'length' || echo "unknown" - )" - if [ "$open_pr_count" = "0" ]; then - echo "No open PRs (including stacked or non-default-base PRs); skipping." - echo "::endgroup::" - continue - fi - - # The scheduler requires --project-flow. Derive it per target the - # same way the single-repository job does: main/master default - # branches are GitHub Flow, develop is Git Flow, anything else - # defaults to GitHub Flow. - case "$default_branch" in - main|master) project_flow="github-flow" ;; - develop) project_flow="git-flow" ;; - *) project_flow="github-flow" ;; - esac - - if [ "$ORG_SWEEP_REVIEW_DISPATCH_LIMIT" = "-1" ]; then - review_dispatch_limit=-1 - else - review_dispatch_limit=$((ORG_SWEEP_REVIEW_DISPATCH_LIMIT - org_review_dispatches_used)) - if (( review_dispatch_limit < 0 )); then - review_dispatch_limit=0 - fi - fi - if [ "$ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT" = "-1" ]; then - stacked_review_dispatch_limit=-1 - else - stacked_review_dispatch_limit=$((ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT - org_stacked_review_dispatches_used)) - if (( stacked_review_dispatch_limit < 0 )); then - stacked_review_dispatch_limit=0 - fi - fi - if [ "$ORG_SWEEP_BRANCH_UPDATE_LIMIT" = "-1" ]; then - branch_update_limit=-1 - else - branch_update_limit=$((ORG_SWEEP_BRANCH_UPDATE_LIMIT - org_branch_updates_used)) - if (( branch_update_limit < 0 )); then - branch_update_limit=0 - fi - fi - - args=( - --repo "$repo_full_name" - --base-branch "$default_branch" - --project-flow "$project_flow" - --max-prs "$ORG_SWEEP_MAX_PRS" - --review-workflow "Required OpenCode Review" - --review-dispatch-limit "$review_dispatch_limit" - --admission-state-path "${RUNNER_TEMP}/review-admission/state.json" - --admission-dispatch-budget "$REVIEW_ADMISSION_DISPATCH_BUDGET" - --admission-sequence "$GITHUB_RUN_ID" - --stacked-review-dispatch-limit "$stacked_review_dispatch_limit" - --branch-update-limit "$branch_update_limit" - --stale-opencode-minutes "$STALE_OPENCODE_MINUTES" - --merge-mode "$ORG_SWEEP_MERGE_MODE" - ) - if [ "$ORG_SWEEP_TRIGGER_REVIEWS" = "true" ]; then - args+=(--trigger-reviews) - fi - if [ "$ORG_SWEEP_ENABLE_AUTO_MERGE" = "true" ]; then - args+=(--enable-auto-merge) - fi - if [ "$ORG_SWEEP_UPDATE_BRANCHES" = "true" ]; then - args+=(--update-branches) - fi - if [ "$DRY_RUN" = "true" ]; then - args+=(--dry-run) - fi - set +e - sweep_output="$(python3 scripts/ci/pr_review_merge_scheduler.py "${args[@]}" 2>&1)" - sweep_rc=$? - set -e - printf '%s\n' "$sweep_output" - repo_stacked_review_dispatches="$(printf '%s\n' "$sweep_output" | grep -Ec '^PR #[0-9]+: review_dispatch: stacked PR onto' || true)" - repo_review_dispatches_total="$(printf '%s\n' "$sweep_output" | grep -Ec '^PR #[0-9]+: (review_dispatch|security_dispatch):' || true)" - repo_review_dispatches=$((repo_review_dispatches_total - repo_stacked_review_dispatches)) - repo_branch_updates="$(printf '%s\n' "$sweep_output" | grep -Ec '^PR #[0-9]+: (update_branch|restamp_head):' || true)" - org_review_dispatches_used=$((org_review_dispatches_used + repo_review_dispatches)) - org_stacked_review_dispatches_used=$((org_stacked_review_dispatches_used + repo_stacked_review_dispatches)) - org_branch_updates_used=$((org_branch_updates_used + repo_branch_updates)) - echo "Org sweep budget consumed: review dispatches=${org_review_dispatches_used}/${ORG_SWEEP_REVIEW_DISPATCH_LIMIT}, stacked review dispatches=${org_stacked_review_dispatches_used}/${ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT}, branch updates=${org_branch_updates_used}/${ORG_SWEEP_BRANCH_UPDATE_LIMIT}." - if [ "$sweep_rc" -ne 0 ]; then - # A structural access denial ("Resource not accessible by - # integration") means the sweep credential cannot read this - # repository at all — the OpenCode app is not installed there or - # PR_REVIEW_MERGE_TOKEN does not cover it. The automation can never - # merge those PRs regardless, so this is a skipped, non-fatal - # "unavailable" repository, not a failure the sweep can act on. - # - # "API rate limit exceeded" means the shared GitHub App - # installation-token bucket (5,000-12,500 requests/hour, pooled - # across at least eight other central workflows that mint tokens - # for the same installation) is exhausted for this hourly window. - # That is routine cross-workflow contention, not a defect in this - # repository, and it self-heals on GitHub's own reset schedule; - # treating it as a hard failure previously turned one exhausted - # bucket into a permanently red cron run on essentially every - # scheduled tick for as long as the contention lasted. Because - # the installation bucket is shared by every remaining - # repository, the current rotation stops after - # recording the first exhausted request instead of repeating the - # same bounded scheduler retries for every target. - # Deferred work is picked up on a later rotation after reset. - # - # Any other non-zero exit is a genuine per-repository failure. - if printf '%s' "$sweep_output" | grep -qF "Resource not accessible by integration"; then - echo "::warning::Skipping ${repo_full_name}: the sweep credential lacks access (HTTP 403 Resource not accessible by integration). Install the OpenCode app on this repository or grant PR_REVIEW_MERGE_TOKEN access to include it in the sweep." - unavailable=$((unavailable + 1)) - unavailable_repos+=("$repo_full_name") - elif printf '%s' "$sweep_output" | grep -qiF "API rate limit exceeded"; then - echo "::warning::Deferring ${repo_full_name} and stopping this rotation: the shared GitHub App installation-token rate limit is exhausted (HTTP 403 API rate limit exceeded). Deferred repositories are retried automatically on the next sweep rotation once the bucket resets." - rate_limited=$((rate_limited + 1)) - rate_limited_repos+=("$repo_full_name") - echo "::endgroup::" - break - else - echo "::error::Queue sweep failed for ${repo_full_name}; see the decision log above for the concrete per-PR reason." - failures=$((failures + 1)) - fi - fi - - echo "::endgroup::" - done - - if [ "$unavailable" -gt 0 ]; then - echo "::warning::${unavailable} repository(ies) were skipped as unreachable by the sweep credential (HTTP 403): ${unavailable_repos[*]}. These do not fail the sweep; install the OpenCode app or grant PR_REVIEW_MERGE_TOKEN access to include them." - fi - if [ "$rate_limited" -gt 0 ]; then - # No fail-closed ceiling here, unlike ORG_SWEEP_MAX_UNAVAILABLE below: - # one exhausted shared installation-token bucket affects every - # remaining repository, so the rotation stops after the first - # observed exhaustion instead of multiplying retries and API calls. - echo "::warning::The organization sweep stopped after ${rate_limited} observed rate-limit exhaustion(s): ${rate_limited_repos[*]}. Deferred work does not fail this sweep and is retried automatically once the shared bucket resets." - fi - # Fail-closed guard: a handful of un-enrolled repositories is expected, - # but if MORE than ORG_SWEEP_MAX_UNAVAILABLE repositories become - # unreachable at once the sweep credential itself has regressed and the - # job must fail loudly rather than silently sweeping nothing. - if [ "$unavailable" -gt "$ORG_SWEEP_MAX_UNAVAILABLE" ]; then - echo "::error::Sweep credential could not access ${unavailable} repositories (limit ${ORG_SWEEP_MAX_UNAVAILABLE}); this indicates a credential-scope regression, not a few un-enrolled repositories. Verify PR_REVIEW_MERGE_TOKEN / the OpenCode app installation." - exit 1 - fi - if [ "$failures" -gt 0 ]; then - echo "::error::Organization queue sweep completed with ${failures} repository failure(s); each failure's reason is printed in its repository group above." - exit 1 - fi - echo "Organization queue sweep completed cleanly." diff --git a/AGENTS.md b/AGENTS.md index cf8df236be..7330ec14cd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,3 +31,27 @@ see [`docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md`](docs/adr/0003 false claim of explicit owner direction and records the resulting availability risk as open and unreviewed, not accepted. The materialization contract is also covered by [`docs/doctoring/exact-artifact-sbom-attestation.md`](docs/doctoring/exact-artifact-sbom-attestation.md). + +## Actions queue and protected-merge procedure + +- Use `github-actions-privileged-pr-scan` when a PR scanner can reach secrets, + and use `github-robot-review-gate` plus `babysit-pr` when diagnosing or + monitoring a protected PR. If a named skill is unavailable, preserve its + fail-closed trust boundary and exact-current-head evidence rules manually. +- PR-triggered workflow concurrency must be trigger-aware. Group by workflow, + target repository, and pull request number with `cancel-in-progress: true`; + do not include the head SHA, because that prevents a new head from cancelling + its predecessor. Non-PR triggers need an explicit collision-safe fallback. +- Put concurrency at workflow scope when queued jobs must be coalesced before a + runner is admitted. Job-level concurrency cannot relieve a saturated runner + queue because it is evaluated only after job admission. +- Keep cleanup repository-local and event-driven. Do not restore an + organization-wide queue sweep, polling `sleep`, or another scheduled scan to + compensate for incorrect concurrency. Cancel only runs proven to belong to a + superseded head of the same PR, then verify each accepted cancellation + reaches `completed/cancelled`. +- Before every review, retry, push, or merge claim, re-fetch the PR's exact head + SHA, base SHA, review threads, required checks, and ruleset result. A push + invalidates earlier checks and reviews. Never self-approve, dismiss reviews, + force-push, disable a security gate, or use admin bypass for product or + security changes. diff --git a/CHANGELOG.md b/CHANGELOG.md index d134f47b89..4b77f809e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ - 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] +- Remove the central `org-queue-sweep` runner and its organization-wide + repository walk. Native PR/review events, auto-merge, trigger-aware + same-PR cancellation, and each repository's daily `scan-pr-queue` recovery + remain the bounded queue owners. +- Move Noema's repository-and-PR concurrency group to workflow admission so a + new HEAD cancels its stale queued run before either consumes a job slot. +- Scope the current-head coalescer's workflow admission to repository and PR, + while retaining exact-HEAD revalidation inside the trusted job. - Align current-main workflow contract tests with native auto-merge completion, validated dispatch concurrency keys, rotating queue pagination, globbed watch paths, admission jobs, and the reviewed OpenCode dispatch blob. diff --git a/docs/doctoring/current-head-run-coalescing.md b/docs/doctoring/current-head-run-coalescing.md index 830c6098c7..94eed3c424 100644 --- a/docs/doctoring/current-head-run-coalescing.md +++ b/docs/doctoring/current-head-run-coalescing.md @@ -10,7 +10,7 @@ The live-ref queue-hygiene repair from #1348 correctly prevents stale pull-reque `.github/workflows/current-head-run-coalescer.yml` executes on trusted `pull_request_target` events for `opened`, `synchronize`, `reopened`, `ready_for_review`, and `converted_to_draft`. It checks out `ContextualWisdomLab/.github` at immutable `github.workflow_sha` with persisted credentials disabled. The job has only `actions: write`, `contents: read`, and `pull-requests: read`; it never checks out or executes pull-request-head code. Event-derived repository/ref/SHA values are first placed in environment variables and are referenced from the shell only as quoted variables, so PR-controlled branch names are never interpolated directly into executable shell text. -The live-head admission and coalescing work share one job. Workflow-level concurrency includes repository, PR number, and event head SHA: duplicate events for one exact head collapse, while a delayed stale-head event cannot cancel the live-head cleanup. The first step re-fetches the PR and gates the remaining steps. This avoids the former two-job admission dependency, which could leave the cleanup worker waiting behind the queue pressure it was meant to relieve. +The live-head admission and coalescing work share one job. Workflow-level concurrency includes the repository and PR number, so a new PR event retires an older queued execution before either consumes another job slot. The first step re-fetches the PR and gates every mutation on the exact current HEAD. This avoids both the former two-job admission dependency and the former HEAD-scoped group that allowed one stale queued coalescer per pushed commit to survive under the organization ceiling. The script re-fetches the live PR before classification. It lists all queued and in-progress repository runs rather than filtering only by workflow-run `head_sha`, because `pull_request_target` runs execute on the trusted base and their workflow head is not the PR head. Those runs are instead bound to the associated pull request's head identity. GitHub exposes repository identity in two different trusted REST shapes: the pull-request endpoint supplies a full repository object with `full_name`, while workflow-run `pull_requests[*].head.repo` and `base.repo` associations can contain only `id`, `name`, and canonical `https://api.github.com/repos/{owner}/{repo}` URL. `_repository_full_name()` therefore normalizes a valid full name directly or derives `owner/name` only from an exact HTTPS `api.github.com/repos/...` URL; malformed, query-bearing, foreign-host, non-HTTPS, or path-sentinel identities fail closed. This prevents a missing `full_name` field from turning every real workflow-run association into an empty repository identity while retaining a narrow authenticated GitHub boundary. diff --git a/docs/org-required-workflow-rollout.md b/docs/org-required-workflow-rollout.md index d56be9840c..88f6cc4deb 100644 --- a/docs/org-required-workflow-rollout.md +++ b/docs/org-required-workflow-rollout.md @@ -212,8 +212,7 @@ 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 the `org-queue-sweep` job keeps one daily missed-event recovery (`17 3 * * *`) for approved or stacked PRs. It re-runs the same guarded scheduler against repositories with open work, but it no longer inventories or cancels repository-wide Actions runs. Same-PR supersession belongs to native trigger-aware concurrency and the repository-local exact-head coalescer; removing the duplicate sweep owner also removes two paginated Actions queries per repository and the associated shared-installation rate-limit pressure. -- 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. +- Recovery posture: native PR and review events own normal progress, GitHub auto-merge owns required-check completion, and each repository keeps one daily `scan-pr-queue` recovery. The central organization-wide polling job was removed because each invocation occupied a runner, walked every repository, and amplified the same Actions and API pressure it was intended to repair. Same-PR supersession remains with trigger-aware concurrency and the repository-local exact-head coalescer. ## Second-reviewer (Noema) posture diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index d1967deb04..21ee42e24c 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2850,13 +2850,13 @@ product/operational decision this record surfaces rather than makes. **Status:** Investigated with a 9-agent workflow (4 independent file audits + 1 direct-evidence pull against the item's own cited example + 4 adversarial re-verification passes) plus a 4-agent follow-up (2 investigate + 2 adversarial verify) triggered by Devin Review findings, per `docs/doctoring/item13-stale-head-cancellation-audit-20260903.md`. Item 13 asks that Strix/OpenCode Review/Noema reliably cancel a PR's previous-head run when a new push supersedes it, citing `ContextualWisdomLab/naruon#1528` (run `33581213829`) as evidence of a gap. -**Verdict: the hypothesis is refuted for the item's own cited evidence, but `noema-review.yml` has a separate, confirmed, unfixed concurrency bug.** `strix.yml`, `opencode-review.yml`, and `pr-review-merge-scheduler.yml` already reliably retire a stale prior-head run on a new push — via correctly SHA-scoped native `concurrency:` groups where that's the right tool (`opencode-review.yml`, fixed after a real prior incident, `#1568`), and purpose-built same-file jobs that call the GitHub Actions API directly to find and cancel stale-head runs by exact `head_sha` match where native concurrency alone can't reach (`strix.yml`'s `cancel-superseded-pr-runs`, `pr-review-merge-scheduler.yml`'s hourly `org-queue-sweep`). `noema-review.yml` does not: its concurrency group has no head-SHA component, so if GitHub ever processes an older push's `synchronize` event after a newer one's (GitHub does not guarantee delivery order), native `cancel-in-progress` cancels the newer, valid, current-head run immediately — before the older run's own stale-trigger check ever executes, and nothing in the file can prevent this since GitHub evaluates `concurrency:` before any job step runs. Confirmed via two independent adversarial re-verification passes, neither of which found a refutation; corroborated by `strix.yml` and `opencode-review.yml` both deliberately using different patterns specifically to avoid this exact hazard. Not fixed here — a live CI concurrency-scoping change deserves its own dedicated PR with a regression test, not a same-breath edit to documentation. See the doctoring record for the full mechanism and evidence. +**Implementation pending protected merge in #1878.** Live pushes to #1878 showed that most workflows retired the prior HEAD automatically, while Required Noema Review and Current Head Run Coalescer each left one prior-HEAD run queued because their effective admission groups did not supersede by stable repository-and-PR identity. #1878 moves Noema concurrency to workflow admission, removes the coalescer's HEAD component, and keeps exact live-HEAD revalidation inside each trusted job before mutation. The same PR removes `org-queue-sweep`; stale-head retirement therefore has one owner at workflow admission instead of depending on an organization-wide runner and repository walk. The older out-of-order-event concern remains bounded by the mandatory live-HEAD gate: a stale event may replace a queued attempt, but it cannot publish review or cancellation evidence after its event HEAD stops matching the live PR. **The cited evidence shows a different, real problem instead: pure queue starvation, not a cancellation gap.** `ContextualWisdomLab/naruon#1528`'s full 17-run history (pulled live) shows every run sharing one unchanged head SHA — no multi-SHA race ever occurred. This corroborates `docs/doctoring/actions-plan-concurrency-ceiling-20260903.md`'s plan-level-ceiling finding with a concrete, individually-named example rather than aggregate counts — the fix is capacity (a plan decision or added runner capacity), not a workflow-config bug. **Correction (2026-09-04, evidence audit):** the specific "cited Strix run sat 23h22m queued before it even started running" claim above is wrong, disproven by direct re-verification. Both attempts of the cited Strix job (`33581213829`) show `created_at == started_at` — attempt 1 (2026-09-02T01:54:46Z→01:56:44Z, 2 min) and attempt 2 (2026-09-03T01:17:10Z→01:31:18Z, 14 min) both started **immediately** and were **cancelled mid-run**, not after a long queue wait. This pattern (prompt start, cancel during execution) is the opposite of queue starvation and is consistent with `strix.yml`'s own `cancel-superseded-pr-runs` mechanism (already documented above as working correctly) firing on this run — though the exact trigger for canceling a run against an unchanged head SHA was not further traced here. The paired OpenCode Review run for the same commit (`33581213805`) tells a different, worse story than "still queued 24+ hours later with no job started": its 5 sequential dependent jobs each queued for hours — `required-workflow-bootstrap` ~7h57m, `coverage-source-tree` ~9h40m, `coverage-evidence` ~13h1m, `opencode-review` ~12h13m — before `opencode-review` finally started 2026-09-03T20:46:49Z, ran for ~6 hours, and was itself cancelled 2026-09-04T02:47:05Z, roughly two full days after the original push. **Net effect on this entry's conclusion: unchanged, if anything understated.** The specific "23h22m" number attached to the wrong run doesn't survive scrutiny, but the underlying severe-queue-congestion finding this entry uses it to support is corroborated more strongly by the OpenCode Review run's real multi-stage delays than the original single figure conveyed. Found via a user-initiated adversarial evidence audit of 6 cited CI runs (5 of 6 confirmed accurate; this was the one exception). -**Not acted on further, deliberately, except for the confirmed `noema-review.yml` bug which is deferred to its own PR.** No fix was applied to item 13's own hypothesis or the (also-refuted) `strix.yml` paths-ignore claim, because no fixable bug was found there — forcing one would have meant inventing a problem the evidence does not support. The `noema-review.yml` concurrency bug is real and confirmed, but a live security-critical CI concurrency-scoping change was deliberately not bundled into this documentation PR; the standing chicken-and-egg bypass-merge authorization remains available for whichever PR carries that fix, once it exists. A peer session's lead on `naruon`'s `pr-governance.yml` (six runs on PR #1528's one unchanged SHA) was investigated further by fetching and reading the workflow and its gate script in full: a `check_run`-triggered job-slot-waste claim was corrected (the job's own `if:` restricts that path to CodeRabbit checks only — GitHub Actions requests no runner for a skipped job), and a proposed same-head debounce fix was found to be unsafe rather than implemented — `scripts/ci/pr_governance_gate.sh` evaluates live required-check/review-thread/CodeRabbit state on every run, not a pure function of head SHA, so skipping re-evaluation whenever the SHA is unchanged would leave the gate reporting a stale blocker list after a check finishes or a review lands. See `docs/doctoring/item13-stale-head-cancellation-audit-20260903.md` for the full trace; recorded as still open, not fixed. +**Current status:** implementation exists on #1878 but is not complete until exact-head required checks, independent review, protected merge, and post-merge workflow evidence succeed. No fix was applied to the refuted `strix.yml` paths-ignore claim. A peer session's lead on `naruon`'s `pr-governance.yml` (six runs on PR #1528's one unchanged SHA) was investigated further by fetching and reading the workflow and its gate script in full: a `check_run`-triggered job-slot-waste claim was corrected (the job's own `if:` restricts that path to CodeRabbit checks only — GitHub Actions requests no runner for a skipped job), and a proposed same-head debounce fix was found to be unsafe rather than implemented — `scripts/ci/pr_governance_gate.sh` evaluates live required-check/review-thread/CodeRabbit state on every run, not a pure function of head SHA, so skipping re-evaluation whenever the SHA is unchanged would leave the gate reporting a stale blocker list after a check finishes or a review lands. See `docs/doctoring/item13-stale-head-cancellation-audit-20260903.md` for the full trace. ## `codeql-pr.yml` required-workflow hard limit closed org-wide — 2026-09-03 diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 26a506ed22..e6b0ab11a0 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -1073,7 +1073,6 @@ assert_opencode_review_uses_codegraph_and_contextual_orchestrator() { assert_file_contains "$merge_scheduler_workflow" 'REVIEW_HEAD_SHA: ${{ github.event.review.commit_id }}' "review-event scheduler binds follow-up to the reviewed commit" assert_file_contains "$merge_scheduler_workflow" "live pull request snapshot could not be read" "review-event scheduler logs target snapshot lookup failures" assert_file_contains "$merge_scheduler_workflow" 'repos/${GITHUB_REPOSITORY}/commits/${REVIEW_HEAD_SHA}/check-runs?per_page=100' "review-event scheduler reads exact-head OpenCode completion evidence" - assert_file_contains "$merge_scheduler_workflow" "The scheduled organization sweep remains authoritative." "review-event scheduler logs its fallback when direct follow-up cannot proceed" assert_file_contains "$workflow_file" 'build_coverage_evidence_check_failure_body()' "opencode approval can describe a coverage-evidence blocker" assert_file_contains "$workflow_file" 'request_changes_for_coverage_evidence_failure' "opencode approval publishes REQUEST_CHANGES when coverage-evidence did not pass" assert_file_contains "$workflow_file" 'update_review_overview "COVERAGE_BLOCKED"' "opencode approval records coverage-evidence blocker states as COVERAGE_BLOCKED after COMMENT fallback" @@ -1562,16 +1561,12 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { assert_file_contains "$workflow_file" 'auto_merge_enabled' "scheduler rechecks already stale PRs as soon as native auto-merge is enabled" assert_file_not_contains "$workflow_file" 'workflow_run:' "required-check completion relies on GitHub auto-merge without spawning scheduler runs" assert_file_contains "$workflow_file" 'cron: "47 3 * * *"' "scheduler keeps one daily central missed-event recovery" - assert_file_contains "$workflow_file" 'cron: "17 3 * * *"' "scheduler keeps one daily organization missed-event recovery" + assert_file_not_contains "$workflow_file" "org-queue-sweep" "scheduler does not consume a runner on organization-wide polling" assert_file_not_contains "$workflow_file" "github.event.pull_request.number == 240" "scheduler must not hard-code repository-specific PR bypasses" assert_file_contains "$workflow_file" "github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number)" "scheduler scopes pull_request_target concurrency to the active PR" - assert_file_contains "$workflow_file" "github.event_name == 'schedule' && format('schedule-{0}', github.event.schedule)" "scheduler isolates the hourly organization sweep from the separate hourly repository-local scan" + assert_file_contains "$workflow_file" "github.event_name == 'schedule' && format('schedule-{0}', github.event.schedule)" "scheduler isolates repository-local recovery from PR runs" assert_file_contains "$workflow_file" "github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository != '' && github.event.client_payload.pr_number != ''" "scheduler scopes targeted manual queue scans to the requested PR" assert_file_contains "$workflow_file" "cancel-in-progress: \${{ github.event_name == 'pull_request_target' || github.event_name == 'pull_request_review' || github.event_name == 'repository_dispatch' }}" "scheduler cancels stale PR/review/manual queue scans instead of accumulating merge/update attempts" - assert_file_contains "$workflow_file" "timeout-minutes: 60" "organization sweep has enough headroom to finish the complete repository walk" - assert_file_contains "$workflow_file" "ORG_SWEEP_TRIGGER_REVIEWS: \${{ github.event_name == 'schedule' ||" "scheduled organization sweeps retry missing current-head OpenCode reviews" - assert_file_contains "$workflow_file" "ORG_SWEEP_ENABLE_AUTO_MERGE: \${{ github.event_name == 'schedule' ||" "scheduled organization sweeps merge approved current heads" - assert_file_contains "$workflow_file" "ORG_SWEEP_UPDATE_BRANCHES: \${{ github.event_name == 'schedule' ||" "scheduled organization sweeps refresh eligible stale branches" assert_file_not_contains "$workflow_file" 'github.event.workflow_run' "scheduler does not poll required-check completion through follow-up workflow runs" assert_file_contains "$workflow_file" "github.event.client_payload.trigger_reviews != false" "scheduler enables review dispatch by default for default-branch dispatch events" assert_file_contains "$workflow_file" "github.event_name == 'schedule' || github.event_name == 'push'" "scheduler can dispatch a bounded OpenCode review from native or recovery events" @@ -1585,7 +1580,6 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { 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" diff --git a/tests/test_actions_queue_saturation_scheduler_cadence.py b/tests/test_actions_queue_saturation_scheduler_cadence.py index 482ed69a2d..670f557c79 100644 --- a/tests/test_actions_queue_saturation_scheduler_cadence.py +++ b/tests/test_actions_queue_saturation_scheduler_cadence.py @@ -7,24 +7,12 @@ WORKFLOW = ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" -def test_org_queue_sweep_is_explicit_recovery_not_scheduled_polling() -> None: - """Native events own normal progress; the expensive org sweep is manual-only.""" +def test_org_queue_sweep_is_removed() -> None: + """Native events own progress without an organization-wide polling job.""" workflow = WORKFLOW.read_text(encoding="utf-8") - assert '- cron: "17 3 * * *"' not in workflow - assert "github.event.client_payload.org_sweep == true" in workflow - assert '- cron: "0 * * * *"' not in workflow - assert '*/15 * * * *' not in workflow - - -def test_org_queue_sweep_wall_clock_fallback_matches_manual_recovery() -> None: - """An explicit sweep still rotates fairly when requested.""" - workflow = WORKFLOW.read_text(encoding="utf-8") - 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 - assert "pending */15 sweep" not in workflow + assert " org-queue-sweep:" not in workflow + assert "github.event.client_payload.org_sweep != true" in workflow + assert "ORG_SWEEP" not in workflow def test_repository_scheduler_keeps_event_driven_wakes() -> None: diff --git a/tests/test_agent_review_runtime_quality_consolidation.py b/tests/test_agent_review_runtime_quality_consolidation.py index d69b5b2974..dfa6342c0d 100644 --- a/tests/test_agent_review_runtime_quality_consolidation.py +++ b/tests/test_agent_review_runtime_quality_consolidation.py @@ -98,7 +98,6 @@ def test_consolidated_workflow_preserves_all_contract_suites() -> None: "tests/test_strix_workflow_dependency_hashes.py", "tests/test_strix_quality_timeout_fixture_budget.py", "scripts/ci/test_strix_quick_gate.sh", - "tests/test_org_sweep_queue_hygiene_owner.py", "scripts/ci/pr_review_conflict_scope.py", "scripts/ci/pr_review_autofix_context.py", "scripts/ci/zdr_policy.py", diff --git a/tests/test_current_head_coalescer_self_cancellation.py b/tests/test_current_head_coalescer_self_cancellation.py index 04aa1356d4..cfe4ed2d3e 100644 --- a/tests/test_current_head_coalescer_self_cancellation.py +++ b/tests/test_current_head_coalescer_self_cancellation.py @@ -8,7 +8,7 @@ def test_current_head_coalescer_admits_live_head_before_native_concurrency() -> None: - """Head-scoped concurrency isolates stale events without a second job.""" + """PR-scoped concurrency retires stale queued heads before job admission.""" workflow_text = WORKFLOW_PATH.read_text(encoding="utf-8") coalescer = workflow_text.split("\n coalesce:\n", 1)[1] concurrency_block = workflow_text.split("\nconcurrency:\n", 1)[1].split( @@ -23,6 +23,7 @@ def test_current_head_coalescer_admits_live_head_before_native_concurrency() -> assert "admit-current-head:" not in workflow_text assert "id: live-head" in coalescer assert coalescer.count("if: steps.live-head.outputs.admitted == 'true'") == 2 - assert "github.event.pull_request.head.sha" in concurrency_block + assert "github.event.pull_request.head.sha" not in concurrency_block + assert "github.event.pull_request.number" in concurrency_block assert "cancel-in-progress: true" in active_lines assert "queue: max" not in workflow_text diff --git a/tests/test_merge_scheduler_runner_image_contract.py b/tests/test_merge_scheduler_runner_image_contract.py index 3334ac5b87..be3812f050 100644 --- a/tests/test_merge_scheduler_runner_image_contract.py +++ b/tests/test_merge_scheduler_runner_image_contract.py @@ -26,10 +26,7 @@ class MergeSchedulerRunnerImageContract(unittest.TestCase): def test_queue_draining_jobs_use_explicit_supported_image(self) -> None: """Require the scheduler control plane to use explicit Ubuntu 24.04.""" workflow = WORKFLOW.read_text(encoding='utf-8') - for job_name in ( - 'scan-pr-queue', - 'org-queue-sweep', - ): + for job_name in ('scan-pr-queue',): block = job_block(workflow, job_name) self.assertIn('runs-on: ubuntu-24.04', block, job_name) self.assertNotIn('runs-on: ubuntu-latest', block, job_name) diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index e225b6139b..72a8b44e56 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -2346,7 +2346,6 @@ def test_merge_scheduler_uses_escalating_mutation_credentials(): 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 "pull_request_review:" in workflow assert "types: [submitted, dismissed]" in workflow diff --git a/tests/test_org_sweep_queue_hygiene_owner.py b/tests/test_org_sweep_queue_hygiene_owner.py deleted file mode 100644 index 7ca06eca5b..0000000000 --- a/tests/test_org_sweep_queue_hygiene_owner.py +++ /dev/null @@ -1,48 +0,0 @@ -"""Pin the single-writer boundary for GitHub Actions queue hygiene.""" - -from pathlib import Path - - -REPO_ROOT = Path(__file__).resolve().parents[1] - - -def _workflow(name: str) -> str: - """Return one trusted central workflow as UTF-8 text.""" - return (REPO_ROOT / ".github" / "workflows" / name).read_text(encoding="utf-8") - - -def test_org_sweep_does_not_inventory_repository_wide_actions_runs() -> None: - """Keep PR-head run coalescing out of the cross-repository organization sweep.""" - scheduler = _workflow("pr-review-merge-scheduler.yml") - org_sweep = scheduler.split(" org-queue-sweep:", 1)[1] - - assert "ORG_SWEEP_STALE_QUEUE_HOURS" not in org_sweep - assert "/actions/runs?status=${active_status}&per_page=100" not in org_sweep - assert "for active_status in queued in_progress" not in org_sweep - assert "revalidate_queue_cancellation.sh" not in org_sweep - - -def test_current_head_coalescer_owns_repo_local_exact_pr_scope() -> None: - """Require target-repository credentials and exact live PR-head scope.""" - workflow = _workflow("current-head-run-coalescer.yml") - helper = ( - REPO_ROOT / "scripts" / "ci" / "current_head_run_coalescer.py" - ).read_text(encoding="utf-8") - - assert "GH_TOKEN: ${{ github.token }}" in workflow - concurrency = workflow.split("\nconcurrency:\n", 1)[1].split("\njobs:\n", 1)[0] - assert "current-head-run-coalescer-${{ github.repository }}-${{" in concurrency - assert "github.event.pull_request.number }}" in concurrency - assert "github.event.pull_request.head.sha }}" in concurrency - assert ( - "EXPECTED_HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }}" - in workflow - ) - assert "EXPECTED_HEAD_REF: ${{ github.event.pull_request.head.ref }}" in workflow - assert "EXPECTED_HEAD: ${{ github.event.pull_request.head.sha }}" in workflow - assert "live_pr = _fetch_pr(repo, number)" in helper - assert 'live_pr.get("state") != "open"' in helper - assert ( - 'raise CoalescingRefused("pull request head moved before duplicate classification")' - in helper - ) diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 6c943ac6b4..803d43ab59 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -51,9 +51,9 @@ def test_scheduler_uses_bounded_run_state_without_cache_lock_claims() -> None: assert workflow.count( '--admission-state-path "${RUNNER_TEMP}/review-admission/state.json"' - ) == 2 - assert workflow.count("--admission-dispatch-budget") == 2 - assert workflow.count("--admission-sequence \"$GITHUB_RUN_ID\"") == 2 + ) == 1 + assert workflow.count("--admission-dispatch-budget") == 1 + assert workflow.count("--admission-sequence \"$GITHUB_RUN_ID\"") == 1 assert "actions/cache/restore" not in workflow assert "actions/cache/save" not in workflow assert "actions/upload-artifact" not in workflow @@ -92,10 +92,10 @@ def test_merge_scheduler_rejects_untrusted_stale_timeout_values() -> None: """Dispatch payloads must not smuggle shell syntax into scheduler arguments.""" workflow = workflow_text("pr-review-merge-scheduler.yml") - assert workflow.count("STALE_OPENCODE_MINUTES must contain only decimal digits") == 2 - assert workflow.count("STALE_OPENCODE_MINUTES must be between 1 and 1440") == 4 - assert workflow.count("stale_opencode_minutes=$((10#$STALE_OPENCODE_MINUTES))") == 2 - assert workflow.count('STALE_OPENCODE_MINUTES="$stale_opencode_minutes"') == 2 + assert workflow.count("STALE_OPENCODE_MINUTES must contain only decimal digits") == 1 + assert workflow.count("STALE_OPENCODE_MINUTES must be between 1 and 1440") == 2 + assert workflow.count("stale_opencode_minutes=$((10#$STALE_OPENCODE_MINUTES))") == 1 + assert workflow.count('STALE_OPENCODE_MINUTES="$stale_opencode_minutes"') == 1 def test_merge_scheduler_uses_native_auto_merge_after_required_checks() -> None: @@ -105,7 +105,7 @@ def test_merge_scheduler_uses_native_auto_merge_after_required_checks() -> None: "permissions:", 1 )[0] - assert "format('org-sweep-{0}', github.repository)" in concurrency_contract + assert "org-sweep" not in concurrency_contract assert "format('repo-dispatch-{0}', github.repository)" in concurrency_contract assert "workflow_run:" not in workflow.split("workflow_call:", 1)[0] assert "github.event.workflow_run" not in concurrency_contract @@ -119,19 +119,12 @@ def test_merge_scheduler_uses_native_auto_merge_after_required_checks() -> None: def test_merge_scheduler_provides_same_repository_dispatch_credential() -> None: """Guard the runner-token dispatch credential for central review workflows. - The OpenCode app installation has no Actions permission and no - PR_REVIEW_MERGE_TOKEN / OPENCODE_APPROVE_TOKEN PAT is configured, so before - this credential existed the org sweep deadlocked every PR needing current-head - review evidence with "no cross-repository repository-dispatch credential". The - scheduler and the sweep both run inside ContextualWisdomLab/.github — the same - repository the required workflows are dispatched on — so the runner's own - github.token (actions: write) must be passed through SCHEDULER_DISPATCH_TOKEN - in BOTH jobs; the scheduler only uses it when GITHUB_REPOSITORY equals the - dispatch repository. + The scheduler runs inside the same repository as the central required + workflows, so its repository-scoped token is the single dispatch credential. """ workflow = workflow_text("pr-review-merge-scheduler.yml") - assert workflow.count("SCHEDULER_DISPATCH_TOKEN: ${{ github.token }}") == 2 + assert workflow.count("SCHEDULER_DISPATCH_TOKEN: ${{ github.token }}") == 1 def test_targeted_scheduler_dispatch_is_allowlisted_and_exact_pr_scoped() -> None: @@ -268,8 +261,7 @@ def test_required_pull_request_workflows_cancel_superseded_runs() -> None: assert "github.event.pull_request.base.repo.full_name" in concurrency_contract assert "github.repository" in concurrency_contract assert "github.event.pull_request.number" in workflow - if filename != "noema-review.yml": - assert re.search(r"(?m)^concurrency:", workflow) + assert re.search(r"(?m)^concurrency:", workflow) assert "cancel-in-progress: true" in concurrency_contract if filename == "security-scan.yml": assert ( @@ -280,8 +272,7 @@ def test_required_pull_request_workflows_cancel_superseded_runs() -> None: assert "required-opencode-review-${{" in concurrency_contract assert "outputs.admitted == 'true'" in workflow elif filename == "noema-review.yml": - assert not re.search(r"(?m)^concurrency:", workflow) - assert re.search(r"(?m)^ concurrency:", workflow) + assert not re.search(r"(?m)^ concurrency:", workflow) assert "github.event.workflow_run" not in concurrency_contract assert "required-noema-review-${{" in concurrency_contract assert "outputs.admitted == 'true'" in workflow @@ -717,8 +708,8 @@ def test_noema_triggers_preserve_standalone_pull_request_review() -> None: """Noema reviews PRs independently of the other review workflows.""" workflow = workflow_text("noema-review.yml") noema_job = workflow.split("\n noema-review:\n", 1)[1] - concurrency_contract = noema_job.split(" concurrency:\n", 1)[1].split( - " permissions:\n", 1 + concurrency_contract = workflow.split("\nconcurrency:\n", 1)[1].split( + "\npermissions:\n", 1 )[0] assert "workflow_run:" not in concurrency_contract @@ -730,10 +721,8 @@ def test_noema_triggers_preserve_standalone_pull_request_review() -> None: "cancel-in-progress:", 1 )[0] assert "cancel-in-progress: true" in concurrency_contract - assert not re.search(r"(?m)^concurrency:", workflow) - assert workflow.index(" admit-current-head:") < workflow.index( - " concurrency:", workflow.index(" noema-review:") - ) + assert re.search(r"(?m)^concurrency:", workflow) + assert not re.search(r"(?m)^ concurrency:", workflow) assert "needs.admit-current-head.outputs.admitted == 'true'" in noema_job assert '[ "${live_head_sha,,}" != "${EXPECTED_HEAD_SHA,,}" ]' in workflow @@ -1024,7 +1013,7 @@ def test_merge_scheduler_has_no_workflow_run_trigger() -> None: def test_review_events_can_dispatch_after_threads_are_resolved() -> None: """Let the scheduler dispatch OpenCode when a review event clears its last blocker.""" workflow = workflow_text("pr-review-merge-scheduler.yml") - scan_job = workflow.split(" scan-pr-queue:", 1)[1].split(" org-queue-sweep:", 1)[0] + scan_job = workflow.split(" scan-pr-queue:", 1)[1] assert "github.event_name == 'pull_request_review'" in scan_job.split( "TRIGGER_REVIEWS:", 1 @@ -1032,17 +1021,9 @@ def test_review_events_can_dispatch_after_threads_are_resolved() -> None: def test_scan_pr_queue_has_a_bounded_runtime() -> None: - """scan-pr-queue must not fall back to GitHub's 360-minute platform default. - - Without a job-level timeout-minutes, a stuck run (rate-limited GitHub API, - a hung gh invocation) can occupy a shared runner for up to six hours, - contributing to org-wide Actions capacity saturation. The bound must be - shorter than org-queue-sweep's timeout-minutes: 60, since scan-pr-queue - only scans this one repository's queue while org-queue-sweep walks every - target repository in the organization. - """ + """Keep one repository-local scan below GitHub's platform timeout.""" workflow = workflow_text("pr-review-merge-scheduler.yml") - scan_job = workflow.split(" scan-pr-queue:", 1)[1].split(" org-queue-sweep:", 1)[0] + scan_job = workflow.split(" scan-pr-queue:", 1)[1] match = re.search(r"^ timeout-minutes: (\d+)$", scan_job, flags=re.MULTILINE) assert match is not None, "scan-pr-queue must declare a job-level timeout-minutes" @@ -1051,560 +1032,6 @@ def test_scan_pr_queue_has_a_bounded_runtime() -> None: assert scan_timeout < 60 -def test_org_queue_sweep_is_explicit_bounded_recovery_only() -> None: - """Guard the explicit org-wide approved-PR fallback sweep contract. - - Target repositories only receive scheduler runs on PR events, so a PR that - becomes mergeable after its last event sits approved-but-unmerged forever. - 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. Native events - handle the normal path; only an explicit bounded dispatch may start the - expensive organization walk. - """ - workflow = workflow_text("pr-review-merge-scheduler.yml") - - assert "org-queue-sweep:" in workflow - assert '- cron: "17 3 * * *"' not in workflow - assert "github.repository == 'ContextualWisdomLab/.github'" in workflow - assert "github.event.client_payload.org_sweep == true" in workflow - org_sweep_header = workflow.split(" org-queue-sweep:", 1)[1].split( - " permissions:", 1 - )[0] - assert "timeout-minutes: 60" in org_sweep_header - # The single-repository scan must not double-run on explicit sweep dispatch. - assert "github.event.client_payload.org_sweep != true" in workflow - # The sweep must never silently no-op with the repository-scoped token. - assert ( - "Organization queue sweep has no cross-repository mutation credential." - in workflow - ) - 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 - assert "/pulls?state=open&per_page=1&base=" not in workflow - assert "No open PRs (including stacked or non-default-base PRs)" in workflow - # Every repository failure must leave a concrete logged reason. - assert "see the decision log above for the concrete per-PR reason" in workflow - # Queue cancellation belongs to native per-PR concurrency and the local - # exact-head coalescer, not this cross-repository recovery walk. - assert "ORG_SWEEP_STALE_QUEUE_HOURS" not in workflow - assert "/actions/runs?status=${active_status}&per_page=100" not in workflow - assert "revalidate_queue_cancellation.sh" not in workflow - # Organization sweep budgets must be consumed across the repository loop; - # resetting the configured limit for every target can flood Actions with - # long-running review dispatches. - assert '"$ORG_SWEEP_REVIEW_DISPATCH_LIMIT" =~ ^(-1|[0-9]+)$' in workflow - assert '"$ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT" =~ ^(-1|[0-9]+)$' in workflow - assert '"$ORG_SWEEP_BRANCH_UPDATE_LIMIT" =~ ^(-1|[0-9]+)$' in workflow - assert "org_review_dispatches_used=0" in workflow - assert "org_stacked_review_dispatches_used=0" in workflow - assert "org_branch_updates_used=0" in workflow - assert 'review_dispatch_limit=$((ORG_SWEEP_REVIEW_DISPATCH_LIMIT - org_review_dispatches_used))' in workflow - assert 'stacked_review_dispatch_limit=$((ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT - org_stacked_review_dispatches_used))' in workflow - assert 'branch_update_limit=$((ORG_SWEEP_BRANCH_UPDATE_LIMIT - org_branch_updates_used))' in workflow - assert '--review-dispatch-limit "$review_dispatch_limit"' in workflow - assert '--stacked-review-dispatch-limit "$stacked_review_dispatch_limit"' in workflow - assert '--branch-update-limit "$branch_update_limit"' in workflow - assert 'grep -Ec \'^PR #[0-9]+: (review_dispatch|security_dispatch):\'' in workflow - assert 'grep -Ec \'^PR #[0-9]+: review_dispatch: stacked PR onto\'' in workflow - assert 'grep -Ec \'^PR #[0-9]+: (update_branch|restamp_head):\'' in workflow - # The scheduler requires --project-flow; the sweep must derive and pass it - # per target repository (regression: the first sweep failed every repo with - # "--project-flow is required"). - assert "--project-flow" in workflow - assert 'main|master) project_flow="github-flow"' in workflow - assert 'develop) project_flow="git-flow"' in workflow - - -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 = 'rotation tick ${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 _extract_org_sweep_rotation_default_snippet(workflow: str) -> str: - """Return only the wall-clock-default/validation block for the rotation index, - without the surrounding `gh api` calls that would require network credentials.""" - - start_marker = " if [ -z \"${ORG_SWEEP_ROTATION_INDEX:-}\" ]; then\n" - end_marker = " exit 1\n fi\n\n repositories_json=" - start = workflow.index(start_marker) - end = workflow.index(end_marker, start) + len(" exit 1\n fi\n") - return textwrap.dedent(workflow[start:end]) - - -def _fake_gh_script(*, get_ok: bool, get_value: str, patch_ok: bool, post_ok: bool) -> str: - """A stand-in `gh` executable simulating the repository-variable API. - - ``get_ok`` controls whether `gh api .../variables/NAME --jq .value` - exits zero at all -- a real "does the variable exist and is it - readable" outcome, kept distinct from what value it prints on success - (``get_value``), so tests can simulate a *failed* read (transient error - or a genuinely missing variable) separately from a *successful* read - of an empty/malformed value. ``patch_ok``/``post_ok`` control whether - the corresponding mutation exits zero, so tests can force the - PATCH-then-POST-create fallback or the full-failure wall-clock - fallback without a real GitHub API call. - """ - get_exit = "0" if get_ok else "1" - patch_exit = "0" if patch_ok else "1" - post_exit = "0" if post_ok else "1" - return textwrap.dedent( - f"""\ - #!/usr/bin/env bash - set -euo pipefail - if [ "$1" != "api" ]; then - echo "unsupported fake gh invocation: $*" >&2 - exit 2 - fi - shift - if [[ "$1" == *"/variables/"* ]] && [[ "$*" == *"-X PATCH"* || "$*" == *"PATCH"* ]]; then - exit {patch_exit} - fi - if [[ "$1" == "repos/"*"/actions/variables" ]]; then - exit {post_exit} - fi - if [[ "$1" == *"/variables/"* ]]; then - if [ "{get_exit}" = "0" ]; then - printf '%s' "{get_value}" - fi - exit {get_exit} - fi - echo "unsupported fake gh api path: $1" >&2 - exit 2 - """ - ) - - -def _run_rotation_default_snippet( - snippet: str, - tmp_path: Path, - *, - get_ok: bool = True, - get_value: str, - patch_ok: bool, - post_ok: bool, -) -> subprocess.CompletedProcess[str]: - """Execute the extracted default/validation block with a fake `gh` on PATH.""" - - fake_gh = tmp_path / "gh" - fake_gh.write_text( - _fake_gh_script(get_ok=get_ok, get_value=get_value, patch_ok=patch_ok, post_ok=post_ok), - encoding="utf-8", - ) - fake_gh.chmod(0o755) - script = snippet + '\nprintf "%s\\n" "$ORG_SWEEP_ROTATION_INDEX"\n' - env = dict(os.environ) - env.pop("ORG_SWEEP_ROTATION_INDEX", None) - env["GITHUB_REPOSITORY"] = "ContextualWisdomLab/.github" - env["PATH"] = f"{tmp_path}{os.pathsep}{env.get('PATH', '')}" - return subprocess.run( - ["bash", "-euo", "pipefail", "-c", script], env=env, capture_output=True, text=True - ) - - -def test_org_queue_sweep_rotation_index_uses_persistent_counter_when_available( - tmp_path: Path, -) -> None: - """The primary source increments a persistent counter by exactly one per - actual sweep execution — immune to how much wall-clock time a prior - slow (up to 60-minute, non-cancelling) run consumed, which a wall-clock - tick alone cannot guarantee (CodeRabbit review finding on #1223).""" - - workflow = workflow_text("pr-review-merge-scheduler.yml") - snippet = _extract_org_sweep_rotation_default_snippet(workflow) - - result = _run_rotation_default_snippet( - snippet, tmp_path, get_value="7", patch_ok=True, post_ok=True - ) - assert result.returncode == 0, result.stderr - assert result.stdout.strip() == "8" # incremented by exactly one - - -def test_org_queue_sweep_rotation_index_counter_increment_forces_base_10( - tmp_path: Path, -) -> None: - """A manually-seeded leading-zero value ("08") must not be parsed as - octal, where it would error under set -e (Devin review finding on - #1223) — unprefixed bash arithmetic treats a leading zero as an octal - literal, and "08"/"09" are not valid octal digits.""" - - workflow = workflow_text("pr-review-merge-scheduler.yml") - snippet = _extract_org_sweep_rotation_default_snippet(workflow) - - result = _run_rotation_default_snippet( - snippet, tmp_path, get_value="08", patch_ok=True, post_ok=True - ) - assert result.returncode == 0, result.stderr - assert result.stdout.strip() == "9" - - -def test_org_queue_sweep_rotation_index_creates_counter_on_first_run(tmp_path: Path) -> None: - """A failed read (variable does not exist yet) falls back to creating it.""" - - workflow = workflow_text("pr-review-merge-scheduler.yml") - snippet = _extract_org_sweep_rotation_default_snippet(workflow) - - result = _run_rotation_default_snippet( - snippet, tmp_path, get_ok=False, get_value="", patch_ok=False, post_ok=True - ) - assert result.returncode == 0, result.stderr - assert result.stdout.strip() == "1" - - -def test_org_queue_sweep_rotation_index_falls_back_to_wall_clock(tmp_path: Path) -> None: - """If the persistent counter is entirely unavailable (both the read and - the create-on-first-run POST fail), degrade to a wall-clock tick rather - than failing the whole sweep over a fairness mechanism.""" - - workflow = workflow_text("pr-review-merge-scheduler.yml") - snippet = _extract_org_sweep_rotation_default_snippet(workflow) - - result = _run_rotation_default_snippet( - snippet, tmp_path, get_ok=False, get_value="", patch_ok=False, post_ok=False - ) - 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()) // 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 - - -def test_org_queue_sweep_rotation_index_transient_read_failure_does_not_reset_counter( - tmp_path: Path, -) -> None: - """A *failed* read must never be treated as "the counter is 0 and safe to - PATCH": that would silently reset an already-accumulated counter value - back down to 1, restarting the rotation sequence instead of degrading to - the wall-clock fallback (Devin review finding on #1223). Simulated here - as: the read fails, and the create-on-first-run POST also fails (as it - should when the variable genuinely already exists and this run simply - could not see it) -- landing on the wall-clock fallback rather than a - PATCH that would have clobbered the real value.""" - - workflow = workflow_text("pr-review-merge-scheduler.yml") - snippet = _extract_org_sweep_rotation_default_snippet(workflow) - - result = _run_rotation_default_snippet( - snippet, tmp_path, get_ok=False, get_value="", patch_ok=True, post_ok=False - ) - assert result.returncode == 0, result.stderr - stdout_lines = result.stdout.strip().splitlines() - computed_tick = int(stdout_lines[-1]) - 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. - assert stdout_lines[-1] != "1" - - -def test_org_queue_sweep_rotation_index_successful_read_but_failed_patch_falls_back( - tmp_path: Path, -) -> None: - """A successful read of an existing value, followed by a failed PATCH, - must fall back to the wall-clock tick and log the value that could not - be written -- not silently drop the accumulated counter.""" - - workflow = workflow_text("pr-review-merge-scheduler.yml") - snippet = _extract_org_sweep_rotation_default_snippet(workflow) - - result = _run_rotation_default_snippet( - snippet, tmp_path, get_ok=True, get_value="41", patch_ok=False, post_ok=False - ) - assert result.returncode == 0, result.stderr - stdout_lines = result.stdout.strip().splitlines() - computed_tick = int(stdout_lines[-1]) - 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 - - -def test_org_queue_sweep_rotation_index_override_is_preserved() -> None: - """An explicitly injected value (as tests do) is never overwritten.""" - - workflow = workflow_text("pr-review-merge-scheduler.yml") - snippet = _extract_org_sweep_rotation_default_snippet(workflow) - script = snippet + '\nprintf "%s\\n" "$ORG_SWEEP_ROTATION_INDEX"\n' - - result = subprocess.run( - ["bash", "-euo", "pipefail", "-c", script], - env={**os.environ, "ORG_SWEEP_ROTATION_INDEX": "42"}, - capture_output=True, - text=True, - ) - assert result.returncode == 0, result.stderr - assert result.stdout.strip() == "42" - - -def test_org_queue_sweep_rotation_index_rejects_malformed_override() -> None: - """A malformed override still fails closed rather than reaching arithmetic.""" - - workflow = workflow_text("pr-review-merge-scheduler.yml") - snippet = _extract_org_sweep_rotation_default_snippet(workflow) - script = snippet + '\nprintf "%s\\n" "$ORG_SWEEP_ROTATION_INDEX"\n' - - result = subprocess.run( - ["bash", "-euo", "pipefail", "-c", script], - env={**os.environ, "ORG_SWEEP_ROTATION_INDEX": "not-a-number"}, - capture_output=True, - text=True, - ) - assert result.returncode != 0 - assert "ORG_SWEEP_ROTATION_INDEX must be a non-negative integer" 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 "ContextualWisdomLab/.github#1219" in workflow - assert ( - 'ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 86400 ))' - ) 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 - # `github.run_number` increments on every trigger of this workflow, not - # only the sweep schedule, so it cannot give the per-sweep-tick rotation - # guarantee the fix is meant to provide (ContextualWisdomLab/.github#1220 - # review finding). The env-block default must not reintroduce it. - 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 || '0'" in workflow - assert "vars.ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT || '1'" in workflow - assert "Stacked PRs have no" 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") - - assert ( - "ORG_SWEEP_REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || " - "vars.ORG_SWEEP_REVIEW_DISPATCH_LIMIT || '0' }}" - ) in workflow - assert ( - "ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.stacked_review_dispatch_limit || " - "vars.ORG_SWEEP_STACKED_REVIEW_DISPATCH_LIMIT || '1' }}" - ) in workflow - assert ( - "STALE_OPENCODE_MINUTES: ${{ github.event.client_payload.stale_opencode_minutes || inputs.stale_opencode_minutes || " - "vars.STALE_OPENCODE_MINUTES || '90' }}" - ) in workflow - assert ( - "ORG_SWEEP_MAX_PRS: ${{ github.event.client_payload.max_prs || inputs.max_prs || vars.ORG_SWEEP_MAX_PRS || '1000' }}" - ) in workflow - assert ( - "ORG_SWEEP_TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.trigger_reviews != false || inputs.trigger_reviews == true }}" - in workflow - ) - assert ( - "ORG_SWEEP_ENABLE_AUTO_MERGE: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.enable_auto_merge != false || inputs.enable_auto_merge == true }}" - ) in workflow - assert ( - "ORG_SWEEP_MERGE_MODE: ${{ github.event.client_payload.merge_mode || inputs.merge_mode || 'direct_or_auto' }}" - in workflow - ) - assert ( - "ORG_SWEEP_UPDATE_BRANCHES: ${{ github.event_name == 'schedule' || github.event_name == 'repository_dispatch' && github.event.client_payload.update_branches != false || inputs.update_branches == true }}" - in workflow - ) - assert 'if [ "$ORG_SWEEP_TRIGGER_REVIEWS" = "true" ]; then' in workflow - assert 'if [ "$ORG_SWEEP_ENABLE_AUTO_MERGE" = "true" ]; then' in workflow - assert '--merge-mode "$ORG_SWEEP_MERGE_MODE"' in workflow - assert 'if [ "$ORG_SWEEP_UPDATE_BRANCHES" = "true" ]; then' in workflow - - -def test_stacked_budget_is_not_declared_as_an_unused_workflow_call_input() -> None: - """Keep the stacked-only organization setting out of the reusable API.""" - workflow = workflow_text("pr-review-merge-scheduler.yml") - workflow_call = workflow.split(" workflow_call:", 1)[1].split( - " schedule:", 1 - )[0] - - assert "stacked_review_dispatch_limit" not in workflow_call - assert "inputs.stacked_review_dispatch_limit" not in workflow - - -def test_org_queue_sweep_treats_inaccessible_repositories_as_non_fatal() -> None: - """A repository the sweep credential cannot read must not fail the sweep. - - When the OpenCode app is not installed on a sibling repository (or the - PR_REVIEW_MERGE_TOKEN does not cover it), every read returns HTTP 403 - "Resource not accessible by integration". That is an access-grant fact the - 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 - ``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 - increments ``failures`` and fails the job, and if MORE than - ``ORG_SWEEP_MAX_UNAVAILABLE`` repositories become unreachable at once (a - credential-scope regression, not a few un-enrolled repos) the job fails. - """ - workflow = workflow_text("pr-review-merge-scheduler.yml") - - # The 403 signal is classified as a skipped, non-fatal "unavailable" repo. - assert "ORG_SWEEP_MAX_UNAVAILABLE" in workflow - assert 'grep -qF "Resource not accessible by integration"' in workflow - assert "unavailable=$((unavailable + 1))" in workflow - assert 'unavailable_repos+=("$repo_full_name")' in workflow - assert "the sweep credential lacks access (HTTP 403" in workflow - # A non-403 failure must still be a hard failure (fail-closed preserved). - assert "failures=$((failures + 1))" in workflow - assert "see the decision log above for the concrete per-PR reason" in workflow - # Widespread inaccessibility is a credential regression and must fail loudly. - assert 'if [ "$unavailable" -gt "$ORG_SWEEP_MAX_UNAVAILABLE" ]; then' in workflow - assert "indicates a credential-scope regression" in workflow - # The ceiling must be validated as a non-negative integer BEFORE the numeric - # test, or a misconfigured non-integer would make "[ -gt ]" error inside an - # if condition (which set -e does not trap) and silently skip the guard. - assert '"$ORG_SWEEP_MAX_UNAVAILABLE" =~ ^[0-9]+$' in workflow - assert "ORG_SWEEP_MAX_UNAVAILABLE must be a non-negative integer" in workflow - - -def test_org_queue_sweep_treats_rate_limited_repositories_as_non_fatal() -> None: - """A shared installation-token rate-limit exhaustion must not fail the sweep. - - Installation 141441800's primary rate limit (5,000-12,500 requests/hour) - is shared by at least eight other central workflows that mint tokens for - the same GitHub App installation. When that bucket is exhausted, ``gh`` - fails with "API rate limit exceeded" — routine cross-workflow contention, - not a defect in the target repository — and self-heals on GitHub's own - hourly reset. Treating it as a hard failure previously turned one - exhausted bucket into a permanently red ``*/15 * * * *`` cron for as long - as the contention lasted (observed: repeated same-signature failures - spanning 15+ hours). That repository is now reported as a skipped, - non-fatal "deferred" repository instead, exactly like the existing - inaccessible-repository handling, and is retried on the next rotation. - - Unlike ``ORG_SWEEP_MAX_UNAVAILABLE``, there is deliberately no fail-closed - ceiling on the rate-limited count: one exhausted installation bucket is - shared by every remaining repository, so the sweep records the current - repository and stops the rotation instead of repeating the same bounded - retries and API calls for every later repository. - """ - workflow = workflow_text("pr-review-merge-scheduler.yml") - - # The rate-limit signal is classified as a skipped, non-fatal "deferred" repo. - assert 'grep -qiF "API rate limit exceeded"' in workflow - assert "rate_limited=$((rate_limited + 1))" in workflow - assert 'rate_limited_repos+=("$repo_full_name")' in workflow - assert "the shared GitHub App installation-token rate limit is exhausted" in workflow - assert "retried automatically" in workflow - # It must be checked as its own branch, distinct from both the existing - # 403 "unavailable" classification and the generic hard-failure branch — - # a rate-limited sweep must not also increment unavailable or failures. - assert ( - 'elif printf \'%s\' "$sweep_output" | grep -qiF "API rate limit exceeded"; then' - in workflow - ) - rate_limited_branch = workflow.split( - 'elif printf \'%s\' "$sweep_output" | grep -qiF "API rate limit exceeded"; then', - maxsplit=1, - )[1].split("\n else\n", maxsplit=1)[0] - assert 'rate_limited_repos+=("$repo_full_name")' in rate_limited_branch - assert 'echo "::endgroup::"' in rate_limited_branch - assert "break" in rate_limited_branch - assert rate_limited_branch.index('rate_limited_repos+=("$repo_full_name")') < ( - rate_limited_branch.index('echo "::endgroup::"') - ) < ( - rate_limited_branch.index("break") - ) - script = ( - "rate_limited=0\n" - "rate_limited_repos=()\n" - "visited_repos=()\n" - "for repo_full_name in ContextualWisdomLab/first ContextualWisdomLab/second; do\n" - " visited_repos+=(\"$repo_full_name\")\n" - + textwrap.indent(textwrap.dedent(rate_limited_branch).strip() + "\n", " ") - + "done\n" - + "printf 'RESULT|%s|%s|%s\\n' \"$rate_limited\" " - '"${rate_limited_repos[*]}" "${visited_repos[*]}"\n' - ) - result = subprocess.run( - ["bash", "-euo", "pipefail", "-c", script], - capture_output=True, - text=True, - ) - assert result.returncode == 0, result.stderr - assert result.stdout.splitlines()[-1] == ( - "RESULT|1|ContextualWisdomLab/first|ContextualWisdomLab/first" - ) - # A genuine (non-403, non-rate-limit) failure must still be a hard failure. - assert "failures=$((failures + 1))" in workflow - # No fail-closed ceiling on rate-limited repositories (see docstring): - # unlike ORG_SWEEP_MAX_UNAVAILABLE, no configured limit ever turns - # widespread rate-limiting into a hard "exit 1" job failure. - assert "ORG_SWEEP_MAX_RATE_LIMITED" not in workflow - - def test_fix_scheduler_cancels_superseded_cron_runs() -> None: """Cancel stale scheduled repair runs before they duplicate mutation work.""" workflow = workflow_text("pr-review-fix-scheduler.yml")