Skip to content

fix: add actionlint coverage for scaffold workflow templates - #1030

Merged
ralphbean merged 4 commits into
mainfrom
lint-scaffold-actionlint
May 26, 2026
Merged

fix: add actionlint coverage for scaffold workflow templates#1030
ralphbean merged 4 commits into
mainfrom
lint-scaffold-actionlint

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • Add hack/lint-scaffold-workflows to run actionlint on scaffold templates (with __ORG__ placeholder substitution) — the existing pre-commit actionlint hook only covers .github/workflows/ in the repo root
  • Wire it into .pre-commit-config.yaml with the same file trigger as lint-workflow-size
  • Remove secrets: {} from shim-workflow-call.yaml — actionlint flags empty secrets sections when the called workflow declares none (fixes the lint failure on chore: update fullsend shim workflow #1028)
  • Remove corresponding test assertion in scaffold_test.go

Context

secrets: {} was introduced in #503 as an explicit "pass no secrets" when migrating from PAT to OIDC, then carried forward in #792 when per-stage jobs were collapsed into a single dispatch job. The intent was correct but actionlint considers it a syntax error. Omitting secrets: entirely has the same effect.

Test plan

  • hack/lint-scaffold-workflows passes after removing secrets: {}
  • pre-commit run lint-scaffold-workflows --all-files passes
  • CI lint job passes

🤖 Generated with Claude Code

The pre-commit actionlint hook only checks .github/workflows/ in the
repo root, so scaffold templates under internal/scaffold/fullsend-repo/
were never linted. This allowed secrets: {} to slip into the
shim-workflow-call template — actionlint flags empty secrets sections
as a syntax error when the called workflow declares no secrets.

Add hack/lint-scaffold-workflows which copies scaffold workflows into a
temp git repo (with __ORG__ placeholder substitution) and runs
actionlint. Wire it into pre-commit with the same file trigger pattern
as lint-workflow-size.

Also remove the now-invalid secrets: {} from the shim template and its
corresponding test assertion.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown

Site preview

Preview: https://8233f28b-site.fullsend-ai.workers.dev

Commit: e24b939ae823fe59e6bf678fb7609336d7a67cd7

CI installs actionlint via pre-commit's isolated Go environment, so it
is not on the system PATH when our language: script hook runs. Fall back
to go run if actionlint is not found.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>

@rh-hemartin rh-hemartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please, make use of pre-commit you can specify which files are passed with files:

- repo: https://github.com/rhysd/actionlint
    rev: v1.7.11
    hooks:
      - id: actionlint
        files: |
          (?x)^(
            \.github/workflows/
            |internal/scaffold/fullsend-repo/.github/workflows
            |internal/scaffold/fullsend-repo/templates
          )
        args: ["-ignore", 'unexpected key "queue" for "concurrency"']

Replace the 83-line hack/lint-scaffold-workflows script with a files
pattern override on the existing actionlint pre-commit hook. actionlint
handles __ORG__ placeholders and non-standard paths fine when given
files directly, so the tempdir-copy-and-substitute approach was
unnecessary.

Suggested-by: Hector Martin <rh-hemartin@users.noreply.github.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean
ralphbean requested a review from rh-hemartin May 19, 2026 17:26
@fullsend-ai-review

fullsend-ai-review Bot commented May 19, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [protected-path] .pre-commit-config.yaml — This PR modifies .pre-commit-config.yaml, which is a protected governance/infrastructure file. The PR has no linked issue. While the PR body explains the rationale (extending actionlint coverage to scaffold templates, fixing lint failure from chore: update fullsend shim workflow #1028), protected-path changes require a linked issue for traceability. Human approval is required for all protected-path changes.
    Remediation: Link the PR to an issue (e.g., use "Fixes chore: update fullsend shim workflow #1028" if that issue covers this work, or create a dedicated issue) to provide formal authorization for modifying governance files.

Low

  • [intent-alignment] .pre-commit-config.yaml — The PR description describes adding a hack/lint-scaffold-workflows script and removing secrets: {} from shim-workflow-call.yaml, but neither change is present in the actual diff. The final implementation expanded the existing actionlint hook's files pattern instead. The PR description should be updated to match the actual implementation to avoid confusion for future readers.
    Remediation: Update the PR body to describe the actual approach (expanding actionlint's files filter and adding ignore rules) rather than the original approach (separate hack script and template modification).
Previous run

Review

Findings

Medium

  • [protected-path] .pre-commit-config.yaml — This PR modifies .pre-commit-config.yaml, which is a protected governance/infrastructure file. The PR body provides sufficient context (extending actionlint coverage to scaffold templates, fixing lint failure from chore: update fullsend shim workflow #1028), but human approval is always required for protected-path changes regardless of context.

Low

  • [correctness] .pre-commit-config.yaml — The PR body describes adding a hack/lint-scaffold-workflows script and references running pre-commit run lint-scaffold-workflows --all-files in the test plan, but the final implementation (per commit "fix: simplify scaffold actionlint to use pre-commit files pattern") took a different approach — expanding the existing actionlint hook's files pattern instead. The PR description should be updated to match the actual implementation to avoid confusion for future readers.
    Remediation: Update the PR body to describe the actual approach (expanding actionlint's files filter) rather than the original approach (separate hack script).

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label May 19, 2026

@fullsend-ai-review fullsend-ai-review 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.

See the review comment for full details.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label May 26, 2026
Merged via the queue into main with commit 6216dd0 May 26, 2026
7 of 8 checks passed
@ralphbean
ralphbean deleted the lint-scaffold-actionlint branch May 26, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants