fix(rules): latent logic bugs #862 shipped behind a compile error (incl. poisoned pin classified arm_auto) - #877
Merged
Conversation
#862 put an unescaped `/` inside a character class in three `~r/.../` sigils (pin_integrity.ex @uses_regex and locked_refs/1, pr_automerge.ex @pin_re). In a `~r/` sigil that `/` terminates the sigil, so `mix compile` fails with MismatchedDelimiterError. Every estate Hypatia scan (hypatia-scan-reusable.yml resolves hypatia@main) has failed since. `\/` inside a character class matches the same byte set as `/`; this change is semantics-preserving and only restores compilation. The two modules' own tests (17) now run for the first time and fail on latent logic bugs in #862; nothing outside test/ calls either module. Those are fixed in a follow-up PR, kept separate so this outage fix stays three characters. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK
PinIntegrity and PrAutomerge never compiled on main, so their 17 tests had never run. With #876 they run; this fixes what they found. PinIntegrity - claimed_version/1: Regex.run drops trailing unmatched groups, so a `v`-led match returned a 1-element list neither clause matched -> nil. That cascaded into pi002 and pi005 never firing. - relabel_line/2 prepended `#` to relabel/2's already-`#`-led result (`##`). Now mirrors estate-pin-integrity.sh: head without `#`, comment handed over with it. - relabel/2 restores `# ` for the `#`-stripped comment pin_sites/1 emits; `#v3` and "" keep the shell mirror's byte-identical output. PrAutomerge - pin_deltas/3 returned a bare map from a flat_map callback, so every delta was flattened into {key, value} tuples. - verdict/3 dropped the scan, so decisions lacked deltas/licence_touch. - SAFETY: close_poison_only went through accept/4, i.e. a pin-only PR onto a DENYLISTED pin was classified safety=arm_auto. Now reject/4. Mutant control: reverting this alone turns exactly that test red. - decision_manifest/2 vetoes use string keys like the rest of the manifest (the schema-shaped output). Test fixture: "a patch that also edits a permissions block" carried the permissions lines as diff CONTEXT (leading space) under a mismatched hunk header, so it asserted an edit the diff did not contain. The code was right; the fixture now makes the edit real (-/+ lines). mix test: 1673 tests, 0 failures. mix compile --warnings-as-errors: clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK
Contributor
|
Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
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 |
hyperpolymath
enabled auto-merge (squash)
September 30, 2026 09:50
hyperpolymath
disabled auto-merge
September 30, 2026 10:00
This was referenced Sep 30, 2026
hyperpolymath
added a commit
that referenced
this pull request
Sep 30, 2026
… red since #820) (#880) ## What The required check `abi-codegen-drift` has failed on every uncached run since #820. Main was red on 09-27, 09-28 and 09-29, and #876/#877 are both stuck on it. The cause: ``` /home/runner/work/_temp/….sh: line 10: idris2: command not found ##[error]Process completed with exit code 127. ``` #820 moved `make install` to `PREFIX="$HOME/.idris2"`, but the same step still ends with a bare `idris2 --version`. `$HOME/.idris2/bin` only joins `$GITHUB_PATH` in the **next** step. The fix calls the binary by its full path. `verify-proofs.yml` has the identical line and gets the same one-line fix. ## Why it matters `abi-codegen-drift` is one of main's two required contexts, so while it is red no hypatia PR can merge. That includes #876, which restores compilation after #862. Every Hypatia scan in the estate is failing until #876 lands. ## Verification This PR's own `abi-codegen-drift` run is the control. It either gets past `Build Idris 2 from source` or it doesn't. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
hyperpolymath
added a commit
that referenced
this pull request
Sep 30, 2026
…us + inline directives honoured (#881) > **Stacked on #877** (itself on #876). The base retargets to `main` once #877 lands. Squash automerge will be armed then, not before: armed now, it would merge into #877's branch. ## Why A Hypatia `hardcoded_tmp` alert on launch-scaffolder#46 exposed a real CWE-377 defect in the launcher generator. That was fixed upstream in launch-scaffolder #54/#58/#62. The rule itself was part of the problem: | Input | Before | After | |---|---|---| | `PID_FILE="/tmp/x.pid"` | fires | **fires** (positive control) | | `d=$(mktemp -d /tmp/foo.XXXXXX)`, the rule's own advice | **fires** | clean | | `# comment mentioning "/tmp/"` | **fires** | clean | | `/tmp` line under `tests/fixtures/`, `test/`, `lib/rules/`, `scripts/fix-scripts/` | **fires** | suppressed | | `# hypatia: allow content_patterns/hardcoded_tmp -- …` | **silently ignored** | honoured | ## Changes - **Remediation text.** It now separates pid/state files, which the next run must re-find (per-user `${XDG_RUNTIME_DIR:-${XDG_STATE_HOME:-$HOME/.local/state}}/<app>/`), from scratch files (`mktemp -d` with no `/tmp` template, plus `trap … EXIT`). The old text said "use mktemp", which is the wrong cure for a pid file. - **Own-cure skip.** `skip_comment_lines: true`, plus a new generic rule key `skip_if_line_matches` (here `~r/\bmktemp\b/`). - **Training-corpus exemption.** Added under `"content_patterns"`, the key the findings are **emitted** under in `cli.ex`. A `"cicd_rules"` key would be vacuous. - **Inline directives.** `hypatia: allow <module>/<rule>` is now honoured by the content engine under either module spelling. Rule ids are atoms, so they are stringified before `inline_allowed?/4`. - **Line-1 fix.** Line 1 no longer treats the file's **last** line as its "previous line" (`Enum.at(lines, -1)` wraps around). - **Docs.** The `.hypatia-ignore` header and `.claude/CLAUDE.md` now document the two-spelling trap and the directive behaviour. ## Not changed, deliberately - `applies_to` stays `["*.sh"]`. Widening it to `*.rs` would flag `#[cfg(test)]` literals under `src/`, and cicd_rules has no Rust comment stripping. - The file-level `hypatia: allow` form is still not wired for content patterns. CLAUDE.md now says so. ## Trade-off, stated plainly After this PR, fixtures under `tests/` are no longer scanned for this rule. On the launch-scaffolder side, the generator's own literal-pinned tests (`DEFAULT_PID_LINE`) are now the detector for a `/tmp` regression. ## Verification - `test/hardcoded_tmp_pipeline_test.exs` has 16 tests running the **real** pipeline (`collect_findings` → normalise → suppress), never a hand-built finding. - Four mutants were each killed: | Mutant | Red tests | |---|---| | Exemption key renamed | 4 | | mktemp skip removed | 1 | | `n > 1` guard reverted | 1 | | `inline_allowed?` clause removed | 3 | - Full suite: 1689 tests, 0 failures. `mix compile --warnings-as-errors --force` is clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
hyperpolymath
added a commit
that referenced
this pull request
Sep 30, 2026
…in (#882) `main` fails `mix compile --force --warnings-as-errors` (the mode used by `tests.yml:109` and `escript-soundness.yml:141`): ``` warning: variable "new" is unused 248 │ defp delta(old, new, from, to, status, source) do ``` **Why it is on main:** #877 was squash-merged at 2026-09-30T10:00Z with commits `09c6b16` + `e6419d4` only. This fix (`76b8c89`) was pushed to the same branch but did not make the squash, so `main` (d369a80) carries the warning. **Change:** `new` → `_new`. One token; no behaviour change. **Verified:** forced strict compile on this head → rc=0; on `origin/main` → rc=1. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #876 (the 3-character compile fix, included here as its first commit; it drops out of the diff once #876 lands).
Why these were never caught
PinIntegrityandPrAutomergenever compiled onmain(#862), so their 17 tests had never executed. Once #876 lands they run and fail. Nothing outsidetest/calls either module, so no live behaviour was wrong; these are fixed before anything wires them in.Fixes
PinIntegrity
claimed_version/1—Regex.rundrops trailing unmatched groups; av-led match returned one element that neither clause matched →nil. This cascaded into PI002 (mislabel) and PI005 (stale) never firing.relabel_line/2produced##(prefixed#ontorelabel/2's already-#-led result). Now mirrorsestate-pin-integrity.shexactly: head without#, comment handed over with it.relabel/2restores#for the#-stripped shapepin_sites/1emits (v3).#v3and""keep the shell mirror's byte-identical output.PrAutomerge
pin_deltas/3returned a bare map from aflat_mapcallback → every delta flattened into{key, value}tuples.verdict/3discarded the scan → decisions lackeddeltas/licence_touch/pin_only.close_poison_onlywent throughaccept/4, so a pin-only PR onto a denylisted pin was classifiedsafety: "arm_auto". Nowreject/4. Mutant control: reverting this line alone turns exactlya pin-only change onto the denylist is closed, not mergedred.decision_manifest/2vetoes now use string keys, like the rest of the schema-shaped manifest.One test fixture changed — please review this one.
a patch that also edits a permissions block is not pin-onlycarried its permissions lines as diff context (leading space) under a mismatched@@ -1,4 +1,4 @@, so it asserted an edit the diff did not contain. The code was correct; the fixture now makes the edit real (-/+lines,@@ -1,2 +1,3 @@). The test still assertsrefute pin_lines_only?.Verified
mix test: 1673 tests, 0 failures (was 17 failures on fix(rules): escape / in three regex sigils so hypatia compiles again #876).mix compile --warnings-as-errors: clean.🤖 Generated with Claude Code
https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK