fix(ci): move hashFiles() out of job-level if: — the last red workflow - #88
Merged
Merged
Conversation
The duplicate `permissions:` key was necessary to fix but NOT sufficient: comprehensive-quality.yml still produced a run with ZERO jobs and no check run on the previous commit, i.e. it was still rejected at parse time. Second, independent defect, confirmed with actionlint: .github/workflows/comprehensive-quality.yml:156:9: calling function "hashFiles" is not allowed here. "hashFiles" is only available in "jobs.<job_id>.steps.*" contexts `hashFiles()` in a job-level `if:` makes Actions reject the WHOLE FILE, so none of the 11 jobs ever materialised. It was also semantically pointless there: at job level nothing is checked out yet, so the glob could never match. Moved the guard onto the two steps that actually need it, where it runs after checkout and is a valid context. Verified: `actionlint .github/workflows/*.yml` now reports zero expression or syntax errors across all 19 files; the only remaining output is shellcheck style/info (SC2086, SC2129, SC2035, ...) in unrelated workflows. Lesson for the estate: a strict YAML duplicate-key scan is NOT sufficient to prove an Actions workflow parses. YAML validity and Actions validity are different things — use actionlint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hyperpolymath
added a commit
that referenced
this pull request
Aug 3, 2026
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: ```console $ 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: ```console $ 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 - `actionlint` → **0 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](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 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.
The final red workflow on
main. Everything else is green as of29abb4c.Why #87 did not finish the job
#87 fixed a genuine defect — a duplicate top-level
permissions:key incomprehensive-quality.yml— but that was necessary, not sufficient. The filewas rejected at parse time for a second, independent reason, so it still
produced runs with zero jobs and no check run afterwards.
(#87 was squash-merged just before the follow-up commit landed on its branch,
so that second fix never reached
main. This PR carries it.)The remaining defect
hashFiles()sat in a job-levelif:. That is not a permitted context andit makes Actions reject the entire file — which is why none of the 11 jobs
ever materialised, and why the workflow still shows in the UI as the raw path
.github/workflows/comprehensive-quality.ymlrather than its name(
Comprehensive Quality Gates): GitHub has never parsed far enough to read it.It was also meaningless where it was — at job level nothing is checked out yet,
so the glob could never match. The guard now sits on the two steps that need it,
evaluating after checkout, which is both valid and actually correct.
Verification
actionlint .github/workflows/comprehensive-quality.yml→ 0 findings(was 1 fatal expression error). Across all workflows, the only remaining
actionlint output is shellcheck style/info in unrelated files.
Expect this run to produce 11 jobs. Job count is the discriminator: zero
jobs means still parse-dead, non-zero means it parses and any failure is a real
quality-gate result to triage on its merits.
Method note worth keeping
A strict YAML duplicate-key scan is not sufficient to prove an Actions
workflow parses — YAML validity and Actions validity are different things, and
my earlier YAML-only check passed this file while GitHub still rejected it.
actionlintmodels the real rules and catches this class immediately. Given theestate tracks ~187 unparseable workflow files, it is the right instrument for
that sweep.
Context:
dev-notes/betlang-sitrep-2026-07-27.md.🤖 Generated with Claude Code