Skip to content

fix(ci): restore PR-scoped concurrency for strix.yml on owner directive #1779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
seonghobae merged 1 commit into from
Sep 3, 2026
Merged

fix(ci): restore PR-scoped concurrency for strix.yml on owner directive #1779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions .github/workflows/strix.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 29 additions & 15 deletions tests/test_required_workflow_queue_contract.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand Down
Loading