fix(ruff): pin the rule set so an upstream default change cannot break the repo

[kyle-sexton]

Closes #1972

Summary

The repo had no ruff configuration, so every ruff invocation in it linted
against whatever upstream's default selection happened to be that release. Ruff
0.16.0 moved that default from 59 rules to 413 and removed eighteen E/F
rules from it — "E401, E402, E701, E702, E703, E711, E712, E713, E714, E721,
E731, E741, E742, E743, F403, F405, F406, and F722"
(0.16.0 release notes).
That single change is behind both failures in #1972: the expansion brought the
TRY category into the default set, and the removals took E712 out of it.

Fix

• ruff.toml selects E4, E7, E9, and F — the pre-0.16 default set.
  The rule set is now a decision recorded in the repo rather than one inherited
  from upstream. It is deliberately a floor: raise it toward the managed policy
  with extend-select as each category is cleared.
  Registered as locally-owned for this repo in
  chore(distribution): record claude-code-plugins as a locally-owned ruff consumer standards#336 (merged), which is what keeps it from being a
  second unregistered lint policy.
• Eval fixtures are excluded. An audit skill's fixture holds the defect its
  eval asserts on, so linting it fails by construction. The comment records that
  exclusion governs traversal: Ruff's settings reference states "Typically,
  Ruff will lint any paths passed in directly, even if they would typically be
  excluded", and that force-exclude "will cause Ruff to respect these
  exclusions unequivocally". Every invocation here passes directories; the
  ruff-format hook passes a file explicitly and already sets --force-exclude.
  Verified both ways against the fixture path.
• ruff-format.test.sh case 4e selects E712 in its fixture instead of
  relying on it being a default rule. A case asserting on one rule's fix safety
  must name that rule; this is version-agnostic rather than re-pinned.
• Drops an unused import stat in python3_alias_probe.py — a real F401
  the pinned set surfaces (line 90 calls path.stat(), the method, not the
  module).

The TRY004 decision, recorded

#1972 asked for this on its merits rather than by config, so both halves:

• On merits: the rule does not fit these sites. All six raise on a decoded
  payload or a persisted state file failing an isinstance guard —
  babysit_gh.py:603,699,728 on a GraphQL response, babysit_lease.py:114,
  manage_feedback_ledger.py:176, and refresh_pr_branch.py:112 on state files
  this code wrote itself. None is a caller passing the wrong type to a function.

  That distinction is the whole argument, so it is worth being explicit that it
  is a judgment rather than something upstream states. TRY004's own rationale is
  "the Python documentation states that TypeError should be raised upon
  encountering an inappropriate type"; the Python docs' operative sentence is
  "Passing arguments of the wrong type … should result in a TypeError",
  which is a caller-contract rule, and they define RuntimeError as "an error
  is detected that doesn't fall in any of the other categories." The broader
  opening sentence ("an operation or function is applied to an object of
  inappropriate type") could be read to cover these; the reading taken here is
  that data-integrity validation of a payload is not argument passing. Anything
  catching RuntimeError at these sites would change behavior if they were
  retyped. No noqa is needed; the sites stand as written.

• Separately: TRY is not in the selected set, so they are not findings
  today regardless. Recorded so a future extend-select = ["TRY"] knows the
  category was examined and rejected on merits, not merely left unselected.

What the prefix selection does not buy

Review raised that E4/E7/E9/F are prefixes, so a later release adding a
rule inside one of those categories enables it here automatically — the original
wording of this PR overstated the invariant, and the file now says so at the
selection.

That exposure is real and is closed by version parity rather than by config. The
pin in .github/requirements-ci.txt is held equal to the fleet inventory
(melodic-software/dotfiles .chezmoidata/uv-tools.yaml), so a newly-added rule
cannot reach CI before a developer or the reverse; melodic-software/dotfiles#416
adds the check that keeps those two equal, and #1953 is the bump that makes them
equal today.

Enumerating the exact pre-0.16 codes was the alternative. It trades a reviewable
statement of intent for a list no reader can evaluate, freezes the policy against
genuinely useful new correctness rules inside F, and still needs the version
pin to be trustworthy — so it buys the appearance of the invariant rather than
the invariant.

Verification

• ruff check --no-cache --statistics plugins/ scripts/ → 0 findings
  (166 under bare 0.16.0 defaults before this change).
• bash plugins/ruff-format/hooks/ruff-format.test.sh → PASS=52 FAIL=0
  (was PASS=51 FAIL=1).
• bash plugins/source-control/skills/babysit-prs/scripts/engine.test.sh →
  exit 0; Ran 597 tests ... OK, ruff section clean (83 findings under bare
  0.16.0 defaults before this change).
• bash scripts/run-plugin-tests.sh → complete sweep of all 170 suites, exit 0,
  All plugin tests passed or were skipped. Those two are also the only suites
  that invoke ruff at all (grep -rl 'ruff check\|ruff format' --include=*.sh
  finds plugins/ruff-format/hooks/ruff-format.sh and engine.test.sh), so no
  other suite could be affected by a lint config.
• The ruff-format fixtures build their own repos under mktemp -d, which
  resolves outside the worktree, so they do not inherit this config — case 4e's
  pass is real evidence about the fixture's own select, not about this file.
• Local ruff is 0.16.0, matching the fleet pin in dotfiles
.chezmoidata/uv-tools.yaml, so these runs reproduce what a developer sees.
  CI still installs 0.15.22; chore(deps): bump ruff from 0.15.22 to 0.16.1 in /.github #1953 is what closes that half.

Related

• chore(distribution): record claude-code-plugins as a locally-owned ruff consumer standards#336 — registers the ruff component as
  locally-owned for this repo; closes Adopt the managed ruff component in claude-code-plugins so upstream default churn is not a repo-breaking event standards#335.
• Refs chore(deps): bump ruff from 0.15.22 to 0.16.1 in /.github #1953 — the Dependabot bump to ruff 0.16.0. This makes it safe against
  the failure that happened: with select explicit, the bump cannot change
  which CATEGORIES run.
• Refs fix(dev-env): resolve ruff from the declared pin instead of PATH #1871 — resolves ruff from the pin file rather than PATH; it reads the
  pin dynamically, so it needs no value change once chore(deps): bump ruff from 0.15.22 to 0.16.1 in /.github #1953 lands.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 2m 47s —— View job

---

Reviewing PR #1991

• Gather context (diff, changed files, CLAUDE.md guidelines)
• Review ruff.toml
• Review ruff-format.test.sh changes
• Review python3_alias_probe.py change
• Post final review

Summary

This is a solid, well-reasoned fix. Pinning select = ["E4", "E7", "E9", "F"] is exactly Ruff's own long-documented default rule set (not just this repo's ad-hoc guess), so the change genuinely restores prior behavior rather than inventing a new policy. I verified each specific claim in the PR body against the actual code:

• ruff-format.sh:214 does pass --force-exclude in RUFF_COMMON, backing the traversal-vs-direct-path exclusion comment in ruff.toml.
• Every ruff check invocation in the repo (engine.test.sh:35, ecosystems/python.yaml) passes a directory, not a file, consistent with the "traversal governs" reasoning.
• import stat in python3_alias_probe.py was genuinely dead — the only stat usage left is path.stat(), a Path method, and the test suite only mocks probe.Path.stat, never the stat module.
• Case 4e's lint.select = ["E712"] dotted-key fixture in ruff-format.test.sh:291-292 is valid TOML (equivalent to a [lint] table) and correctly targets the rule the case actually asserts on.
• No other ruff.toml/pyproject.toml [tool.ruff] files exist elsewhere in the repo, so there's no config-discovery conflict with this new root file.

One non-blocking note: the ruff.toml header comment leans on melodic-software/standards#336 being merged to keep this file from being clobbered as a second, unregistered lint policy on the next sync (per AGENTS.md's "synced standards are overwritten, not edited here" rule). I couldn't reach GitHub from this sandbox to confirm that PR's merged state — worth a final glance before merge, since if it's not actually registered as locally-owned, the next standards sync could silently overwrite this file.

No correctness, architecture, or test-coverage issues found. The TRY004 rationale is a documented judgment call rather than a config change, which is appropriately out of scope for this fix and doesn't need to be revisited here.

[claude]

Claude finished @kyle-sexton's task in 1m 15s —— View job

---

Security Review — complete

• Gather context (PR diff, changed files)
• Review ruff.toml
• Review plugins/ruff-format/hooks/ruff-format.test.sh
• Review plugins/disk-hygiene/skills/setup/scripts/python3_alias_probe.py
• Post findings

No security issues found.

This PR is a lint-policy change with no runtime security surface touched:

• ruff.toml (new) only pins select = ["E4","E7","E9","F"] and adds extend-exclude = ["**/evals/fixtures/**"]. No S (bandit-style) security rules were selected before this PR either — Ruff never auto-selects that category, in any default set, pre- or post-0.16.0 — so this pin doesn't remove security-lint coverage that existed. The exclude only governs directory traversal for a fixture directory that intentionally holds known-defective specimens for eval assertions; it doesn't touch how any hook invokes Ruff (--force-exclude behavior for explicitly-passed paths is unchanged code, only newly documented in a comment).
• ruff-format.test.sh changes are confined to a synthetic, throwaway test fixture (lint.select = ["E712"] written into a mktemp -d repo). All inputs are static literals authored in the test, not derived from user/network/CI input — no injection surface.
• python3_alias_probe.py drops an unused import stat (dead code per F401); no behavioral change.

No workflow files, permissions, secrets/token handling, or trust boundaries are touched by this PR, so there's nothing in scope for the injection/authz/GitHub-Actions checks either.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60d6b3d731

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @kyle-sexton's task in 2m 23s —— View job

---

Reviewing PR #1991

• Gather context (diff, changed files, CLAUDE.md/AGENTS.md guidelines)
• Review ruff.toml (including the doc update responding to the Codex comment)
• Review ruff-format.test.sh changes
• Review python3_alias_probe.py change
• Verify claims against the actual repo (exclude glob, CI invocations, version pins)
• Post final review

Summary

This is a well-scoped, well-documented fix. select = ["E4", "E7", "E9", "F"] is Ruff's own pre-0.16 default set, so the change restores prior behavior rather than inventing a new policy, and the last commit (ea5a9c2) already responds directly to Codex's P2 comment about prefixes vs. enumerated codes — it doesn't change the code, but adds an honest paragraph in ruff.toml:14-23 stating exactly what prefix-selection does not buy (immunity to a future rule landing inside an already-selected category) and why enumeration was rejected as the fix for that gap (an unreadable list, that still needs a version pin anyway). That's a reasonable judgment call, correctly labeled as a tradeoff rather than asserted as a complete fix.

I independently verified the specific claims backing that tradeoff and the rest of the PR body:

• Eval fixtures genuinely need the exclude: plugins/code-tidying/skills/audit-comment-residue/evals/fixtures/residue-snippet.py is a real .py fixture that extend-exclude = ["**/evals/fixtures/**"] (ruff.toml:36) would correctly cover — not a no-op.
• "Traversal governs" is accurate for every current caller: engine.test.sh:35 runs ruff check . tests, and plugins/toolchain/reference/ecosystems/python.yaml / docs/conventions/ecosystem-commands/examples/python.yaml both run ruff check . — all directory args, consistent with the comment at ruff.toml:30-35.
• The import stat removal is a real dead import: neither python3_alias_probe.py nor test_python3_alias_probe.py references the stat module anywhere; the only .stat() use is Path.stat() at line 89. Confirmed [F401] is legitimate, not a false positive.
• Case 4e's fixture (ruff-format.test.sh:290-292) selecting E712 explicitly is the right fix — it makes the case assert on the rule it actually exercises instead of depending on Ruff's shifting default set, and the updated comment explains why.

One documentation-accuracy note (non-blocking): ruff.toml:18-21 states in the present tense that "the pin in .github/requirements-ci.txt is held equal to the fleet inventory... so a new rule cannot reach CI before a developer, or the reverse." That invariant doesn't hold yet: .github/requirements-ci.txt currently pins ruff==0.15.22, while the PR body itself says the local/fleet pin (dotfiles .chezmoidata/uv-tools.yaml) is already 0.16.0 — and names #1953 as the still-open PR that closes that exact gap. The comment reads as describing a currently-closed loop, but per the PR's own "Refs #1953" line, it's aspirational until that lands. Worth either softening the tense ("is meant to be held equal" / "once #1953 lands") or confirming #1953 merges before/alongside this, so the recorded rationale doesn't misstate present state to a future reader. This doesn't affect correctness of select today (explicit select makes the two Ruff versions behave identically for the pinned prefixes regardless), only the accuracy of the safety argument documented alongside it.

No other correctness, error-handling, or test-coverage issues found. The TRY004 write-up is a documented judgment call correctly kept out of the config (since TRY isn't selected), which is appropriately out of scope for a lint-pin fix.

[claude]

Claude finished @kyle-sexton's task in 1m 17s —— View job

---

Security Review — complete

• Gather context (diff since last review, changed files)
• Review ruff.toml for security-relevant config changes
• Review plugins/ruff-format/hooks/ruff-format.test.sh changes
• Review plugins/disk-hygiene/skills/setup/scripts/python3_alias_probe.py change
• Post findings

No security issues found.

This synchronize event only added two documentation commits (60d6b3d, ea5a9c2) on top of the previously-reviewed b0f71ed — both are comment-only edits to ruff.toml explaining the prefix-vs-enumeration tradeoff (responding to the Codex P2 comment) and updating a doc URL. No functional or executable content changed since the last security pass, and the conclusion is unchanged:

• ruff.toml (new) only pins select = ["E4","E7","E9","F"] (L38-L44) and adds extend-exclude = ["**/evals/fixtures/**"] (L36). No S (bandit-style) security-rule category was selected before this PR either — Ruff never auto-selects it in any default set, pre- or post-0.16.0 — so this pin removes no security-lint coverage. The exclude only affects directory-traversal scope for a fixture directory holding intentionally-defective eval specimens; it doesn't change how any hook invokes Ruff (--force-exclude behavior for explicitly-passed paths is pre-existing code, only newly documented here).
• ruff-format.test.sh changes are confined to a synthetic, throwaway test fixture (lint.select = ["E712"] written into a mktemp -d repo). All inputs are static literals authored in the test, not derived from user/network/CI input — no injection surface.
• python3_alias_probe.py drops an unused import stat (dead code per F401); no behavioral change.

No workflow files, permissions, secrets/token handling, or trust boundaries are touched by this PR, so there's nothing in scope for the injection/authz/GitHub-Actions checks either.