Skip to content

fix(strix): serialize scans and resolve live NVIDIA NIM models - #1297

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

fix(strix): serialize scans and resolve live NVIDIA NIM models#1297
seonghobae wants to merge 34 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의 불필요한 스캔 실행이 정리됩니다.
    • 스캔 결과가 완료되지 않은 경우 안전하게 실패 처리됩니다.
  • 문서

    • 스캔 큐 처리, 모델 대체 및 실행 정리 방식이 업데이트되었습니다.

@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

Next included review available in 21 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 532cbeb6-2f76-4d40-a710-697136935ab2

📥 Commits

Reviewing files that changed from the base of the PR and between 641eef0 and adf861c.

📒 Files selected for processing (9)
  • .github/workflows/strix.yml
  • docs/doctoring/strix-nvidia-nim-not-found-fallback.md
  • scripts/ci/emit_opencode_failed_check_fallback_findings.sh
  • scripts/ci/select_nvidia_nim_model.py
  • scripts/ci/strix_required_workflow_smoke.sh
  • scripts/ci/test_strix_quick_gate.sh
  • tests/test_required_workflow_queue_contract.py
  • tests/test_select_nvidia_nim_model.py
  • tests/test_strix_nvidia_nim_not_found_fallback.py
📝 Walkthrough

Walkthrough

Strix 워크플로가 저장소·이벤트 유형별로 실행을 직렬화한다. 종료된 PR의 Strix 실행을 별도 작업에서 정리한다. 저장소 가시성, 모델 fallback, provider 장애 재시도 및 관련 검증을 갱신한다.

Changes

Strix 실행 제어

Layer / File(s) Summary
저장소·이벤트별 실행 큐
.github/workflows/strix.yml, tests/test_required_workflow_queue_contract.py, scripts/ci/test_strix_quick_gate.sh
Concurrency group을 저장소와 이벤트 유형 기준으로 변경했다. 진행 중인 실행을 취소하지 않으며, pending 실행이 대체되면 현재 HEAD를 재디스패치한다.
종료 PR 실행 정리
.github/workflows/strix.yml, CHANGELOG.md, tests/test_required_workflow_queue_contract.py
종료된 PR의 Strix pull_request_target 실행을 페이지네이션으로 조회한다. HEAD SHA 또는 PR 번호가 일치하는 실행을 취소하며, 조회·파싱·취소 오류는 경고 후 계속 처리한다.
저장소 가시성과 모델 fallback
.github/workflows/strix.yml, scripts/ci/test_strix_quick_gate.sh, tests/test_required_workflow_queue_contract.py
이벤트의 저장소 가시성을 우선 사용한다. 기본 모델과 직접 OpenAI fallback 모델을 gpt-5.4로 변경하고 fallback API base와 자격 증명 처리를 추가했다.
Provider 장애 재시도와 검증
.github/workflows/strix.yml, scripts/ci/test_strix_quick_gate.sh
인식된 provider 장애이고 취약점이 없을 때 최대 3회까지 선형 backoff로 재시도한다. 취약점 보고, 구성 오류, 예상 밖 종료 코드는 재시도하지 않는다. NVIDIA rate-limit 및 overload fallback 시나리오를 추가했다.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟡 Moderate · up to 641ee

The workflow now serializes scans per repository, but closed pull requests can still leave dispatch retries running and consuming provider capacity or changing security evidence, and inherited fallback configuration can make the validation contract fail. These issues should be fixed or explicitly accepted before merging.

Sequence Diagram(s)

sequenceDiagram
  participant PullRequestEvent
  participant StrixWorkflow
  participant GitHubActionsAPI
  PullRequestEvent->>StrixWorkflow: closed 이벤트 전달
  StrixWorkflow->>GitHubActionsAPI: Strix pull_request_target 실행 목록 조회
  GitHubActionsAPI-->>StrixWorkflow: queued 및 in_progress 실행 반환
  StrixWorkflow->>GitHubActionsAPI: 종료 PR과 일치하는 실행 취소
Loading
sequenceDiagram
  participant StrixWorkflow
  participant NVIDIAProvider
  participant OpenAIFallback
  StrixWorkflow->>NVIDIAProvider: Strix 실행
  NVIDIAProvider-->>StrixWorkflow: provider 장애 반환
  StrixWorkflow->>OpenAIFallback: gpt-5.4 fallback 실행
  OpenAIFallback-->>StrixWorkflow: 결과 반환
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (3 skipped: 2 …
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 Strix 스캔 직렬화와 NVIDIA NIM 모델 해결이라는 주요 변경 사항을 정확히 요약합니다. 짧고 구체적이며 변경 내용과 관련이 있습니다.
Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. (3 skipped: 2 unsupported, 1 too large.)

✨ 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 and resolve live NVIDIA NIM models #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.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae enabled auto-merge (squash) August 26, 2026 16:35

@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 0 new potential issues.

Open in Devin Review

@seonghobae

Copy link
Copy Markdown
Contributor Author

Exact-head hosted-check repair at 8f0b82511d33d809c9fa569a0e4bc1193ccd5ed7:

  • Devin reported zero new findings on predecessor 0db456d4; all review threads remain resolved.
  • The required Strix check then failed deterministically because protected main’s trusted smoke contract still requires the ordered NVIDIA-hosted fallback before direct OpenAI.
  • Restored that contracted fallback and aligned the branch tests.
  • Validation: actionlint passed; bash syntax passed; 79 focused Python contracts passed; the trusted required-workflow smoke passed.

Auto-merge remains enabled. The stale OpenCode CHANGES_REQUESTED review was not dismissed; current-head hosted checks and an eligible independent review remain authoritative.

@opencode-agent
opencode-agent Bot disabled auto-merge August 26, 2026 17:10
devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae
seonghobae enabled auto-merge (squash) August 26, 2026 17:20
coderabbitai[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae seonghobae changed the title fix(strix): serialize scans per repository to stop shared-key rate-limit storms fix(strix): serialize scans and resolve live NVIDIA NIM models Aug 26, 2026

@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 thread .github/workflows/strix.yml Outdated
@opencode-agent
opencode-agent Bot disabled auto-merge August 26, 2026 17:56
@seonghobae
seonghobae enabled auto-merge (squash) August 26, 2026 17:59
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