fix(strix): skip Draft pull-request admission - #2374
seonghobae wants to merge 3 commits into
Conversation
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 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. Comment |
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
There was a problem hiding this comment.
Noema LLM review
The changes correctly implement the skipping of Strix runner admission for Draft pull requests specifically within the .github repository, while maintaining admission for Draft PRs in other repositories (which are covered by organization rulesets) and ensuring non-Draft PRs remain unaffected. The exclusion of converted_to_draft from the cleanup job is logically sound given the workflow-level concurrency settings. The addition of a targeted test case ensures these Boolean constraints are maintained.
Reviewed changed lines
.github/workflows/strix.yml:132 (RIGHT): The updated if-condition(github.event.pull_request.draft == false || github.repository != 'ContextualWisdomLab/.github')correctly implements the requirement to skip Draft PRs only in the.githubrepository. Hypothesis 1: 'Does this block Draft PRs in ruleset-covered repositories?' is falsified because ifgithub.repositoryis not.github, the expression evaluates to true regardless of draft status. Hypothesis 2: 'Does this block non-Draft PRs in .github?' is falsified because ifdraft == false, the expression evaluates to true..github/workflows/strix.yml:192 (RIGHT): The admission logic foradmit-current-headis mirrored from thechanged-scopejob, maintaining consistency in the admission pipeline for Draft PRs..github/workflows/strix.yml:251 (RIGHT): The exclusion ofconverted_to_draftfrom thecancel-superseded-pr-runsjob is consistent with the provided documentation. The logic(github.event.action == 'synchronize' && (github.event.pull_request.draft == false || github.repository != 'ContextualWisdomLab/.github'))ensures that cleanup still occurs for non-Draft PRs or those in other repositories, while relying on the workflow-levelcancel-in-progressfor theconverted_to_draftevent to retire older runs without spawning a new runner.tests/test_required_workflow_queue_contract.py:1161 (RIGHT): The testtest_strix_draft_pr_events_skip_runner_admission_until_readycorrectly probes the YAML for the specific Boolean expressions implemented. It verifies the presence of the repository check and the draft check across the relevant jobs and explicitly confirms thatconverted_to_draftis no longer in the cleanup header.
Adversarial validation
.github/workflows/strix.yml:132 (RIGHT)falsified: Draft PRs in non-.github repositories will be blocked from scanning. — The OR condition specifically isolates the Draft restriction to the .github repository..github/workflows/strix.yml:132 (RIGHT)falsified: Non-Draft PRs in .github will be blocked from scanning. — The draft == false check takes precedence for all repositories.- Residual risk: none
Findings
- No blocking findings.
- Result: APPROVE
- Head SHA:
697fca7e8a349d2fa24817656a65debf3061965a - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
.github/workflows/strix.yml— GitHub Actions review jobCHANGELOG.d/20260925-strix-draft-admission.md— repository behaviordocs/doctoring/strix-draft-admission.md— operator or user guidancetests/test_required_workflow_queue_contract.py— regression suite
Changed behavior
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["Repository file: 20260925-strix-draft-admission.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Repository file: 20260925-strix-draft-admission.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: strix-draft-admission.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: strix-draft-admission.md"]
R3 --> V3["docs review"]
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"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
697fca7e8a349d2fa24817656a65debf3061965a - Workflow run: 36199583192
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
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["Repository file: 20260925-strix-draft-admission.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Repository file: 20260925-strix-draft-admission.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: strix-draft-admission.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: strix-draft-admission.md"]
R3 --> V3["docs review"]
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"]
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
Admission correction — exact head
|
Summary
Stop Strix from admitting runner jobs for Draft pull requests only when this workflow runs natively in
ContextualWisdomLab/.github, while preserving required security coverage in ruleset-targeted repositories.Behavior
.githubpull_request_targetDraftopened,synchronize, andreopenedevents skipchanged-scope,admit-current-head, and thereforestrixat the job level.ready_for_reviewremains subscribed and carriesdraft == false, creating fresh exact-head Strix evidence.github.repository != 'ContextualWisdomLab/.github'. The ruleset ignorestypesand does not re-trigger onready_for_review, so skipping those runs would leave Draft-origin PRs without a later required scan.converted_to_draftremains subscribed so workflow-levelcancel-in-progressretires the prior Ready generation; the replacement does not admit the explicit cleanup runner. The cleanup condition also uses the same.github-only Draft boundary.repository_dispatchscans remain preserved.A native
.githubDraft run publishes skipped job conclusions rather than relying on a trigger-level filter that could leave a required context Pending. Ruleset-targeted repositories continue to run Strix while Draft because their required workflow cannot depend onready_for_reviewre-entry.Timeout decision
The current
strixjob has no job-leveltimeout-minutes. This PR intentionally does not add one: the job runs model inference synchronously, explicitly disables request/process/total model deadlines, and repository policy accepts Strix scans taking more than two hours. A wall-clock cap could terminate legitimate large-repository analysis; progress-based occupancy handling remains the applicable protection.Scope and follow-up
This PR changes only Strix Draft admission and the ruleset-boundary documentation. Security Scan, SAST Semgrep, and CodeQL receive the same
.github-only boundary in follow-up #2376. Python Security and Agent Review Runtime Quality retain their direct-run Draft guards because they are not ruleset-required.Validation
GITHUB_ACTIONS=true—129 passed, 1 subtest passed.go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/strix.ymlpassed.git diff --checkpassed.Developer experience: native Draft PR pushes avoid Strix runner admission while ruleset-targeted repositories retain required coverage despite the missing Ready-event re-entry.
User experience:
.githubreviewable PRs receive fresh Strix evidence without weakening security checks elsewhere in the organization.