Skip to content

validate-task.yml's shell lint step: shfmt is documented fleet law but unenforced, and the *.sh glob misses extensionless scripts #950

Description

@ptr727

Found while resyncing Blog (ptr727/Blog) against the hub, following RESYNC.md's "Adopting the Gates" section, after #924 unblocked the rest of the migration.

The gap

validate-task.yml's lint job (hub main at 30b93226820112ebf8bd93e69c8a80932e0c4b39, tag 2.0.440) runs a generic "Check shell scripts step":

- name: Check shell scripts step
  run: |
    set -Eeuo pipefail
    mapfile -t scripts < <(git ls-files '*.sh')
    if [ "${#scripts[@]}" -gt 0 ]; then
      docker run --rm --pull=always -v "$PWD":/mnt --workdir /mnt koalaman/shellcheck:stable "${scripts[@]}"
    else
      echo "no shell scripts are tracked"
    fi

Two gaps in this one step, both bearing on the same "shell clean-compile" CODESTYLE.md defines:

  1. shfmt is documented fleet law with no hub enforcement anywhere. CODESTYLE.md states: "shellcheck is the linter and shfmt the formatter. The clean-compile is shellcheck clean at default severity plus shfmt -d, both reporting nothing before a commit." The step above never runs shfmt. scripts/docker_lint.py's LINTERS tuple, the "known-working invocation" GOVERNANCE.md points agents to, has no shfmt entry either (editorconfig-checker, actionlint, markdownlint, cspell, shellcheck, PSScriptAnalyzer only). A repo adopting the hub's validate-task.yml gets shellcheck but never the formatter half CODESTYLE.md promises, and there is no local wrapper command that runs it either.
  2. The *.sh glob misses a tracked shell script with no extension. docker_lint.py's own shellcheck Linter uses the identical pattern, ("*.sh",). Blog carries ops/vps-backup-pull, a tracked #!/usr/bin/env bash script with no extension (a common shape for a script meant to run as a bare command). Neither the hub CI step nor the local wrapper ever lints it.

Where this actually bites

Blog's pre-migration validate-task.yml ran both shellcheck and shfmt -d against an explicit file list that included ops/vps-backup-pull. Adopting the hub's generic step as-is would silently drop shfmt enforcement fleet-wide and drop lint coverage of that one script. Blog's new .github/actions/validate/action.yml validate hook (this repo's own domain-check extension point) keeps a duplicate "Check shell scripts step" that re-runs shellcheck on the full five-script list (including ops/vps-backup-pull) and adds shfmt -d, just to avoid losing coverage the old local file already had. That duplicates the hub lint job's own shellcheck pass over the four .sh files for no reason but this gap.

Ask

Whichever of these fits the hub's existing design best:

  • Add an shfmt -d invocation (mvdan/shfmt:latest -d) alongside the existing shellcheck call in validate-task.yml's shell step, and add a matching shfmt entry to docker_lint.py's LINTERS, so every fleet repo gets the clean-compile CODESTYLE.md already documents, with no per-repo opt-in needed.
  • Broaden shell-script discovery past a bare *.sh glob, either by detecting a #!.../bash (or /bin/sh) shebang on extensionless tracked files, or by adding an optional workflow_call input (for example shell-extra-paths) a caller appends to the discovered list, the same shape validate-task.yml's markdownlint glob has no per-repo exclusion point, breaking repos that vendor/import content #924 added for markdownlint's globs.

Either fix lets Blog delete its own duplicate shell-check step entirely and rely on the hub's generic lint job alone, which is the same "minimal unique code, reuse of field-proven code" outcome #924 aimed for.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions