fix(ci): lint gh-aw generated workflows instead of skipping them - #1854
Merged
Conversation
The workflow-lint job skipped any file whose header marked it DO-NOT-EDIT generated output. The reason was sound -- actionlint v1.7.12 reports errors against gh-aw's emitted YAML for schema gaps rather than real defects -- but the cure removed the gate. The *.lock.yml file is the artifact that actually runs, and it was linted by nothing at all. Measured against the pinned actionlint by compiling the sources and linting the emitted output: 7 errors, all in three classes, none a genuine defect. Those are now suppressed by exact message and scoped path in .github/actionlint.yaml, so generated files are linted with three known exemptions rather than skipped wholesale. Any new error class fails the build. Verified by mutation: a bogus job key, a different unknown permission scope, and an empty string outside the scoped file all still gate. Closes #1827 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 20afe6d2-444e-414e-8a39-e67ab67ca6df
Contributor
🏗️ Architectural Review
Automated architectural review — informational only. |
Contributor
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ✅ | Single commit | 1 commit — clean history |
| ✅ | Not in draft | Ready for review |
| ✅ | Branch up to date | Up to date with dev |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | No source files changed — changeset not required |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ❌ | Copilot threads resolved | 1 unresolved Copilot thread(s) — fix and resolve before merging |
| ✅ | CI passing | All checks passing |
Files Changed (3 files, +172 −40)
| File | +/− |
|---|---|
.github/actionlint.yaml |
+38 −0 |
.github/workflows/squad-workflow-lint.yml |
+16 −40 |
test/actionlint-config.test.ts |
+118 −0 |
Total: +172 −40
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
🟡 Impact Analysis — PR #1854Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedci-workflows (2 files)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new test file imports the yaml package, but no workspace/root package.json declares it, so the test suite is likely to fail with a missing-module error.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the GitHub Actions workflow lint gate to lint gh-aw generated workflows (instead of skipping them), while scoping actionlint suppressions to the exact known schema gaps and adding a test guard to prevent the suppression list from expanding silently.
Changes:
- Replace the “skip DO-NOT-EDIT generated workflows” logic with a direct
actionlint .github/workflows/*.ymlrun. - Add path-scoped, message-specific suppressions in
.github/actionlint.yamlfor known actionlint schema gaps affecting generated outputs. - Add a Vitest guard (
test/actionlint-config.test.ts) that freezes the suppression list and checks it remains narrowly scoped and documented.
File summaries
| File | Description |
|---|---|
test/actionlint-config.test.ts |
Adds a test suite that asserts the actionlint suppression set is exact, scoped, and documented, and that the generated-workflow skip pattern is not reintroduced. |
.github/workflows/squad-workflow-lint.yml |
Removes the header-grep skip and lints all workflows directly via actionlint. |
.github/actionlint.yaml |
Introduces narrowly scoped suppressions for known actionlint schema gaps in generated workflows. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+29
to
+33
| import { describe, it, expect } from 'vitest'; | ||
| import { readFileSync } from 'node:fs'; | ||
| import { join } from 'node:path'; | ||
| import { parse } from 'yaml'; | ||
|
|
This was referenced Aug 24, 2026
Closed
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 #1827
What the measurement found
The issue asked me not to suppress all 6 errors without first determining which bucket each falls into. I did that, using the actionlint version CI pins (v1.7.12), by compiling the workflow sources and linting the emitted output.
Every error is an actionlint schema gap. None is a genuine defect.
squad.lock.yml×2,squad-implement-worker.lock.yml×2unknown permission scope "copilot-requests"squad.lock.yml×1,squad-implement-worker.lock.yml×1unexpected key "queue" for "concurrency"That is where the issue's "6" comes from: 3 errors each across two lock files, not 6 in one. The issue's follow-on premise — "suppress the 2 known ones so the remaining 4 still gate" — does not survive: there is no remaining 4. All six are the two known classes, so scoping the ignores leaves zero generated-output errors gating today. The value is entirely in what gates tomorrow.
A third gap the issue didn't know about
agentics-maintenance.ymlis also generated, and also skipped, and it carries a 7th error nobody could see:It is the
''entry in aworkflow_dispatchchoice input — the "no operation selected" option, matching the input's owndefault: ''. GitHub accepts it; actionlint rejects any empty string. Generated by gh-aw'spkg/workflow/maintenance_workflow.go, so it cannot be fixed here.This is the clearest argument for the change: that file has been sitting in
.github/workflows/accumulating unreviewed lint state, and the skip meant nobody would ever have known whether it was one benign error or ten real ones.The change
The skip and the scoped ignore look similar and are not. A skip hides every future error class; a scoped ignore hides exactly the classes that were measured and explained.
.github/actionlint.yaml— three suppressions, each by exact message, each scoped to a path glob (**/*.lock.yml,**/agentics-maintenance.yml), each with a comment recording what it works around and that it expires when actionlint catches up..github/workflows/squad-workflow-lint.yml— the 45-line header-grep skip becomesactionlint .github/workflows/*.yml. Generated files go from unlinted to linted-with-three-exemptions. Net −40 lines.test/actionlint-config.test.ts— freezes the suppression list so it cannot grow silently.Verification
actionlintover all 26 workflow files, generated included: exit 0.Mutation-tested, because a suppression that hides more than it claims is worse than the skip it replaced:
syntax-checkstill live in generated filestotally-fake-scope)copilot-requestsmessage is ignoredagentics-maintenance.ymlThe guard suite was mutation-tested too — adding an ignore entry, widening a gh-aw pattern to the repo-wide glob, deleting a rationale comment, and reintroducing the header-grep skip each turn it red; all four recover green on restore.
Tests: 103 passed, 13 skipped, 0 failed.
Not done here
The lock file is compiled in the consumer's repo — no
.lock.ymlis committed to this one. So this PR gates the emitted output only insofar asagentics-maintenance.ymlrepresents it. Truly gating the lock file needs a CI step that runsgh aw compileand lints the result, and no job here installs thegh awCLI today. I'll file that as a follow-up rather than bolt a new network dependency onto a fast lint job.The optional companions in the issue — the
git addline dropping.github/aw/actions-lock.json, and the three default-token scope gaps — are untouched and stay open on the issue.