From ab9f378259ae09c866409483d5a03d369ffc94d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 18:14:28 +0000 Subject: [PATCH] fix(actions): pin central required review workflows off starved ubuntu-latest strix.yml, opencode-review.yml, and noema-review.yml -- the three workflows the org's own required-workflow ruleset runs against every PR in every sibling repository -- still requested the floating ubuntu-latest image after #1618 (security gates) and #1609 (merge scheduler) already pinned their own jobs off it as "the observed starved floating image". Since these three are the actual required-check gate blocking merge organization-wide, a starved image here is a direct, high-leverage contributor to the sustained multi-hour organization-wide Actions queuing observed throughout this session (independently corroborated by #1630's own record of 822 queued runs at merge time). Pinned all 9 occurrences (3 + 5 + 2) to ubuntu-24.04, matching the established pattern exactly. New tests/test_required_review_runner_image_contract.py mirrors test_required_security_runner_image_contract.py's structure. Also fixed 4 unrelated pre-existing test failures on main, confirmed to reproduce identically on a clean origin/main checkout: #1630 moved the organization sweep's rotation cadence from every 15 minutes to hourly (reducing control-plane pressure under the same Actions saturation), changing pr-review-merge-scheduler.yml's ORG_SWEEP_ROTATION_INDEX wall-clock fallback divisor from 900 to 3600, but left test_required_workflow_queue_contract.py's four rotation-index tests asserting the old divisor/string. Full suite: 2407 passed, 1 skipped, 21 subtests. Coverage 100% on scripts/ci. Interrogate 100%. All touched workflow files re-parse as valid YAML. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 --- .github/workflows/noema-review.yml | 4 +- .github/workflows/opencode-review.yml | 10 ++--- .github/workflows/strix.yml | 6 +-- CHANGELOG.md | 1 + docs/product-technical-gap-baseline.md | 13 +++++++ ...t_required_review_runner_image_contract.py | 37 +++++++++++++++++++ .../test_required_workflow_queue_contract.py | 8 ++-- 7 files changed, 65 insertions(+), 14 deletions(-) create mode 100644 tests/test_required_review_runner_image_contract.py diff --git a/.github/workflows/noema-review.yml b/.github/workflows/noema-review.yml index 6b2e3fcede..2c941983f9 100644 --- a/.github/workflows/noema-review.yml +++ b/.github/workflows/noema-review.yml @@ -35,7 +35,7 @@ permissions: jobs: cancel-closed-pr-runs: if: github.event_name == 'pull_request_target' && github.event.action == 'closed' - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 permissions: actions: write contents: read @@ -179,7 +179,7 @@ jobs: noema-review: name: noema-review - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 if: >- github.event_name == 'repository_dispatch' || ( diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 87827f5322..0864993179 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -44,7 +44,7 @@ permissions: jobs: required-workflow-bootstrap: name: required-workflow-bootstrap - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - name: Materialize the required review workflow run: >- @@ -250,7 +250,7 @@ jobs: coverage-source-tree: name: coverage-source-tree needs: [required-workflow-bootstrap] - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - run: >- echo "PR-head source and coverage execution are delegated to the @@ -259,7 +259,7 @@ jobs: coverage-evidence: name: coverage-evidence needs: [coverage-source-tree] - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 steps: - run: >- echo "This required-workflow job preserves the stable branch-protection @@ -268,7 +268,7 @@ jobs: opencode-review-target: name: opencode-review needs: [coverage-evidence] - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 permissions: contents: read pull-requests: read @@ -475,7 +475,7 @@ jobs: # immediately beforehand, so a run for this job that is itself somehow # delayed/stale cannot wrongly cancel a still-authoritative run. if: github.event_name == 'pull_request_target' && github.event.action == 'synchronize' - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 permissions: actions: write contents: read diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 26d5d8b2cb..7674b3040f 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -79,7 +79,7 @@ permissions: jobs: cancel-superseded-pr-runs: if: github.event_name == 'pull_request_target' && (github.event.action == 'synchronize' || github.event.action == 'closed') - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 # Prefer the established scheduler credential, but let the close event use # its job-scoped token so abandoned scans are cancelled even when that # optional secret is unavailable. This job never checks out PR code. @@ -190,7 +190,7 @@ jobs: # scans may take more than two hours per model (docs/product-goal-directive.md). # Inference has no wall-clock deadline; cancellation is reserved for an # explicit operator action or a superseded head. - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 # Least-privilege token scoped to this job (Scorecard alert #43): the scan # exchanges an OIDC token (id-token) and publishes same-repo status evidence # from the scan job only. @@ -1019,7 +1019,7 @@ jobs: name: publish-manual-pr-evidence-status needs: strix if: ${{ always() && !cancelled() && github.event_name == 'repository_dispatch' && github.event.client_payload.pr_head_sha != '' }} - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 permissions: id-token: write statuses: write # Required for downscoped OIDC status publication. diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f980f794d..4b661772cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ this file. The format follows Keep a Changelog, and versioned releases follow Semantic Versioning where the repository publishes a release. ## [Unreleased] +- **Pin the three central required review workflows (Strix, OpenCode Review, Noema Review) off the observed starved floating `ubuntu-latest` runner image.** Following the same repair already rolled out to security gates (`#1618`) and the merge scheduler (`#1609`), `strix.yml`, `opencode-review.yml`, and `noema-review.yml` now request the explicit `ubuntu-24.04` image on every job. These three workflows are the org's own required-workflow gate for every sibling repository, so a starved floating image here directly contributes to organization-wide required-check queuing. New `tests/test_required_review_runner_image_contract.py` asserts no job in any of the three files still requests the floating image. Also fixed 4 pre-existing, unrelated test failures on `main` left by `#1630`'s organization-sweep rotation cadence change (every 15 minutes to hourly, to reduce control-plane pressure under the same Actions saturation): `tests/test_required_workflow_queue_contract.py`'s rotation-index tests still asserted the old `/ 900` (15-minute) divisor against the new `/ 3600` (hourly) production value. - **Refresh Noema reviewer App authority after long model work (`#1616`).** A real `naruon#1497` review outlived its repository-scoped GitHub App installation token and failed the next exact-head GitHub operation with HTTP 401. The trusted workflow now prepares the validated verdict into a private runner-local envelope, remints the same least-privilege repository-scoped App authority after model work, independently re-fetches exact live head/reviewer identity, and only then publishes. Skipped preparation creates no envelope, predecessor App tokens cannot authorize publication, PAT/OIDC remain explicit fail-closed sources, malformed handoffs are cleaned up, and executable plus step-scoped regressions cover stale-head, identity, alias, workflow wiring, and migration of legacy broader-suite contracts away from the retired single-process reviewer path. - Fix `existing_noema_review()` treating a "legacy" Noema review (one posted before `NOEMA_REVIEW_FOOTER_MARKER` existed) as proof the current head was already reviewed. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 7ba1d7cd41..41d95b6f57 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -2576,3 +2576,16 @@ Higgins, S. S., Crepalde, N., & Fernandes, L. (2021). Segmented multiplexity: A **Regression-suite consistency.** Legacy broader-suite assertions that still named the retired single-process Noema step/module are migrated to the two-phase prepare/publish contract, including step-scoped helper and envelope-argument evidence. This closes the false-GREEN gap where focused token-lifetime CI could pass while unchanged broader contracts described an impossible execution path. **Residual external verification.** After this central change reaches protected `main`, replay Required Noema Review for unchanged `naruon#1497@152d1998c4e8024be9dc7026c8789d343c884fd0`. Closure evidence requires a current-head schema-valid review or typed review-unavailable outcome without expired-token 401; a pre-merge run cannot prove the merged workflow-source path and is not promoted to release evidence. + + +## 2026-09-01 central required review workflows: floating runner image contributing to organization-wide queuing + +**Observed gap.** `#1618` (required security gates) and `#1609` (merge scheduler) already pinned their jobs off `ubuntu-latest` after this session found it to be, in that fix's own words, "the observed starved floating image" — GitHub-hosted runners requesting the floating `ubuntu-latest` label were being left `queued` with no runner assignment for hours, well beyond ordinary scheduling latency, while identical jobs on other repositories/workflows completed normally. `strix.yml`, `opencode-review.yml`, and `noema-review.yml` — the three workflows the org's own required-workflow ruleset runs against every PR in every sibling repository — still requested `ubuntu-latest` on every job (9 occurrences total: 3 in `strix.yml`, 5 in `opencode-review.yml`, 2 in `noema-review.yml`; `pr-review-merge-scheduler.yml` was already covered by `#1609`). Since these three are the actual required-check gate blocking merge across the whole organization, a starved image here is a direct, high-leverage contributor to the sustained multi-hour organization-wide queuing observed throughout this session (independently corroborated by `#1630`'s own record of 822 queued Actions runs at merge time). + +**Fix.** Pinned all 9 occurrences to the explicit `ubuntu-24.04` image, matching the pattern already established by `#1618`/`#1609` exactly (a literal `runs-on:` value swap, no other job semantics touched). New `tests/test_required_review_runner_image_contract.py` asserts no job in any of the three files requests the floating image and pins the expected per-file occurrence count, mirroring `test_required_security_runner_image_contract.py`'s existing structure. + +**Unrelated pre-existing failures fixed in the same pass.** `#1630` (merged shortly before this fix, itself an owner-authorized `QUEUE_SATURATION_CHICKEN_EGG` bypass addressing the same 822-run backlog) moved the organization sweep's rotation cadence from every 15 minutes to hourly to reduce control-plane pressure, changing `pr-review-merge-scheduler.yml`'s `ORG_SWEEP_ROTATION_INDEX` wall-clock fallback divisor from `900` (15 minutes in seconds) to `3600` (1 hour), but left `tests/test_required_workflow_queue_contract.py`'s four rotation-index tests asserting the old `900` divisor and the old literal workflow string. Confirmed these 4 failures reproduce identically on a clean `origin/main` checkout with no changes from this branch, independent of and pre-dating this fix. Updated all four to the new `3600` divisor/string, preserving each test's original intent (wall-clock fallback on total counter unavailability, transient-read-failure-does-not-reset, successful-read-but-failed-patch-falls-back, and the documentation/input-validation contract) unchanged. + +**Validation.** Full suite `2407 passed, 1 skipped, 21 subtests`; `coverage` 100% on `scripts/ci`; `interrogate` 100%; all four touched/added workflow files re-parse as valid YAML; `test_opencode_workflow_shell_syntax.py` and related shell-syntax tests pass unchanged. + +**Residual.** This closes the specific floating-image contribution from these three central workflows; it does not by itself guarantee the organization-wide Actions queue is fully drained, since other repositories' own workflows and any remaining unpinned central workflows may still request the floating image. Worth a follow-up sweep across the rest of `.github/workflows/` and sibling-repo workflows if queuing persists after this lands. diff --git a/tests/test_required_review_runner_image_contract.py b/tests/test_required_review_runner_image_contract.py new file mode 100644 index 0000000000..c173716e3e --- /dev/null +++ b/tests/test_required_review_runner_image_contract.py @@ -0,0 +1,37 @@ +"""Contract tests for central required review workflow runner images.""" + +from __future__ import annotations + +from pathlib import Path +import unittest + + +STRIX = Path(".github/workflows/strix.yml") +OPENCODE_REVIEW = Path(".github/workflows/opencode-review.yml") +NOEMA_REVIEW = Path(".github/workflows/noema-review.yml") + + +class RequiredReviewRunnerImageContract(unittest.TestCase): + """Keep required review jobs off the observed starved floating image.""" + + def test_strix_uses_explicit_supported_image(self) -> None: + """Require every Strix job to use explicit Ubuntu 24.04.""" + workflow = STRIX.read_text(encoding="utf-8") + self.assertNotIn("runs-on: ubuntu-latest", workflow) + self.assertEqual(workflow.count("runs-on: ubuntu-24.04"), 3) + + def test_opencode_review_uses_explicit_supported_image(self) -> None: + """Require every OpenCode Review job to use explicit Ubuntu 24.04.""" + workflow = OPENCODE_REVIEW.read_text(encoding="utf-8") + self.assertNotIn("runs-on: ubuntu-latest", workflow) + self.assertEqual(workflow.count("runs-on: ubuntu-24.04"), 5) + + def test_noema_review_uses_explicit_supported_image(self) -> None: + """Require every Noema Review job to use explicit Ubuntu 24.04.""" + workflow = NOEMA_REVIEW.read_text(encoding="utf-8") + self.assertNotIn("runs-on: ubuntu-latest", workflow) + self.assertEqual(workflow.count("runs-on: ubuntu-24.04"), 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index e18e021fae..8614d02903 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1292,7 +1292,7 @@ def test_org_queue_sweep_rotation_index_falls_back_to_wall_clock(tmp_path: Path) assert result.returncode == 0, result.stderr stdout_lines = result.stdout.strip().splitlines() computed_tick = int(stdout_lines[-1]) # last line: the printed value; earlier: the warning - expected_tick = int(time.time()) // 900 + expected_tick = int(time.time()) // 3600 assert abs(computed_tick - expected_tick) <= 1 # tolerate a tick boundary race assert "could not read/write" in result.stdout # a `::warning::` workflow command @@ -1318,7 +1318,7 @@ def test_org_queue_sweep_rotation_index_transient_read_failure_does_not_reset_co assert result.returncode == 0, result.stderr stdout_lines = result.stdout.strip().splitlines() computed_tick = int(stdout_lines[-1]) - expected_tick = int(time.time()) // 900 + expected_tick = int(time.time()) // 3600 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. @@ -1341,7 +1341,7 @@ def test_org_queue_sweep_rotation_index_successful_read_but_failed_patch_falls_b assert result.returncode == 0, result.stderr stdout_lines = result.stdout.strip().splitlines() computed_tick = int(stdout_lines[-1]) - expected_tick = int(time.time()) // 900 + expected_tick = int(time.time()) // 3600 assert abs(computed_tick - expected_tick) <= 1 assert "read ORG_SWEEP_ROTATION_COUNTER=41 but could not PATCH it" in result.stdout @@ -1386,7 +1386,7 @@ def test_org_queue_sweep_documents_rotation_leverage_and_validates_input() -> No assert "ContextualWisdomLab/.github#1219" in workflow assert ( - 'ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 900 ))' + 'ORG_SWEEP_ROTATION_INDEX=$(( $(date -u +%s) / 3600 ))' ) in workflow assert ( 'if ! [[ "$ORG_SWEEP_ROTATION_INDEX" =~ ^[0-9]+$ ]]; then'