[yamllint-fixer] fix(workflow): eliminate yamllint comments-indentation noise in generated lock files#45879
Conversation
Commented-out processed fields (e.g. `# roles:`, `# state:`, `# skip-author-associations:`) at the end of the `on:` section are followed, in the assembled workflow, by a top-level key (`permissions:`, `concurrency:`, ...) at column 0. yamllint's comments-indentation rule flags a comment whose indentation matches neither the preceding nor the following content line, so a trailing block indented under `on:` was reported (3 warnings across the generated lock files). Re-indent only the final trailing comment block of the `on:` section to column 0 so it aligns with the top-level key that follows. Middle comment blocks already have a matching indentation anchor below them and are left untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
|
✅ Test Quality Sentinel completed test quality analysis. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (62 additions across 3 files). |
There was a problem hiding this comment.
Pull request overview
Fixes yamllint comment-indentation warnings in generated workflow lock files.
Changes:
- De-indents trailing processed-field comments to column 0.
- Updates related test expectations.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/frontmatter_on_section_cleanup.go |
Adds trailing-comment de-indentation. |
pkg/workflow/skip_author_associations_test.go |
Updates compiled output assertions. |
pkg/workflow/frontmatter_extraction_yaml_fuzz_test.go |
Updates fuzz-test expectations. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Medium
| mustContain("# labels: # Label filtering applied via job conditions") | ||
| mustContain("# - " + topAQuoted + " # Label filtering applied via job conditions") | ||
| mustContain("# - " + topBQuoted + " # Label filtering applied via job conditions") | ||
| mustContain(" # - " + nestedAQuoted) | ||
| mustContain(" # - " + nestedBQuoted) | ||
| mustContain("# - " + nestedAQuoted) | ||
| mustContain("# - " + nestedBQuoted) |
| if !strings.Contains(result, " # - "+nestedAQuoted) { | ||
| // Commented on.steps content is the trailing block of the on: section, so it is | ||
| // de-indented to column 0 (yamllint comments-indentation). | ||
| if !strings.Contains(result, "# - "+nestedAQuoted) { |
| t.Fatalf("expected nested labels item to remain in on.steps output:\n%s", result) | ||
| } | ||
| if !strings.Contains(result, " # - "+nestedBQuoted) { | ||
| if !strings.Contains(result, "# - "+nestedBQuoted) { |
There was a problem hiding this comment.
Logic is correct and well-guarded. The dedentTrailingOnCommentBlock function cleanly handles edge cases (all-comment files, blank trailing lines, non-comment last line). Tests updated consistently. LGTM.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 11 AIC · ⌖ 4.25 AIC · ⊞ 5K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — approving with one minor suggestion.
📋 Key Themes & Highlights
Key Themes
- Root cause addressed: the fix targets the generator (column-0 re-indent of trailing comment blocks) rather than suppressing yamllint warnings downstream — correct approach
- Scope is precise: only the final comment block is adjusted; middle blocks with a matching anchor below are left untouched, avoiding unintended changes
- Tests updated correctly: fuzz test and integration test assertions reflect the new behavior; the
"\n# ..."prefix inskip_author_associations_test.gois a stronger assertion than the previous indented form
Positive Highlights
- ✅
dedentTrailingOnCommentBlockis a pure function with a clear single responsibility and well-written godoc - ✅ The
start == 0guard correctly handles the edge case where the entire slice is comments - ✅ PR description includes before/after metrics and lists the three affected lock files — makes the change easy to verify
Minor Suggestion
dedentTrailingOnCommentBlock is a non-trivial algorithm (last-non-blank scan, trailing-comment-block walkback, guard conditions). The existing fuzz and integration tests cover it indirectly, but a small direct unit test table (e.g. trailing block only, middle block only, all-comments, empty input) would make future regressions much faster to diagnose per /tdd.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 20.9 AIC · ⌖ 4.4 AIC · ⊞ 6.7K
Comment /matt to run again
🧪 Test Quality Sentinel Report✅ Test Quality Score: 100/100 — Excellent
📊 Metrics (3 tests)
Verdict
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
| // | ||
| // Only the final block is adjusted. Commented blocks with real `on:` content after them | ||
| // already have a matching indentation anchor below and are left untouched. | ||
| func dedentTrailingOnCommentBlock(lines []string) []string { |
There was a problem hiding this comment.
@copilot add dedicated unit test suite for this function
There was a problem hiding this comment.
Added TestDedentTrailingOnCommentBlock in pkg/workflow/frontmatter_on_section_cleanup_test.go with 10 table-driven cases covering: empty input, no trailing comment, single/multiple indented comments dedented to column 0, trailing blank lines with a comment block, already-column-0 (no-op), middle block left untouched, entirely-comments file (no-op), tab indentation, and mixed space+tab indentation.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…mentBlock Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot merge main and recompile |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…on-comments-indentation-696223561d8ee625 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — merged main and recompiled all 257 workflows (0 errors). Committed in the latest push. |
|
@copilot merge main and recompile |
…on-comments-indentation-696223561d8ee625 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
… skill template, recompile Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Merged main (including the new
|
|
🎉 This pull request is included in a new release. Release: |
Summary
Fixes yamllint
comments-indentationviolations in generated lock files. When the workflow compiler emits a commented-out block at the end of theon:section (e.g.skip-author-associationssub-keys), those comment lines were indented to match their YAML context. yamllint requires top-level comments to be at column 0, causing lint noise on every generated file.Changes
Core fix —
pkg/workflow/dedentTrailingOnCommentBlock: scans theon:section of a generated YAML lock file, identifies any trailing comment-only block (lines prefixed with#after stripping leading whitespace/tabs, with optional trailing blank lines), and strips all leading whitespace from those lines to move them to column 0.Tests —
pkg/workflow/dedent_trailing_on_comment_block_test.go— 8 table-driven test cases covering: no trailing comments (no-op), single/multiple indented trailing comments dedented, trailing blank lines preserved, already-at-col-0 unchanged, middle comment blocks untouched, tab-indented comments stripped, mixed indentation stripped.skip_author_associations_test.go— assertions updated from" # issue_comment: contributor"(2-space indent) to"\n# issue_comment: contributor"(column 0) across allskip-author-associationssub-key checks, reflecting the new dedent behaviour.CI / generated files
main.agentic-auto-upgrade.ymlcron schedule updated:11 4 * * 6→21 3 * * 5(Friday 03:21 UTC).pkg/actionpins/data/action_pins.jsonaction pin versions refreshed.multi-agent-research.mdadded to CI fallback list and skill template.Motivation
Every generated lock file that included commented-out
on:sub-keys triggered a yamllintcomments-indentationwarning, creating persistent lint noise across the repository. The fix is applied at generation time so all future recompiles produce clean YAML without requiring per-file suppressions.Test coverage
All new behaviour is covered by the dedicated unit test suite. The existing
skip_author_associationsintegration test is updated to assert the corrected output format.