fix(scheduler): derive org-queue-sweep rotation tick from wall-clock time

.github/workflows/pr-review-merge-scheduler.yml

@@ -617,11 +617,17 @@ jobs:
       # order every tick (the org repos API response order), so the same early
       # repositories always exhaust the shared budget and every later repository
       # starves indefinitely even with zero-open-thread, all-green PRs
-      # (ContextualWisdomLab/.github#1219). `github.run_number` increments on
-      # every run of this workflow, so rotating the walk order by it spreads the
-      # same fixed total budget across repositories over successive ticks instead
-      # of raising it.
-      ORG_SWEEP_ROTATION_INDEX: ${{ github.run_number }}
+      # (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, workflow_run), 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 900s 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
@@ -826,8 +832,95 @@ jobs:
             echo "::error::ORG_SWEEP_BRANCH_UPDATE_LIMIT must be -1 or a non-negative integer; got '${ORG_SWEEP_BRANCH_UPDATE_LIMIT}'. Fix the ORG_SWEEP_BRANCH_UPDATE_LIMIT repository variable."
             exit 1
           fi
+          # 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 900s) is *not* sufficient on its own:
+          # this job is single-flight/non-cancelling with up to a 60-minute
+          # timeout, so a delayed or backlogged execution can let more than
+          # one 900s window elapse between two real sweep runs, and if that
+          # 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) / 900 ))
+              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) / 900 ))
+            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}'. This is derived from github.run_number and should never be malformed."
+            echo "::error::ORG_SWEEP_ROTATION_INDEX must be a non-negative integer; got '${ORG_SWEEP_ROTATION_INDEX}'."
             exit 1
           fi
 
@@ -845,10 +938,12 @@ jobs:
             ' <<<"$repositories_json"
           )
           sweep_target_count=${#sweep_targets[@]}
-          # Rotate the fixed walk order by the run number so the same
-          # organization-wide review-dispatch/branch-update budget lands on a
-          # different starting repository each tick instead of always exhausting
-          # on the same early repositories (#1219). Total dispatches per tick are
+          # 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 budget lands on a different starting repository
+          # each execution instead of always exhausting on the same early
+          # repositories (#1219). Total dispatches per execution are
           # unchanged; only which repositories receive them rotates over time.
           rotation_offset=0
           if [ "$sweep_target_count" -gt 0 ]; then
@@ -860,7 +955,7 @@ jobs:
               )
             fi
           fi
-          echo "Sweeping ${sweep_target_count} repositories starting at rotation offset ${rotation_offset} (run number ${ORG_SWEEP_ROTATION_INDEX})."
+          echo "Sweeping ${sweep_target_count} repositories starting at rotation offset ${rotation_offset} (rotation tick ${ORG_SWEEP_ROTATION_INDEX})."
 
           failures=0
           unavailable=0

CHANGELOG.md

@@ -45,6 +45,7 @@ Semantic Versioning where the repository publishes a release.
 
 ### Fixed
 
+- Derived `org-queue-sweep`'s rotation index (added in `ContextualWisdomLab/.github#1220` to stop the walk-order starvation from `ContextualWisdomLab/.github#1219`) from a persistent `ORG_SWEEP_ROTATION_COUNTER` repository variable incremented by exactly one at the start of every actual sweep execution, instead of `github.run_number` (which increments on every trigger of this workflow, not only the sweep schedule — Devin review finding on `#1220`) or a wall-clock tick alone (which can repeat an offset when this single-flight, up-to-60-minute job runs behind schedule by an exact multiple of the repository count — CodeRabbit review finding on `#1223`). Falls back to the wall-clock tick only if the persistent counter itself is unavailable, so a fairness mechanism never blocks the sweep's review-dispatch/merge work.
 - Retried the Strix scan up to `STRIX_TRANSIENT_RETRY_PER_MODEL` times, same model, when the log shows the upstream strix-agent Caido sandbox bootstrap timing race (`loginAsGuest failed after N attempts` / `Failed to connect to 127.0.0.1 port <port>`; tracked upstream as usestrix/strix#1036, #1037, #1056). A slow CI runner can exceed strix-agent's fixed 10-attempt sandbox-login budget before its local intercepting proxy is reachable, even though the penetration test itself never started and no vulnerability evidence was produced or lost; the Docker image is already cached from the failed attempt, so a same-model retry is cheap and typically clears the one-off boot race. Not wired into cross-model fallback, since switching LLM models cannot change local sandbox container boot timing.
 - Replaced nonexistent `job.workflow_repository` / `job.workflow_sha` / `job.workflow_ref` / `job.workflow_file_path` context references (actionlint: "property ... is not defined in object type") in `pr-review-fix-scheduler.yml`'s called-workflow source verification and `exact-artifact-sbom-attestation.yml`'s trusted-verifier checkout. Both always failed closed on the missing properties (ContextualWisdomLab/.github#1212) or, for the SBOM attestation checkout, silently resolved an empty repository/ref instead of the pinned trusted source (downstream `gh attestation verify --signer-repo`/`--signer-workflow`, using the separately hardcoded `SIGNER_REPOSITORY` constant rather than any workflow_ref, still failed closed on the resulting empty signer identity). `github.workflow_ref`/`github.workflow_sha` are real, documented properties, but for a `workflow_call` target they reflect the top-level *calling* workflow, not the reusable workflow's own file — a prefix match against the reusable workflow's own path can never succeed. `exact-artifact-sbom-attestation.yml`'s checkout now uses `github.workflow_sha` (correct today: it has no callers yet); `pr-review-fix-scheduler.yml`'s identity check instead validates `github.repository`, since every current caller uses a local, same-repo `uses: ./...` where caller and callee share one commit and `github.workflow_sha` is still the right pin. Tracked follow-up for the SBOM attestation checkout once a real (potentially cross-repo) caller exists: ContextualWisdomLab/.github#1228.
 - Used the receiving repository's workflow token for same-repository scheduler

docs/doctoring/org-queue-sweep-rotation.md

@@ -19,12 +19,46 @@ RankWeave's own turn.
 
 ## Decision
 
-Rotate the sweep's repository walk order by `github.run_number` (a value
-GitHub increments on every run of this workflow) before applying the
-unchanged organization-wide budget. `rotation_offset = run_number %
+Rotate the sweep's repository walk order by a rotation index before applying
+the unchanged organization-wide budget. `rotation_offset = rotation_index %
 repository_count`; the walk starts at that offset and wraps. This spreads the
 exact same total per-tick dispatch budget across repositories over successive
-ticks instead of raising it.
+sweep executions instead of raising it.
+
+`ORG_SWEEP_ROTATION_INDEX`'s primary source is a persistent
+`ORG_SWEEP_ROTATION_COUNTER` repository variable on `ContextualWisdomLab/.github`
+itself, incremented by exactly one at the start of every actual
+`org-queue-sweep` execution (`gh api .../actions/variables/ORG_SWEEP_ROTATION_COUNTER
+-X PATCH`, falling back to `-X POST` to create it on the first run). It falls
+back to a wall-clock tick (`$(date -u +%s) / 900`) 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. `ORG_SWEEP_ROTATION_INDEX` is left unset in the
+job's `env:` block in production so the sweep step computes it; tests inject
+it directly, or stub `gh` on `PATH`, for determinism.
+
+This design went through two prior, each independently review-flagged
+iterations, both instructive about why neither alone is sufficient:
+
+1. **`github.run_number`** (original `#1220`). Rejected because `run_number`
+   increments on every trigger of this workflow — push, `pull_request_target`,
+   `pull_request_review`, `workflow_run` — not only the `*/15` sweep schedule,
+   so it cannot give the "bounded by `repository_count` executions" guarantee
+   a rotation is meant to provide (Devin review finding on `#1220`; that
+   version merged before the correction landed, since the review comment was
+   informational rather than a blocking request-changes).
+2. **Wall-clock tick alone** (`#1223`, first revision). Rejected as the sole
+   source because `org-queue-sweep` is single-flight/non-cancelling with up to
+   a 60-minute `timeout-minutes`: a delayed or backlogged real execution can
+   let more than one 900-second window elapse before the next real run, and if
+   that elapsed-tick gap happens to be an exact multiple of `repository_count`
+   the modulo offset repeats — reintroducing the exact starvation `#1220`
+   fixed for a different reason (CodeRabbit review finding on `#1223`).
+
+A persistent per-execution counter is immune to both: it is untouched by
+non-sweep triggers of this workflow (unlike `run_number`) and advances by
+exactly one every time the sweep body actually runs, regardless of how much
+wall-clock time a slow prior run consumed (unlike a wall-clock tick alone).
 
 The budget-sizing question in #1219 (is `1` a deliberate LLM-provider
 cost/rate ceiling, or an unconsidered default?) is explicitly **not**
@@ -40,16 +74,21 @@ ceiling turns out to be conservative.
 
 - Every repository with ready work eventually reaches the front of the walk
   order and receives the shared dispatch, bounded by `repository_count`
-  ticks in the worst case, instead of never.
+  actual sweep executions in the worst case, instead of never.
 - Total review dispatches per tick, and therefore LLM-provider call volume
   per tick, are unchanged.
 - `rotation_offset` is logged (`Sweeping N repositories starting at rotation
-  offset O (run number R).`) so a specific tick's walk order is reconstructable
-  from the run log alone.
+  offset O (rotation tick T).`) so a specific execution's walk order is
+  reconstructable from the run log alone.
 - `ORG_SWEEP_ROTATION_INDEX` follows the same fail-closed numeric-validation
   pattern as the sibling `ORG_SWEEP_*_LIMIT` variables (reject non-digit
   input before it reaches arithmetic context, where an unguarded `set -e`
-  would not trap the error).
+  would not trap the error), applied after the persistent-counter/wall-clock
+  default fills it in when the environment does not already provide one.
+- A degraded run (counter unavailable) still rotates by wall-clock time
+  rather than reverting to the original fixed order; it only loses the
+  strict per-execution guarantee for that one run, logged as a
+  `::warning::`.
 
 ## Verification
 
@@ -59,13 +98,30 @@ ceiling turns out to be conservative.
   full permutation of the input, not a subset.
 - `test_org_queue_sweep_rotation_offset_is_safe_with_no_targets` covers the
   zero-repository edge case.
+- `test_org_queue_sweep_rotation_index_uses_persistent_counter_when_available`
+  stubs `gh` on `PATH` to simulate a successful read-increment-write and
+  confirms the counter advances by exactly one.
+- `test_org_queue_sweep_rotation_index_creates_counter_on_first_run` confirms
+  the POST-create fallback when the PATCH target does not exist yet.
+- `test_org_queue_sweep_rotation_index_falls_back_to_wall_clock` confirms the
+  wall-clock degraded path and its `::warning::` when the counter is entirely
+  unavailable.
+- `test_org_queue_sweep_rotation_index_override_is_preserved` and
+  `test_org_queue_sweep_rotation_index_rejects_malformed_override` cover the
+  test-injection and fail-closed-validation paths.
 - `test_org_queue_sweep_documents_rotation_leverage_and_validates_input`
-  locks the `#1219` cross-reference and confirms the shared budget constant
-  itself is untouched.
+  locks the `#1219` cross-reference, confirms `github.run_number` is not
+  reintroduced as the source, and confirms the shared budget constant itself
+  is untouched.
 - `actionlint` (with `shellcheck` on `PATH`) reports no findings against the
   modified workflow.
 
 ## References
 
 `ContextualWisdomLab/.github#1219` — original starvation report with sweep
 run evidence.
+`ContextualWisdomLab/.github#1220` — original rotation fix; `run_number` vs.
+per-execution-guarantee review discussion.
+`ContextualWisdomLab/.github#1223` — wall-clock correction, then the
+persistent-counter correction this document and the current workflow source
+reflect.