Skip to content

fix(strix): serialize scans per repository to stop shared-key rate-limit storms - #1297

Open
seonghobae wants to merge 9 commits into
mainfrom
fix/strix-repo-level-serialization
Open

fix(strix): serialize scans per repository to stop shared-key rate-limit storms#1297
seonghobae wants to merge 9 commits into
mainfrom
fix/strix-repo-level-serialization

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Root cause (5 Whys)

  1. Why did 7 open PRs in contextual-orchestrator fail the required Strix gate simultaneously? Every scan ended with litellm.RateLimitError: Nvidia_nimException (429) and fail-closed evidence rules.
  2. Why rate-limited? The org's shared NVIDIA NIM key received up to 7 concurrent scans × 3 retries each, every time any PR was pushed.
  3. Why so many concurrent scans? The Strix workflow concurrency group was scoped per PR (pr-{N}), so sibling PRs never shared a queue.
  4. Why does that matter? Provider-backed security evidence is only as reliable as provider capacity; concurrent fan-out converts a transient 429 into guaranteed gate failures across the whole queue.
  5. Root cause: concurrency design assumed per-PR isolation without modeling shared upstream provider budgets.

Fix

  • Scope the Strix concurrency group per repository (event class still separated: required pull_request_target evidence never interleaves with default-branch repository_dispatch retries).
  • cancel-in-progress: false + queue: max: queued evidence runs are preserved — nothing is dropped; accuracy prioritized over latency.
  • Queued runs already fetch the expected head SHA directly (refs/pull/<n>/head has already advanced handling), so late-started runs stay head-exact.
  • Contract test updated to encode repository-level serialization and no-cancel semantics.

Evidence

  • Failure logs from ContextualWisdomLab/contextual-orchestrator runs 32646490187 / 32676196905: repeated Nvidia_nimException - RateLimitError, all fallback models exhausted, zero-finding-before-infrastructure-failure fail-closed.
  • Local: pytest tests/ -q → 1342 passed (+ updated queue-contract tests), YAML validated.

Open in Devin Review

Summary by CodeRabbit

  • 새로운 기능

    • 보안 스캔이 저장소와 이벤트 유형별로 순차 처리되도록 개선되었습니다.
    • 스캔 대기 작업이 자동 취소되지 않고 큐에 보존됩니다.
    • 닫힌 PR과 관련된 이전 스캔 실행은 자동으로 정리됩니다.
  • 문서

    • 스캔 실행 순서와 큐 처리 방식에 대한 변경 사항이 기록되었습니다.
  • 버그 수정

    • 닫힌 PR의 불필요한 진행 중 또는 대기 중 스캔이 남는 문제를 해결했습니다.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Review Change Stack

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fe2c46cb-fe81-4879-a25d-2ae198bc031d

📝 Walkthrough

Walkthrough

Strix 워크플로의 동시성 그룹을 저장소·이벤트 유형 기준으로 변경했다. 진행 중인 실행과 대기 실행을 유지한다. 종료된 PR의 기존 실행은 별도 작업에서 GitHub API로 취소한다. 관련 계약 테스트와 변경 로그를 갱신했다.

Changes

Strix 실행 제어

Layer / File(s) Summary
저장소·이벤트별 실행 큐
.github/workflows/strix.yml, CHANGELOG.md
Strix 실행 그룹을 대상 저장소와 이벤트 유형 기준으로 변경했다. cancel-in-progress: falsequeue: max를 적용했다.
종료 PR 실행 정리
.github/workflows/strix.yml
종료된 PR의 queuedin_progress 실행을 페이지네이션으로 조회한다. head SHA 또는 PR 번호가 일치하고 현재 실행이 아닌 실행을 GitHub API로 취소한다.
실행 제어 계약 검증
scripts/ci/test_strix_quick_gate.sh, tests/test_required_workflow_queue_contract.py
저장소별 직렬화, 대상 저장소 fallback, 실행 보존, 종료 PR 정리 작업의 계약 검증을 갱신했다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🟠 High · up to c7000

The repository-level scan serialization change is not ready to merge because its contract test currently conflicts with the workflow, queued scans can still be dropped beyond the concurrency limit, and cleanup may cancel another pull request’s scan when commits are shared. These issues can cause required evidence to be skipped or the wrong validation run to be canceled.

Sequence Diagram(s)

sequenceDiagram
  participant PR as pull_request_target
  participant WF as Strix 워크플로
  participant API as GitHub Actions API
  PR->>WF: closed 이벤트 전달
  WF->>API: Strix 실행 목록 조회
  API-->>WF: queued 및 in_progress 실행 반환
  WF->>API: 현재 실행을 제외한 일치 실행 취소
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 저장소별 Strix 스캔 직렬화와 공유 키의 rate-limit 문제 완화라는 주요 변경 사항을 정확하고 간결하게 설명합니다.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/strix-repo-level-serialization

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

…mit storms

Root cause: the per-PR concurrency group let sibling PRs in one repository
scan concurrently; each run retried the shared NVIDIA NIM key up to three
times, producing litellm.RateLimitError storms and fail-closed gate failures
on every open PR (observed across ContextualWisdomLab/contextual-orchestrator
2026-08-23/24).

Change: scope the concurrency group per repository (event class still
separated so required pull_request_target evidence never interleaves with
default-branch repository_dispatch retries), set cancel-in-progress: false
with queue: max so queued evidence runs are preserved, and update the queue
contract test to encode the new serialization contract. Accuracy is
prioritized over scan latency; queued runs already fetch the expected head
SHA directly, so late-started runs stay head-exact.
@seonghobae
seonghobae force-pushed the fix/strix-repo-level-serialization branch from 60390f3 to d7133e2 Compare August 24, 2026 09:48
devin-ai-integration Bot and others added 2 commits August 24, 2026 09:57
…evel concurrency

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review loop

  • Pull request: fix(strix): serialize scans per repository to stop shared-key rate-limit storms #1297
  • Exact head reviewed: c7000c2fe77fe5e41ba17a4803eaed8bc8e92f3a
  • Valid finding fixed: the prior cancel-closed-pr-runs job only echoed a message. With repository-level cancel-in-progress: false, a closed pull request could leave queued or running Strix scans occupying the repository queue.
  • Change: closed pull-request events now use a separate concurrency group and a least-privilege actions: write job that cancels only queued/in-progress pull_request_target Strix runs matching the closed PR number or exact head SHA, excluding the current run.
  • Verification: python3 -m pytest -q tests/test_required_workflow_queue_contract.py — 64 passed; git diff --check — passed.
  • The branch was pushed only after re-reading the live PR head 623f7ac18cd5e667cc44b85d0865aafe66ca3172; the new head is c7000c2fe77fe5e41ba17a4803eaed8bc8e92f3a.

Please run the current-head review and required Checks again. No merge is requested until those exact-head gates and an eligible approval are present.

coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head validation and review disposition for ab17b10 (base 613a33e):

  • Replaced the unsupported GitHub Actions queue: max key with the native one-active/one-pending concurrency contract; the changelog now documents exact-head scheduler recovery after pending supersession.
  • Closed-PR cleanup now enumerates Actions runs, filters Strix Security Scan, and cancels only runs with a verifiable matching pull_requests[].number; it no longer cancels by shared head_sha.
  • The event-class split is deliberate: pull_request_target evidence is serialized per target repository and kept separate from default-branch repository_dispatch evidence. The workflow wording now says repository/event-class scope.
  • Full pytest: 1406 passed, 1 skipped, 16 subtests passed. Queue contract: 64 passed. actionlint, bash syntax, git diff --check, and the filtered Strix shell contract (RC=0) passed.

The remaining informational event-class thread is dispositioned by the documented contract above. Current required hosted Checks are still QUEUED and no current independent approval is present; predecessor evidence is not reused, so the PR remains on the normal protected path and is not merged yet.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae enabled auto-merge (squash) August 24, 2026 11:05

Copy link
Copy Markdown
Contributor Author

Additional Inkspan-bound acceptance evidence for this existing Strix capacity owner; no duplicate source writer or gate bypass is being created.

  • Dedicated Inkspan scheduler PR .github#1267 is unchanged at exact head 34efa03ecec7d815d8e6a4f7354767208fb1ce4a.
  • Its required Strix generation 32653596467, job 97228819582, reached the trusted scan path and then emitted repeated litellm.RateLimitError: Nvidia_nimException / HTTP 429 signals. The run failed closed, as it should.
  • That same incomplete generation also emitted one model-authored HIGH report before provider exhaustion, so a plain rerun/passing-status substitution is not acceptance; evidence still has to converge through the exact-report/source-authority owner.

This broadens the reproduced rate-limit failure from contextual-orchestrator to the Inkspan review-control lane and supports the repository/event-class serialization RCA here. After this PR is protected-main integrated, use a fresh legitimate generation rather than predecessor-run evidence: rerun the then-current .github#1267 head through required Strix and require an observed exact checkout plus terminal coherent Strix evidence. Then revalidate the Inkspan review queue itself by observing bounded review-repair progress for unchanged inkspan#362@11d5cfecdcc0949ec98e6ca110d482124bff00c4 and/or inkspan#373@c2db417df2c8f041d31ac5880c4898f00692c04e, without duplicating their existing trusted mentions. If repository serialization strands an earlier pending Inkspan head, the existing merge/review scheduler redispatch path must restore that exact-head evidence; an absent or superseded required check remains non-passing.

@opencode-agent
opencode-agent Bot disabled auto-merge August 24, 2026 11:13
@seonghobae
seonghobae enabled auto-merge (squash) August 24, 2026 11:21
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review disposition

  • Exact head: 309bbf5965c30ae2918b0ffabf66670b99ed7d13
  • The three Devin notes were reviewed against the exact diff.
  • The two name/head-sha notes are informational and match the repository's existing run identity contract.
  • The repository/event-class concurrency group and cancel-in-progress: false are deliberate: they prevent shared-provider rate-limit storms. The scheduler re-dispatches exact current heads after pending-run supersession; active_review_run_refs separates current and stale run identities, and dispatch_strix_evidence carries the exact pr_head_sha.
  • Existing contracts cover the repository-scoped key, non-cancellation policy, exact-head dispatch, and stale-run cleanup (tests/test_required_workflow_queue_contract.py, tests/test_pr_review_merge_scheduler.py).

This is an accepted latency tradeoff with a bounded recovery path, not a source defect or deadlock. Required hosted checks and independent approval remain pending; keep the PR on the protected normal path.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head review

Revalidated current head 309bbf5965c30ae2918b0ffabf66670b99ed7d13 against the complete four-file diff. The repository-scoped, event-class concurrency group prevents shared-provider rate-limit storms while preserving one active scan and one pending exact-head recovery slot; closed-PR cleanup is scoped to the matching PR and never checks out untrusted code.

Local evidence: python3 -m pytest -q tests/test_required_workflow_queue_contract.py — 64 passed; required Strix workflow smoke — passed; bash -n and git diff --check — passed. The scheduler scan-pr-queue cancellations are control-plane cancellation results, not source failures. Current hosted required checks are not terminal and no exact-head independent approval is present; keep normal protected merge gating. @opencode-agent @cwl-noema-review

@seonghobae

Copy link
Copy Markdown
Contributor Author

Additional exact-head validation for 309bbf5965c30ae2918b0ffabf66670b99ed7d13: full central suite 1406 passed, 1 skipped, 16 subtests passed; no source failure observed. Hosted required Checks and independent approval remain authoritative.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for 309bbf5965c30ae2918b0ffabf66670b99ed7d13.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: strix.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: strix.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["Changed file: CHANGELOG.md"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
  R2 --> V2["required checks"]
  Evidence --> S3["CI script: test_strix_quick_gate.sh"]
  S3 --> I3["review and security gate shell path"]
  I3 --> R3["Review risk: CI script: test_strix_quick_gate.sh"]
  R3 --> V3["bash -n plus Strix self-test"]
  Evidence --> S4["Test: test_required_workflow_queue_contract.py"]
  S4 --> I4["regression suite"]
  I4 --> R4["Review risk: Test: test_required_workflow_queue_contract.py"]
  R4 --> V4["targeted test run"]
Loading

@opencode-agent

opencode-agent Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 309bbf5965c30ae2918b0ffabf66670b99ed7d13
  • Workflow run: 32746075867
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for 309bbf5965c30ae2918b0ffabf66670b99ed7d13.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: strix.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: strix.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["Changed file: CHANGELOG.md"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
  R2 --> V2["required checks"]
  Evidence --> S3["CI script: test_strix_quick_gate.sh"]
  S3 --> I3["review and security gate shell path"]
  I3 --> R3["Review risk: CI script: test_strix_quick_gate.sh"]
  R3 --> V3["bash -n plus Strix self-test"]
  Evidence --> S4["Test: test_required_workflow_queue_contract.py"]
  S4 --> I4["regression suite"]
  I4 --> R4["Review risk: Test: test_required_workflow_queue_contract.py"]
  R4 --> V4["targeted test run"]
Loading

@opencode-agent
opencode-agent Bot disabled auto-merge August 24, 2026 17:40

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for 309bbf5965c30ae2918b0ffabf66670b99ed7d13.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Workflow: strix.yml"]
  S1 --> I1["GitHub Actions review job"]
  I1 --> R1["Review risk: Workflow: strix.yml"]
  R1 --> V1["actionlint plus required checks"]
  Evidence --> S2["Changed file: CHANGELOG.md"]
  S2 --> I2["repository behavior"]
  I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
  R2 --> V2["required checks"]
  Evidence --> S3["CI script: test_strix_quick_gate.sh"]
  S3 --> I3["review and security gate shell path"]
  I3 --> R3["Review risk: CI script: test_strix_quick_gate.sh"]
  R3 --> V3["bash -n plus Strix self-test"]
  Evidence --> S4["Test: test_required_workflow_queue_contract.py"]
  S4 --> I4["regression suite"]
  I4 --> R4["Review risk: Test: test_required_workflow_queue_contract.py"]
  R4 --> V4["targeted test run"]
Loading

@seonghobae
seonghobae enabled auto-merge (squash) August 25, 2026 01:34
@opencode-agent
opencode-agent Bot disabled auto-merge August 25, 2026 01:37
@seonghobae

Copy link
Copy Markdown
Contributor Author

Live queue evidence 2026-08-25T02:37Z, not OpenCode approval and not merge evidence.

This exact head 309bbf59 required Strix 32719353720 job 97653260509 has been in Run Strix (quick) since 01:37:26Z (60+ minutes, still in progress on runner GitHub Actions 1001300095). Trusted pull_request_target loaded concurrency from protected main@8fd471a, which is still per-PR (format('pr-{0}', ...)). This head's per-repository serialization is therefore not what scheduled the run.

While this job holds a runner, #1316@e4f9865a (32799128752) and a #941 re-run (97663710882) stay queued. Parallel per-PR scans this hour produced the shared-key 429 storm this PR describes, then discarded completed Vulnerabilities 0 49b results on the MODEL QUALITY WARNING banner (this repo #1311 32748314313 and #941 32718660538).

pull_request_target will keep using main's per-PR group until this concurrency change is on protected main. OpenCode CHANGES_REQUESTED remains on this SHA; opencode-review success is the bootstrap stub. Do not fold this slice into #1314.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Same-head required Strix 32719353720 job 97653260509 finished FAILURE at 02:44:26Z (Run Strix since 01:37:26Z). Not OpenCode approval and not merge evidence.

Overlay fallbacks were still luna (STRIX_FALLBACK_MODELS: ... openai-direct/gpt-5.6-luna). Trusted gate (pre-#1311):

  • Primary completed with Vulnerabilities 0, then report artifacts emitted warning/fatal/denied/timeout (banner).
  • Fallback 49b also Vulnerabilities 0, then emitted provider infrastructure or failure-signal output.
  • openai-direct/gpt-5.6-luna died in 4s.
  • Final: zero vulnerabilities before provider infrastructure failure.

That is the third hub reproduction this hour of #1311 (with #1311 32748314313 and #941 32718660538). This PR's per-repo concurrency still does not apply until it is on main. Do not fold into #1314.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

Open in Devin Review

Comment on lines +123 to +142
cancel_runs() {
local status="$1"
local runs_url="repos/${TARGET_REPOSITORY}/actions/runs?event=pull_request_target&status=${status}&per_page=100"
gh api --paginate "$runs_url" |
jq -r --arg pr "$CLOSED_PR_NUMBER" --arg head_sha "$CLOSED_PR_HEAD_SHA" --arg current "$CURRENT_RUN_ID" '
.workflow_runs[]
| select((.id | tostring) != $current)
| select(.name == "Strix Security Scan")
| select(.head_sha == $head_sha or any(.pull_requests[]?; ((.number | tostring) == $pr)))
| .id
' |
while IFS= read -r run_id; do
[ -n "$run_id" ] || continue
gh api --method POST "repos/${TARGET_REPOSITORY}/actions/runs/${run_id}/cancel" >/dev/null
echo "Cancelled Strix run ${run_id} for closed PR #${CLOSED_PR_NUMBER}."
done
}

cancel_runs queued
cancel_runs in_progress

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Closed-PR cleanup misses repository_dispatch and fork-PR runs

cancel_runs filters only event=pull_request_target, so a repository_dispatch scan for the closed PR is not cancelled. The pull_requests[] matcher is empty for fork runs; if head_sha also does not match, a closed fork PR's in-progress scan is left running and holds the per-repo queue. Same-repo PRs match by head SHA and PR number.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant