ci: lint the workflow files so an uncompilable one cannot ship (#1122)

[frankbria]

Closes #1122.

Why this class of bug is invisible

An uncompilable workflow does not fail like a normal job. The run has zero
jobs and no logs, gh run view --log-failed returns log not found, and the
invalid run ignores its own trigger filters — so it surfaces on branches the
workflow should never touch and reads as an unrelated trigger bug rather than an
outage. deploy.yml failed that way on every push to main for a week; nobody
noticed staging had stopped deploying. claude-review (#1011) was the same shape.

YAML validity does not catch it: yaml.safe_load() parses these files fine. It
is GitHub's expression pass that rejects them.

Changes

workflow-lint job — runs actionlint on every PR.

Deliberately not paths-filtered. A required check that reports "skipped"
exactly when a workflow changed is the same silence this job exists to remove,
and actionlint takes about a second.

Wired into test-summary twice — into needs, and into the exit-1
condition. Being in needs alone is not a gate: test-summary runs under
if: always(), so the failure branch is what makes it real. This is the
CLAUDE.md rule about gates that quietly stop gating.

actionlint is a pinned release verified by sha256, not a floating action —
this job's whole purpose is stopping unverifiable CI config from landing, so it
should not itself pull an unpinned artifact.

CLAUDE.md records the diagnostic, since the symptom is the hard part: zero
jobs and no logs means the file will not compile; run actionlint first.

Evidence — verified against the actual regression

actionlint on deploy.yml as it stood at a0a69636:

.github/workflows/deploy.yml:570:135: unexpected end of input while parsing
variable access, function call, null, bool, int, float or string.
expecting "IDENT", "(", "INTEGER", "FLOAT", "STRING" [expression]
exit=1

That is the empty ${{ }} that caused the week-long outage — found in under a
second. Current main is clean under the checks this job enforces
(actionlint -shellcheck=, exit=0), including this PR's own additions.

Correction: the first version of this PR failed its own job

My initial local verification was invalid. actionlint runs shellcheck over
every run: block whenever shellcheck is on PATH — it is on GitHub runners,
it was not on my machine, so that entire check class silently vanished locally
and the run looked clean. The job failed on its first CI run.

Reproduced after installing shellcheck locally:

actionlint              # 44 findings — every one of them [shellcheck]
actionlint -shellcheck= # clean

Zero non-shellcheck findings: the workflows compile. The job therefore runs
-shellcheck=, which keeps exactly the failure mode #1122 is about, and the 44
shell findings are tracked in #1130 (P2.34) rather than fixed here — they are
pre-existing, almost all in deploy.yml, and quoting changes in the deploy path
cannot be validated from a PR. Deploying is the only way to know an SSH heredoc
still behaves, and changing it blind is how the original outage happened.

Two tests pin that the suppression stays documented and stays narrow, so it
cannot quietly become permanent.

The wiring tests are verified non-tautological

tests/test_workflow_lint_wiring_1122.py pins that the job exists, is in
needs, is in the exit-1 condition, is not conditionally skipped, and is
checksum-pinned.

I removed each half in turn to confirm they actually fail. That caught a weak
test of my own: test_test_summary_actually_fails_on_it originally searched for
needs.workflow-lint.result anywhere in the script, which passed with the gate
removed because the summary table row also interpolates that value. It now
asserts the exact exit-1 condition.

Acceptance criteria

• A CI job runs actionlint over .github/workflows/*.yml and fails on findings
• Wired into test-summary so it cannot be skipped silently
• Runs on PRs that change workflow files (runs on all PRs, a superset)
• Verified against a0a69636 — reproduced the exact finding
• Note in CLAUDE.md: zero jobs + no logs means it will not compile

Not done here — deliberately

The issue's "worth considering in the same change" item — notifying when a
valid workflow starts failing on main — is not in this PR. Linting stops a
broken workflow from landing; it does nothing for a workflow that compiles and
then fails, which is the other half of why staging was down for a week. That
needs a decision about where notifications go (the #560 webhook plumbing exists,
but pointing it at CI failures is a new policy, not a refactor). The issue itself
offers to split it out, so I have: say the word and I will file it as its own
P-numbered issue rather than guessing at the routing.

[github-actions]

GLM precision review — done

1 confirmed defect (1 major), posted as an inline comment + a summary comment.

• major — .github/workflows/test.yml:98: the workflow-lint job already fails on this PR. actionlint runs its shellcheck pass by default and the ubuntu-latest runner ships shellcheck, so it exits 1 on the existing workflows (~50 [shellcheck] findings in deploy.yml, claude-code-review.yml, test.yml). CI confirms it: job "Workflow Lint (actionlint)" = failure, Process completed with exit code 1 (run 31409088554 / job 93522528634; checksum actionlint.tar.gz: OK, so every failure is a shellcheck finding). Since this PR wires workflow-lint into test-summary's exit-1 condition, the gate is red on the PR itself and would block all merges once enabled — contradicting the PR body's "Current main is clean (exit=0)" (that only reproduces locally without shellcheck on PATH).

Details + fix options are in the inline thread. No other concrete defects found.

View job run

[claude]

Review

Reviewed the diff directly (.github/workflows/test.yml, CLAUDE.md, tests/test_workflow_lint_wiring_1122.py). This is the first substantive review on this PR (only an automated "working…" placeholder comment precedes it).

Overall
Solid, well-scoped fix. The PR does exactly what it says: adds a workflow-lint job, wires it into test-summary's needs and its exit-1 condition (correctly avoiding the "in needs but not actually gating" trap the PR itself calls out), pins actionlint by version + sha256, and backs the wiring with tests that were verified non-tautological by deliberate removal. The CLAUDE.md addition is a good diagnostic note for future incidents.

Things that check out

• Gate wiring is correct: workflow-lint is in both the needs: array (test.yml:585) and the if failure condition (test.yml:604), matching the pattern used by the other required jobs (code-quality, backend-tests, etc.).
• Not path-filtered: reasonable given the stated rationale — a "skipped" required check on exactly the PRs that touch workflows is the same silent hole this job is meant to close, and the cost (~1s) is trivial.
• Download hygiene: curl -sSLf (fails loudly on HTTP error), set -euo pipefail, and the sha256 check happen before tar/execution — a checksum mismatch aborts before anything downloaded runs. This fails safe: a wrong hash breaks the job loudly rather than silently running something unverified.
• Pinning style is consistent with the rest of the file (actions/checkout pinned to full SHA + version comment, same convention used elsewhere in test.yml).
• Test coverage is good and honest: the docstring in tests/test_workflow_lint_wiring_1122.py::test_test_summary_actually_fails_on_it explicitly documents a self-caught false positive (a looser assertion that matched the summary table row and passed even with the gate removed) — good practice, and the tightened assertion is a real regression guard, not a tautology.
• test_every_workflow_file_still_parses is appropriately scoped as a cheap local backstop (YAML-parses-to-a-mapping-with-jobs), not a substitute for actionlint's expression-level checking — the docstring says as much.
• pytestmark = pytest.mark.v2 is applied per the CLAUDE.md convention for new v2 tests. pyyaml is already a project dependency, so no new dependency introduced.

Minor notes (non-blocking)

• Checksum provenance: I don't have network access in this review environment, so I could not independently verify ACTIONLINT_SHA256 against the officially published checksums for actionlint v1.7.7. Given the fail-safe behavior described above, this isn't a security concern, just something worth a final sanity check before merge — the first real run will confirm or deny it immediately.
• No permissions: block on the new job: two other jobs in this file set explicit permissions:, but most (including this one) rely on default token permissions. Since workflow-lint only needs contents: read, an explicit least-privilege block would be a nice-to-have, but it's consistent with the file's existing majority convention, so not a defect.
• Stray actionlint.tar.gz / extracted actionlint binary land in the checked-out working directory, but the job has no later steps that care about working-tree cleanliness, so this is harmless.

No bugs, security issues, or missing test coverage found. Good to merge from a code-review standpoint, modulo the checksum sanity-check note above.

[frankbria]

Good catch — the job failed on its own first run, and my local verification had been invalid.

actionlint runs shellcheck over every run: block whenever shellcheck is on PATH. It is on GitHub runners; it was not on my machine. So that entire check class silently disappeared locally and the run looked clean. I installed shellcheck and reproduced it exactly:

actionlint            # 176 lines, 44 findings — ALL [shellcheck]
actionlint -shellcheck=   # clean

Zero non-shellcheck findings. The workflows compile; the failures are pre-existing shell style, almost all in deploy.yml.

The job now runs -shellcheck=, which keeps precisely what #1122 is about — workflows GitHub cannot compile, the failure mode that yields a run with zero jobs and no logs. Verified both halves still hold: clean on the current tree, and still reports the a0a69636 regression at deploy.yml:570.

I did not fix the 44 here. Two reasons: it would block a P1 CI fix behind unrelated edits, and most are quoting changes in the deploy path that cannot be validated from a PR — deploying is the only way to know an SSH heredoc still behaves, and changing it blind is how the original outage happened. Filed as #1130 (P2.34), including the two findings I think are worth real scrutiny: an SC2087 unquoted ENDSSH delimiter (changes where variables expand over SSH) and an SC1072/SC1073 parse error at deploy.yml:570 that is probably a ${{ }} false positive but should be confirmed, not assumed.

Two tests pin that the suppression stays documented and stays narrow, so it cannot quietly become permanent.

[github-actions]

Claude finished @frankbria's task in 4m 10s —— View job

---

GLM precision review — done

✅ No defects found. (Re-checked the full diff of ad6c70b against origin/main plus the surrounding test-summary block.)

The earlier major finding (actionlint's default shellcheck pass failing the job on the runner) is resolved by -shellcheck= and verified green by CI, not just by the diff. On the latest commit the new job's own run is:

• Workflow Lint (actionlint) → success, no failed steps (run 31409546687 / job 93524020337).

That one result closes the two items the prior reviews explicitly could not confirm and left to "the first real run":

1. ACTIONLINT_SHA256 is correct for v1.7.7 linux_amd64. The echo … | sha256sum -c - step runs before tar/execution under set -euo pipefail; a wrong digest would fail that step and the job would be red. It's green, so the pinned checksum matches the release artifact.
2. -shellcheck= does disable shellcheck integration (empty value). If it did not, actionlint would re-emit the ~44 [shellcheck] findings the author reproduced and exit 1. It exited 0, so the suppression works as intended — the job gates on compilability/expression errors only, which is exactly [P1.41] Nothing lints the workflow files — a workflow that will not compile ships silently (twice now) #1122's scope.

Gate wiring re-confirmed in test.yml:

• workflow-lint is in needs (test.yml:589) and in the exit-1 branch (test.yml:610), so it actually gates rather than just being depended-on under if: always().
• It is failure-only by design (== "failure"), matching backend-tests/frontend-tests/code-quality/check-hardcoded-urls; only e2e-browser-smoke gets the cancelled/timed_out treatment, as the comment at test.yml:606-608 states. Consistent, not a defect.

No inline findings to post — nothing rose to a concrete, reproducible failure scenario. Note: this environment has no network egress, so the checksum/flag confirmation above is read from the CI run result (the job that actually downloaded and ran the binary) rather than re-fetched from upstream — which is the stronger evidence anyway.

[claude]

Re-review — follow-up commit (ad6c70b)

This addresses the major finding from the automated GLM review and matches what my own prior review flagged as a non-blocking checksum/verification caveat: the shellcheck-integration failure. The fix is sound.

What checks out

• Root cause is correctly diagnosed: actionlint runs shellcheck over every run: block whenever shellcheck is on PATH, which is true on GitHub runners but wasn't on the author's machine — so the entire check class silently vanished during local verification. That's a good, honest catch, and the commit message documents it clearly.
• -shellcheck= is the correct way to disable that integration (empty value = disabled), and scoping down to compilability-only keeps this PR true to its stated purpose ([P1.41] Nothing lints the workflow files — a workflow that will not compile ships silently (twice now) #1122: catch workflows GitHub cannot compile, not shell style). Verified per the commit message: 44 findings pre-fix, all [shellcheck], zero after; the a0a69636 regression (deploy.yml:570, empty ${{ }}) still reproduces with the flag on.
• The suppression isn't silent: it's commented inline, filed as its own follow-up ([P2.34] actionlint's shellcheck findings are suppressed in workflow-lint — 44 pre-existing issues in run: blocks #1130), and pinned by two new tests (test_the_shellcheck_suppression_is_documented_and_tracked, test_workflow_compilability_is_still_checked) so it can't quietly become a permanent, unexplained carve-out or accidentally swallow the compilability check itself.
• Not fixing the 44 shell-style findings inline is the right call — most are quoting changes in the deploy path that can't be validated from a PR, which is exactly the class of blind edit that caused the original outage.

Still open (non-blocking, unchanged from my prior review)

• I still don't have network access in this environment to independently verify ACTIONLINT_SHA256 against actionlint v1.7.7's published checksums. The value is well-formed (64 hex chars) and the download fails safe (set -euo pipefail + sha256sum -c - before extraction/execution), so this is a sanity-check item, not a security concern.
• Minor doc nit: the PR description's "Current main is clean (exit=0), including this PR's own additions" predates this fix and was true only for the local (no-shellcheck) run. Not misleading about the actual defect (which is well explained elsewhere in the thread and commit message), just worth a quick edit for anyone skimming the top of the PR later.

No new bugs, security issues, or test-coverage gaps in this commit. Good to merge.