Repository navigation
fix(rules): restore the RE001 structural detectors deleted by #773 - #827
Merged
Merged
Conversation
) PR #773 described itself as "documentation-only" and stated that validation was not run. Its measured diff is 10 insertions and 104 deletions: it removed eight private helpers and replaced the RE001 job analysis with two regexes over raw job text. That broke the rule in BOTH directions at once: * false positive — `uses: "step-security/harden-runner@..."` is valid YAML and does install the hardener, but the regex requires an unquoted value, so RE001 fired against jobs that were already hardened; * false negative — the same string inside a `run: |` or `env: >` block scalar is shell or data, not a step, but the regex matched it and suppressed a genuine finding. Two of the deleted helpers are named for the exact fixes whose tests now fail: `harden_runner_uses?/1` (ab1f855) and `block_scalar?/1` (e047d46). Restores `direct_job_field?/2`, `harden_runner_step?/1`, `job_field_indent/1`, `lines_in_steps/2`, `step_indent/2`, `harden_runner_uses?/1` and `block_scalar?/1`, and re-points the two detectors at them. The @doc is kept and genuinely expanded — which is what #773 claimed to do but did not; it reworded the prose slightly while deleting the logic, and dropped the "without a direct `runs-on` field" precision that `direct_job_field?/2` implements. The new section documents why the check is structural rather than line-oriented, naming both failure directions. Mutants (a passing suite proves nothing until one dies) — every red below is an assertion failure, not a compile error: 1. harden detection reverted to the #773 regex -> BOTH tests red (quoted-uses false positive AND block-scalar false negative) 2. runs-on detection reverted to the #773 regex -> :120 red, reporting a finding at line 8 for a `runs-on` nested inside an `env:` block scalar 3. `block_scalar?/1` neutered to always-false -> :120 red Clean control: 1593 tests, 0 failures. Secondary defect, same issue: tests/e2e.sh printed `tail -120` on failure. ExUnit prints failure diagnostics INLINE as each test fails, not collected at the end, so at ~1600 tests under --trace no fixed tail window can contain one. Measured here: the failing run's log is 4819 lines and `tail -120` contains zero failure blocks — which is why five consecutive red CI runs were undiagnosable and this had to be reproduced locally. The harness now selects failure blocks by content (from the first `N) test` to the end, capped), and falls back to a tail only when no failure block exists, which means the run never reached the tests. Closes #826 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0113HQM9LVGkNCzU1WwkJZSV
Contributor
|
Warning Review limit reachedNext included review available in 22 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 |
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
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.
Closes #826.
What happened
PR #773 says in its own body: "Expand the RE001 rule documentation… Validation was not run; this is a documentation-only change."
Its measured diff is 10 insertions, 104 deletions — only 4 of the inserted lines are prose. It deleted eight private helpers and replaced the RE001 job analysis with two regexes over raw job text.
The tell is that two of the deleted helpers are named for the exact fixes whose tests now fail:
harden_runner_uses?/1(fromab1f855) andblock_scalar?/1(frome047d46).A regex that replaces a parser breaks in both directions at once
- uses: "step-security/harden-runner@main"run: |orenv: >The
runs-onhalf had the same shape:~r/^\s+runs-on:/mmatches aruns-onnested inside anenv:block scalar, and it also cannot tell a job's own field from a sibling's — which is precisely whatdirect_job_field?/2exists to decide, since a reusable job delegates its runtime to the source workflow.The fix
Restores
direct_job_field?/2,harden_runner_step?/1,job_field_indent/1,lines_in_steps/2,step_indent/2,harden_runner_uses?/1andblock_scalar?/1, and re-points the two detectors at them.The
@docis kept and genuinely expanded — which is what #773 claimed to do but did not. It reworded the prose slightly while deleting the logic, and in doing so dropped the "without a directruns-onfield" precision thatdirect_job_field?/2implements. The new section documents why the check is structural rather than line-oriented, naming both failure directions so the next person is less likely to "simplify" it back into a regex.Mutants — a passing suite proves nothing until one dies
Every red below was verified to be an assertion failure, not a compile error:
runs-ondetection reverted to the #773 regex:120red, reporting a finding at line 8 for aruns-onnested inside anenv:block scalarblock_scalar?/1neutered to always-false:120redClean control: 1593 tests, 0 failures. The suite is green for the first time since 2026-09-12.
Secondary defect — why five red CI runs were undiagnosable
tests/e2e.shprintedtail -120of the run on failure. ExUnit prints failure diagnostics inline, as each test fails — it does not collect them at the end. At ~1600 tests under--tracethe log is dominated by two trace lines per test, so no fixed tail window can contain a failure block.Measured directly here: the failing run's log is 4819 lines, and
tail -120contains zero failure blocks. That is why the reds could not be read from CI at all and had to be reproduced locally — and why the failing tests were initially misidentified from passing trace lines that happened to land inside the window.The harness now selects failure blocks by content, not by position — from the first
N) test …to the end, capped at 500 lines — and falls back to a tail only when there is no failure block, which means the run never reached the tests (a compile error or a startup crash), where the tail is the informative part.HYPATIA_KEEP_MIX_LOG=1retains the full log.Verification
mix test→ 1593 tests, 0 failures, 242 excludedmix format --check-formatted→ cleanbash -n tests/e2e.sh→ syntax OKactive_content, and the vaguer prose🤖 Generated with Claude Code
https://claude.ai/code/session_0113HQM9LVGkNCzU1WwkJZSV