ci(workflows): add daily bash-step hygiene auditor agentic workflow#498
Merged
jamesadevine merged 3 commits intomainfrom May 10, 2026
Merged
ci(workflows): add daily bash-step hygiene auditor agentic workflow#498jamesadevine merged 3 commits intomainfrom
jamesadevine merged 3 commits intomainfrom
Conversation
Adds tests/bash_lint_tests.rs, an integration test that compiles a representative set of fixtures and runs shellcheck against every literal bash: body in the generated YAML. The lint catches the actual silent-failure patterns ADO's "fail on last command" default lets through (SC2164 cd-without-||, SC2155 masked-return, SC2086/2046 unquoted variables, SC2154 unset refs, SC2088 tilde-in-quotes). This replaces the previously proposed approach of sprinkling `set -eo pipefail` across every bash step (PR #492). That approach added boilerplate to ~27 sites without enforcement, drifted as new steps were added, and in two spots actually masked errors more than the original code (`grep ... | tail -1 || true`). Real bugs surfaced and fixed by the new lint: * `src/engine.rs` — `Engine::Copilot::log_dir()` returned `~/.copilot/logs`. Tilde does not expand inside the double-quoted `[ -d "..." ]` test that consumes this value, so the directory check always failed and Copilot logs were silently never collected to the pipeline artifact. Replaced with `$HOME/.copilot/logs`. * `src/runtimes/node/mod.rs` and `src/runtimes/dotnet/mod.rs` — the ensure-`.npmrc` and ensure-`nuget.config` step generators used Rust `\<newline>` line continuations in their format strings, which strip leading whitespace. The emitted YAML had body lines flush-left against `- bash: |`, producing invalid YAML. Replaced with raw string literals so indentation is preserved. * Multiple `cd "$DOWNLOAD_DIR"` in `base.yml` / `1es-base.yml` had no `|| exit` guard. Added. * `exit $AGENT_EXIT_CODE` (multiple sites) — quoted. * `mkdir -p {{ working_directory }}/safe_outputs` and the matching `cp -a ...` — quoted the substitution. * `JSON_CONTENT=$(echo "$RESULT_LINE" | sed 's/.*PFX://')` rewritten to `${RESULT_LINE##*PFX:}` (avoids forking sed and removes a shellcheck SC2001 finding). Targeted `set -eo pipefail` additions (only where masked-pipeline exit codes matter): * `base.yml` / `1es-base.yml` ado-aw download steps (3 stages × 2 templates): `grep "ado-aw-linux-x64" checksums.txt | sha256sum -c -` silently passes when grep matches nothing because sha256sum returns 0 on empty stdin. Without pipefail, the unverified binary would install successfully. * `src/compile/extensions/trigger_filters.rs` script-download step: same `grep | sha256sum` pattern. * `src/runtimes/lean/mod.rs` install step: `curl ... | sh` would silently install nothing on curl failure. The two pre-existing `set -eo pipefail` instances on the AWF download + docker pull steps (introduced in PR #439) and on the `tee`-piped agent / threat-analysis runs are preserved — those were correct. Skip vs. enforce: * Locally, the test prints a notice and returns early when shellcheck is missing. * CI installs shellcheck and sets `ENFORCE_BASH_LINT=1` so a missing shellcheck becomes a hard failure rather than a silent skip. A new `tests/fixtures/runtime-coverage-agent.md` exercises the Lean, Node-with-feed-url, and .NET-with-feed-url runtimes plus the cache-memory tool, ensuring every code-generated bash step is reached. The lint enforces a `REQUIRED_STEP_DISPLAY_NAMES` coverage list to catch fixture/generator drift. Documented in AGENTS.md and docs/extending.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds `.github/workflows/bash-lint-auditor.md`, a daily agentic workflow that complements the PR-gate lint added in PR #496. The PR gate gives fast feedback on every PR; this workflow runs once a day and lands small, mechanical improvements that the gate can't: * When a finding does slip onto main (e.g. via merge conflict), the auditor fixes it the next morning instead of waiting for the next contributor PR. * Audits stale `# shellcheck disable=` directives — removes ones that no longer fire (i.e. the underlying code has been cleaned up but the suppression was forgotten). * Audits whether the lint's exclude list could be tightened. * Verifies fixture coverage of every bash-step generator and proposes fixture additions when a new generator appears. When the auditor finds something actionable, it opens a focused PR (one concern per PR) with the structured "what was found / how it was fixed / verification" body. When the lint is green and no proactive improvement is feasible, it exits cleanly with `noop`. Configuration notes: * `schedule: daily around 09:00` — fuzzy schedule scattering across the hour, matching the convention of other daily workflows in this repo (e.g. `cyclomatic-complexity-reducer.md`). * `allowed-files` restricts the auditor to bash-generator code paths plus the tests/fixtures it depends on. `protected-files: fallback-to-issue` ensures that if it tries to edit anything else, the change falls back to an issue rather than a PR. * `cache-memory: true` persists state across runs so the auditor doesn't loop on the same suggestion if a maintainer rejects it. * `bash: ["*"]` + `network.allowed: [defaults, rust]` gives the agent what it needs to install shellcheck (via apt with a static- binary fallback) and run cargo against the rust ecosystem. Compiled with `gh aw compile bash-lint-auditor`; the matching `.lock.yml` is included along with new SHAs in `.github/aw/actions-lock.json` (cache, checkout, download-artifact registered for the first time by this workflow's setup steps). Stacked on top of branch `lint-bash-steps` (PR #496) because the auditor relies on `tests/bash_lint_tests.rs` and `tests/fixtures/runtime-coverage-agent.md`, which are introduced there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
@copilot resolve the merge conflicts in this pull request |
# Conflicts: # tests/bash_lint_tests.rs Co-authored-by: jamesadevine <4742697+jamesadevine@users.noreply.github.com>
Contributor
Resolved and pushed in fbb5a75. I merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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.
Stacks on top of #496. This PR's base is
lint-bash-steps(notmain) because the auditor relies ontests/bash_lint_tests.rsandtests/fixtures/runtime-coverage-agent.md, which are introduced in #496. Once #496 merges, GitHub will retarget this PR tomainautomatically.What this adds
A daily agentic workflow (
.github/workflows/bash-lint-auditor.md) that complements the PR-gate lint from #496. The PR gate gives fast feedback on every PR; this auditor runs once a day and lands small, mechanical improvements the gate can't make on its own.Why it's a separate layer from the PR gate
The PR gate is passive — it blocks regressions. The daily auditor is proactive:
main— if anything ever slips through (force-push, admin override, history rewrite), the auditor fixes it the next morning instead of waiting for the next contributor PR.# shellcheck disable=directives — temporarily deletes each suppression, reruns the lint, and removes the ones that are no longer needed.--exclude=SC1090,SC1091could be tightened, preferring per-line directives when justified.When there's nothing actionable, it exits cleanly with
noop.Configuration highlights
schedule: daily around 09:00— fuzzy scattering, matches the convention of other daily workflows here (cyclomatic-complexity-reducer.md,doc-freshness-check.md).allowed-filesrestricts the auditor to bash-generator code paths plus the tests and fixtures it depends on:src/data/**,src/runtimes/**/mod.rs,src/compile/extensions/**.rs,src/compile/common.rs,src/engine.rs,src/tools/**/extension.rstests/bash_lint_tests.rs,tests/fixtures/**AGENTS.md,docs/extending.mdprotected-files: fallback-to-issue— any attempted edit outsideallowed-filesfalls back to filing an issue rather than opening a PR.cache-memory: true— persists state so the auditor doesn't loop on the same suggestion when a maintainer rejects it.bash: ["*"]+network.allowed: [defaults, rust]— required for the shellcheck install (apt with a static-binary fallback) andcargo test.What runs in each daily execution
cache-memory. If the previous run's PR is still open, exit early.ENFORCE_BASH_LINT=1 cargo test --test bash_lint_tests. Three outcomes:cargo test+cargo clippy --all-targets. All must pass before opening a PR.cache-memory.Validation
gh aw compile bash-lint-auditor— 0 errors, 0 warnings.gh aw compile --validate bash-lint-auditor— clean (one informational notice about an available newer SHA foractions/github-script; non-blocking)..github/aw/actions-lock.jsonupdated with pinned SHAs for the new actions this workflow registers (cache, checkout, download-artifact).cyclomatic-complexity-reducer.md,doc-freshness-check.md,frontmatter-aligner.md).Out of scope
create-pull-request,noop,report-incomplete,missing-data(all existing).