Skip to content

fix(ci): repair the two quality gates that #88 exposed - #90

Merged
hyperpolymath merged 2 commits into
mainfrom
fix/quality-gates-license-semgrep
Aug 3, 2026
Merged

fix(ci): repair the two quality gates that #88 exposed#90
hyperpolymath merged 2 commits into
mainfrom
fix/quality-gates-license-semgrep

Conversation

@hyperpolymath

Copy link
Copy Markdown
Owner

Follow-up to #88. Making comprehensive-quality.yml parse turned 0 jobs into
11 — and two of those jobs then failed for real. They are genuine defects in
gates that had never once executed, not regressions from #88.

Both are still present on main today.

1. license — the glob matched a directory

head -5 LICENSE* also matches the LICENSES/ directory. head exits 1 on
a directory, and 2>/dev/null suppresses the message but not the exit code,
so the step died under bash -e immediately after printing "License file present".

Reproduced locally:

$ bash -e -c 'head -5 LICENSE* 2>/dev/null'; echo "exit=$?"
==> LICENSE <== ... ==> LICENSES <==
exit=1

Same family as the target/release/* trap: a glob silently matching a directory.

A trap inside the fix, worth recording. The obvious repair —
[ -f "$f" ] && head -5 "$f" in a loop — is also broken: as the last statement
in the loop body, a false test makes the whole step exit 1 under bash -e. I hit
exactly that and only caught it because I ran the form before pushing:

$ bash -e -c 'for f in LICENSE LICENSE.txt LICENSE.md; do [ -f "$f" ] && head -5 "$f"; done'; echo "exit=$?"
exit=1          # <- still broken
$ bash -e -c 'for f in LICENSE LICENSE.txt LICENSE.md; do if [ -f "$f" ]; then head -5 "$f"; fi; done'; echo "exit=$?"
exit=0          # <- correct

Shipped with the explicit if.

2. security — the semgrep pin did not exist

##[error]Unable to resolve action `returntocorp/semgrep-action@73f32468...`,
unable to find version `73f32468...`

The repo is alive (last push 2024-01-22), so this was an invented SHA, not
a dead action repo — the same class as the estate's ~80 invented pins.

Re-pinned to v1 = 713efdd345f3035192eaa63f56867b88e63e4e5d, taken from the
tags API and confirmed resolvable via repos/.../commits/<sha>. I did not guess.

Worth recording: an unresolvable uses: ref fails during action
resolution, before the step body runs — so the continue-on-error: true on
that step could not and did not rescue it. continue-on-error never protects
against a bad pin.

Caveat, deliberately not hidden: this action is deprecated (last push
2024-01) and the step is non-blocking, so SAST here is advisory only. Fixing the
pin makes it resolve; it does not make it a real gate. Replacing it with a
maintained scanner is a separate call and I have not made it for you.

Verification

  • actionlint0 findings on the file.
  • Both shell forms exercised locally under bash -e (outputs above).
  • New semgrep SHA confirmed to resolve via the API.

Note on scope

Test on Racket current also failed on #88's branch (30m14s, a hang). That is
test.yml, untouched here, and it passes consistently on main — so it is not
caused by this change. Flagging it, not fixing it blind.

🤖 Generated with Claude Code

Making the file parse (previous commit) revealed two REAL failures in
jobs that had never once executed in this repo's history. Both are
genuine defects, not regressions.

1. `license` job — glob matched a DIRECTORY.

   `head -5 LICENSE*` also matches the LICENSES/ directory. `head` exits
   1 on a directory, and `2>/dev/null` hides the message but NOT the
   exit code, so the step died under `bash -e` immediately after
   printing "License file present".

   Reproduced locally:
     $ bash -e -c 'head -5 LICENSE* 2>/dev/null'; echo $?
     ==> LICENSE <== ... ==> LICENSES <== ... 1

   Same family as the `target/release/*` glob trap: a glob silently
   matching a directory.

   NOTE: the obvious fix `[ -f "$f" ] && head -5 "$f"` inside a loop is
   ALSO broken — as the last statement, a false test makes the step exit
   1 under `bash -e`. Verified, then replaced with an explicit `if`.
   Now exits 0.

2. `security` job — the semgrep pin did not exist.

   `returntocorp/semgrep-action@73f3246...` was unresolvable:
     ##[error]Unable to resolve action ... unable to find version 73f3246...

   The repo IS alive (last push 2024-01-22), so this was an INVENTED
   SHA, not a dead action repo. Re-pinned to v1 = 713efdd..., verified
   against the tags API and confirmed resolvable.

   Worth recording: an unresolvable `uses:` ref fails during action
   RESOLUTION, before the step body runs, so the `continue-on-error:
   true` on that step could not and did not rescue it.

   The action is deprecated and the step is non-blocking, so SAST here
   is advisory only. Flagged in the PR rather than silently swapped for
   a different scanner.

Verified: actionlint reports 0 findings on this file; both shell forms
exercised locally under `bash -e`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

Note

Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime.
Learn more

Code Review ✅ Approved

CI workflow quality gate fixes correcting the LICENSE glob directory check and updating the deprecated semgrep action pin. No issues found.

Auto-approved and auto-merge armed: No blocking issues found.
Please see Auto-approve Docs for details on setting custom approval criteria. — merges when pipeline and required approvals pass.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Important

Your trial ends in 7 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

@gitar-bot
gitar-bot Bot enabled auto-merge (squash) August 3, 2026 12:45

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitar has auto-approved this PR and enabled auto-merge (configure)

@gitar-bot gitar-bot Bot added the gitar-approved Added by Gitar label Aug 3, 2026
The previous runs on this branch were all startup_failure — repo-level, not
caused by this change. Root cause: allowed_actions=selected with an EMPTY
patterns_allowed, which rejects every non-github-owned uses: at parse time.

Repaired via gitbot-fleet fix-actions-policy.sh (non-widening path):
allowed_actions stays 'selected', sha_pinning_required stays true, only the
empty patterns_allowed was populated (0 -> 118, mirroring standards).

startup_failure runs cannot be retried, so this empty commit re-triggers CI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hyperpolymath
hyperpolymath disabled auto-merge August 3, 2026 13:15
@hyperpolymath
hyperpolymath merged commit df52730 into main Aug 3, 2026
@hyperpolymath
hyperpolymath deleted the fix/quality-gates-license-semgrep branch August 3, 2026 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gitar-approved Added by Gitar

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant