Skip to content

fix(ci): skip eval fixtures in check-affected's ruff step - #12582

Merged
kyle-sexton merged 2 commits into
mainfrom
fix/check-affected-ruff-force-exclude
Nov 3, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
fix/check-affected-ruff-force-exclude

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

No related issue: a one-line fix to the pre-push check, found while pushing #12549 and blocking it.

Summary

check-affected.sh passes changed .py files to ruff by explicit path, and ruff applies extend-exclude only to files it finds by traversal (ruff.toml:13). So any branch that renames or edits a Python file under evals/fixtures/ fails pre-push on defects those fixtures hold on purpose.

Fix

The pre-push ruff step now passes --force-exclude, so it skips the same files CI skips. Every other check is unchanged. The hook runs the merge-base copy of this script, so #12549 cannot carry this fix itself.

Verification

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01VQb9Pzf47aw4gsMAyk98Wn

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

@github-actions github-actions Bot added the needs-issue-linkage Pull request body has no issue linkage line; set by the PR-contract check, advisory. label Nov 3, 2026
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 01:40
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T01:41:38.820426Z 160bb07 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 160bb0745feddf3e1d3c9f738af5567bda025e95: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review: no security issues found

I read the full diff: one hunk in one file, scripts/check-affected.sh. The change adds --force-exclude to the local pre-push ruff call at scripts/check-affected.sh#L464. With that flag, ruff applies its configured extend-exclude to files passed by explicit path, so pre-push now skips the same files CI skips.

Why it isn't a security issue

  • Lint only: the change affects linting on the developer's own machine. It doesn't touch credentials, tokens, workflow permissions, trust controls, merge holds, the kill switch, plugin manifests or any standing instruction.
  • No injection risk: the arguments are still passed as the quoted "${py[@]}" array. The only addition is a fixed literal flag.
  • Nothing new is excluded: files are skipped only if ruff.toml's exclude list already covers them. Pre-push now matches CI's traversal behaviour rather than checking those files more strictly than CI does.
  • No trusted paths: the changed file matches none of the trusted-path globs.

critical-findings: 0

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed 160bb0745feddf3e1d3c9f738af5567bda025e95: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #12582, fix(ci): skip eval fixtures in check-affected's ruff step

No findings. This review covered the whole change: the single hunk in scripts/check-affected.sh, which is 1 of 1 changed files.

What changed

The pre-push ruff step now passes --force-exclude to scripts/run-ruff.sh check. Changed .py files under extend-exclude (**/evals/fixtures/**) are now skipped there too, so pre-push matches CI, which finds files by walking the tree.

Checks

  • The PR's reason holds. ruff.toml#L13-L14 says it outright: "Excludes govern traversal only: a directly-passed path needs --force-exclude."
  • The flag reaches ruff. scripts/run-ruff.sh passes "$@" unchanged to ruff or uvx ruff==<pin>.
  • The displayed command matches the run. The label string given to run_check changed together with the argv, so the two still agree (L464).
  • No test or doc breaks. No test or other file asserts the old command string. The only path-scoped rule touching ruff, .claude/rules/ruff-pin.md, covers **/*.py, not this .sh file.
  • Unverified, from the author. The PR says that with the flag, a fixture-only run prints "No Python files found" and exits clean. I have no shell, so I did not run it.

Closing issue

None. The PR body says there is no related issue, so there are no acceptance criteria to check.

Notes

  • Prior findings: none (0 rounds, cap 3). Settled threads: none were provided.
  • No embedded instructions found in the PR context.
  • No hold: the change spends no money and has no security impact.

@kyle-sexton
kyle-sexton enabled auto-merge November 3, 2026 01:45
@github-actions github-actions Bot removed the needs-issue-linkage Pull request body has no issue linkage line; set by the PR-contract check, advisory. label Nov 3, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 77d9c75 Nov 3, 2026
82 checks passed
@kyle-sexton
kyle-sexton deleted the fix/check-affected-ruff-force-exclude branch November 3, 2026 01:52
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Post-merge mutation testing

Killed 0, survived 1, timeout 0, no coverage 0, invalid 0, not run 0.
Covered-code score 0%; mutation score 0%.

File Killed Survived Timeout No coverage Invalid Not run
scripts/check-affected.sh 0 1 0 0 0 0

Surviving mutants

Location Operator Disposition Cause
scripts/check-affected.sh:464 SBR productive no-assertion

The run's summary has the full report.

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