Skip to content

fix(strix): skip Draft pull-request admission - #2374

Draft
seonghobae wants to merge 3 commits into
mainfrom
cursor/strix-draft-admission-554c
Draft

seonghobae wants to merge 3 commits into
mainfrom
cursor/strix-draft-admission-554c

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

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

  • Native .github pull_request_target Draft opened, synchronize, and reopened events skip changed-scope, admit-current-head, and therefore strix at the job level.
  • Native ready_for_review remains subscribed and carries draft == false, creating fresh exact-head Strix evidence.
  • Ruleset-launched runs in other repositories bypass the Draft condition through github.repository != 'ContextualWisdomLab/.github'. The ruleset ignores types and does not re-trigger on ready_for_review, so skipping those runs would leave Draft-origin PRs without a later required scan.
  • converted_to_draft remains subscribed so workflow-level cancel-in-progress retires the prior Ready generation; the replacement does not admit the explicit cleanup runner. The cleanup condition also uses the same .github-only Draft boundary.
  • Closed PR cleanup, non-Draft synchronize cleanup, push, schedule, and forced repository_dispatch scans remain preserved.

A native .github Draft 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 on ready_for_review re-entry.

Timeout decision

The current strix job has no job-level timeout-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

  • RED: the original Strix Draft contract failed before implementation.
  • GREEN: focused Strix/required-workflow suite with GITHUB_ACTIONS=true — 129 passed, 1 subtest passed.
  • go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/strix.yml passed.
  • git diff --check passed.

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: .github reviewable PRs receive fresh Strix evidence without weakening security checks elsewhere in the organization.

Open in Web Open in Cursor 

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

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: Advanced

Run ID: 1a1dad1b-e4ca-4012-91fb-48e894814d45


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.

@seonghobae
seonghobae marked this pull request as ready for review September 25, 2026 16:12
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>

@cwl-noema-review cwl-noema-review 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.

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 .github repository. Hypothesis 1: 'Does this block Draft PRs in ruleset-covered repositories?' is falsified because if github.repository is not .github, the expression evaluates to true regardless of draft status. Hypothesis 2: 'Does this block non-Draft PRs in .github?' is falsified because if draft == false, the expression evaluates to true.
  • .github/workflows/strix.yml:192 (RIGHT): The admission logic for admit-current-head is mirrored from the changed-scope job, maintaining consistency in the admission pipeline for Draft PRs.
  • .github/workflows/strix.yml:251 (RIGHT): The exclusion of converted_to_draft from the cancel-superseded-pr-runs job 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-level cancel-in-progress for the converted_to_draft event to retire older runs without spawning a new runner.
  • tests/test_required_workflow_queue_contract.py:1161 (RIGHT): The test test_strix_draft_pr_events_skip_runner_admission_until_ready correctly 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 that converted_to_draft is 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]

@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 reviewed the current-head product diff. Coverage is a separate gate.

Changed files

  • .github/workflows/strix.yml — GitHub Actions review job
  • CHANGELOG.d/20260925-strix-draft-admission.md — repository behavior
  • docs/doctoring/strix-draft-admission.md — operator or user guidance
  • tests/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"]
Loading

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"]
Loading

@opencode-agent

opencode-agent Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

Admission correction — exact head 697fca7e8a349d2fa24817656a65debf3061965a

Current-head Runtime Quality 36162306463, CodeQL 36162306433, and Python Security 36162306202 are terminally failed; the latest opencode-agent review is CHANGES_REQUESTED. An approval from another reviewer does not supersede those failures. Canonical foundation successor .github#2385@950ab88553fe7415a73401c7ccc2e4749554395e carries the missing coverage lock, AnyIO audit repair, and CodeQL owner parents. This PR is returned to Draft/Proposed with its exact head preserved.

This branch has not been deployed

No deployments
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.

2 participants