fix(rules): escape / in three regex sigils so hypatia compiles again - #876
Closed
hyperpolymath wants to merge 2 commits into
Closed
hyperpolymath wants to merge 2 commits into
hyperpolymath wants to merge 2 commits into
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
Contributor
|
Warning Review limit reachedNext included review available in 50 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 (2)
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:47
hyperpolymath
added a commit
that referenced
this pull request
Sep 30, 2026
…cl. poisoned pin classified arm_auto) (#877) 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 `PinIntegrity` and `PrAutomerge` never compiled on `main` (#862), so their 17 tests had never executed. Once #876 lands they run and fail. **Nothing outside `test/` calls either module**, so no live behaviour was wrong; these are fixed before anything wires them in. ## Fixes **PinIntegrity** - `claimed_version/1` — `Regex.run` drops trailing unmatched groups; a `v`-led match returned one element that neither clause matched → `nil`. This cascaded into PI002 (mislabel) and PI005 (stale) never firing. - `relabel_line/2` produced `##` (prefixed `#` onto `relabel/2`'s already-`#`-led result). Now mirrors `estate-pin-integrity.sh` exactly: head without `#`, comment handed over with it. - `relabel/2` restores `# ` for the `#`-stripped shape `pin_sites/1` emits (`v3`). `#v3` and `""` keep the shell mirror's byte-identical output. **PrAutomerge** - `pin_deltas/3` returned a bare map from a `flat_map` callback → every delta flattened into `{key, value}` tuples. - `verdict/3` discarded the scan → decisions lacked `deltas` / `licence_touch` / `pin_only`. - ⚠ **Safety:** `close_poison_only` went through `accept/4`, so **a pin-only PR onto a denylisted pin was classified `safety: "arm_auto"`**. Now `reject/4`. Mutant control: reverting this line alone turns exactly `a pin-only change onto the denylist is closed, not merged` red. - `decision_manifest/2` vetoes 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-only` carried 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 asserts `refute pin_lines_only?`. ## Verified - `mix test`: **1673 tests, 0 failures** (was 17 failures on #876). - `mix compile --warnings-as-errors`: 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>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
hyperpolymath
disabled auto-merge
September 30, 2026 10:08
Owner
Author
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>
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.
What broke
#862 (merged 2026-09-26) placed an unescaped
/inside a character class in three~r/.../sigils:lib/rules/pin_integrity.ex—@uses_regexand the regex inlocked_refs/1lib/rules/pr_automerge.ex—@pin_reIn a
~r/sigil that/ends the sigil, somix compilefails (MismatchedDelimiterError).hypatia-scan-reusable.ymlbuilds hypatia atmain, so every estate Hypatia scan has failed since 09-26.Fix
[A-Za-z0-9_./-]→[A-Za-z0-9_.\/-]. Inside a character class\/matches the same bytes as/, so this only restores compilation.verify_action_shas.exhas the same class but uses~r|…|, so it was always legal.Verified
mix compile→ 1 compilation error. With it: compiles.mix test: 1673 tests, 17 failures — all intest/rules/{pin_integrity,pr_automerge}_test.exs, which have never run before because their modules never compiled.Known residual (follow-up PR)
Those 17 are latent logic bugs in #862, not caused by this escape. Examples:
Regex.rundrops trailing unmatched groups, soclaimed_version("v4.38.0")→nil;{:status, :ok}tuples read as maps; missing:licence_touch/:deltaskeys. Nothing outsidetest/calls either module, so compiling them turns on no behaviour. They are fixed in a separate PR to keep this outage fix at three characters.tests.yml(which runs those two files) will stay red until that lands; it is not a required check.🤖 Generated with Claude Code
https://claude.ai/code/session_0136eszqrQ53Kj7aBH1D4rXK