diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 22bbeab242..32a72ad664 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -250,12 +250,26 @@ jobs: needs: changed-scope if: (github.event_name != 'pull_request_target' || github.event.action != 'closed') && needs.changed-scope.outputs.code == 'true' concurrency: - # Keep provider-backed scans serial per repository and event class while - # allowing the trusted cleanup job above to retire an obsolete head now. + # PR-scoped (workflow-repository-PR), matching every other central + # required workflow's group-key convention. This was deliberately + # repository-wide instead, from 2026-08-24 through 2026-09-03, because + # PR-scoping is what caused a real litellm.RateLimitError storm against + # the shared NVIDIA NIM key on 2026-08-23/24 (.github#1297) -- widening + # it back reintroduces that risk, now at a larger blast radius since + # Strix is required org-wide via ruleset 18156473. Restored to PR-scoped + # on explicit owner authorization (2026-09-03) after confirming the two + # NVIDIA NIM credentials (NVIDIA_NIM_API_KEY, NVIDIA_NIM_API_KEY_SUB) + # have independent rate limits (~40 RPM each per community-reported + # figures, no official SLA) rather than a shared pool -- see + # docs/product-technical-gap-baseline.md for the full tradeoff writeup. + # cancel-in-progress stays false: a same-PR push still queues behind an + # in-flight scan for that PR rather than cancelling it, preserving that + # head's scan log (the trusted cleanup job above independently retires + # a genuinely superseded head). group: >- strix-${{ (github.event_name == 'pull_request_target' || github.event_name == 'repository_dispatch') && - format('{0}-{1}', github.event_name, github.event.client_payload.target_repository || github.event.pull_request.base.repo.full_name || github.repository) || + format('{0}-{1}-{2}', github.event_name, github.event.client_payload.target_repository || github.event.pull_request.base.repo.full_name || github.repository, github.event.pull_request.number || github.event.client_payload.pr_number || github.run_id) || format('{0}-{1}-{2}', github.event_name, github.repository, github.ref) }} cancel-in-progress: false diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index dcd5e2ef49..cd3b3e8668 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -334,15 +334,26 @@ def test_central_semgrep_binds_pr_scans_and_sarif_to_the_exact_head() -> None: ) -def test_strix_serializes_provider_evidence_per_repository() -> None: - """Serialize Strix per repository so shared provider keys are not rate-limited. - - Root cause (2026-08-23/24): sibling PRs scanned concurrently, each retrying - the shared NVIDIA NIM key three times, producing litellm.RateLimitError - storms and fail-closed gate failures on every open PR. The concurrency group - now scopes the scan job per repository and event class. The cleanup job is - outside that queue so a synchronize event can immediately retire an older - exact-head run without allowing sibling scans to overlap. +def test_strix_serializes_provider_evidence_per_repository_and_pr() -> None: + """Scope Strix per repository AND PR, matching every other central workflow. + + History: from 2026-08-24 through 2026-09-03 the concurrency group was + deliberately repository-wide (not PR-scoped) because PR-scoping is what + caused a real litellm.RateLimitError storm against the shared NVIDIA NIM + key on 2026-08-23/24 -- sibling PRs scanned concurrently, each retrying the + shared key three times, producing fail-closed gate failures on every open + PR. That repository-wide scoping fixed the storm but starved cross-PR + Strix evidence within the same repository instead (a different PR's scan + always queued behind whichever scan was already running there). + + Restored to PR-scoped on explicit owner authorization (2026-09-03) after + confirming NVIDIA_NIM_API_KEY and NVIDIA_NIM_API_KEY_SUB have independent + rate limits rather than a shared pool, giving materially more headroom + than the single-key 2026-08-23/24 incident had. The concurrency group now + scopes the scan job per repository, PR (or run id for non-PR events), and + event class. The cleanup job is outside that queue so a synchronize event + can immediately retire an older exact-head run without allowing sibling + scans for *other* PRs to be blocked by it. """ workflow = workflow_text("strix.yml") concurrency_contract = workflow.split("concurrency:", 1)[1].split( @@ -354,15 +365,18 @@ def test_strix_serializes_provider_evidence_per_repository() -> None: assert "github.event.pull_request.base.repo.full_name" in concurrency_contract assert "github.repository" in concurrency_contract assert ( - "format('{0}-{1}', github.event_name, github.event.client_payload.target_repository || " - "github.event.pull_request.base.repo.full_name || github.repository)" + "format('{0}-{1}-{2}', github.event_name, github.event.client_payload.target_repository || " + "github.event.pull_request.base.repo.full_name || github.repository, " + "github.event.pull_request.number || github.event.client_payload.pr_number || github.run_id)" ) in concurrency_contract assert ( "format('{0}-{1}-{2}', github.event_name, github.repository, github.ref)" in concurrency_contract ) - # Repository-level (not PR-level) grouping: no pr-{N} component remains. - assert "format('pr-{0}', github.event.pull_request.number)" not in concurrency_contract + # PR-scoped grouping: the PR (or client_payload) number is part of the key. + assert "github.event.pull_request.number || github.event.client_payload.pr_number" in ( + concurrency_contract + ) assert "github.event.pull_request.head.sha" not in concurrency_contract assert "github.event.client_payload.pr_head_sha" not in concurrency_contract # Running scans are not cancelled; GitHub's native group has one pending slot. @@ -613,10 +627,10 @@ def test_pull_request_close_events_cancel_superseded_runs_without_heavy_jobs() - assert "${{ secrets." not in opencode_bootstrap strix_workflow = workflow_text("strix.yml") - # Strix serializes scans per repository while cleanup stays outside that + # Strix scopes scans per repository and PR while cleanup stays outside that # queue so synchronize and close events can immediately retire old work. assert "cancel-in-progress: false" in strix_workflow - assert "Keep provider-backed scans serial per repository" in strix_workflow + assert "PR-scoped (workflow-repository-PR)" in strix_workflow def test_close_empty_pr_metadata_lookup_retries_and_fails_open() -> None: