diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d0c2e21c6..23361bcf10 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -740,10 +740,27 @@ jobs: sudo chmod +x /usr/local/bin/yq yq --version - name: Audit + id: audit env: GH_TOKEN: ${{ steps.setup.outputs.token }} run: bash scripts/audit_branch_protection.sh + # The job-level `continue-on-error: true` above silences the audit + # step's failure entirely during the #1555 stabilization window, + # which means real drift could ship invisibly. Surface a warning + # annotation when the audit detected drift; the warning shows up + # in the run summary and check-run UI even though the job stays + # green per #1555. Promote the job to blocking once #1555 closes + # the 30-day zero-drift window. + - name: Report drift outcome + if: always() + env: + AUDIT_OUTCOME: ${{ steps.audit.outcome }} + run: | + if [ "$AUDIT_OUTCOME" = "failure" ]; then + echo "::warning::Branch-protection drift detected -- see Audit step log. Job stays green via job-level continue-on-error during the #1555 stabilization window; promote to blocking once the spec has survived 30 days of zero-drift runs." + fi + # ── Gate: all required checks passed ── ci-pass: name: CI Pass diff --git a/.github/workflows/cli.yml b/.github/workflows/cli.yml index ea5bc0aacf..9f1066a164 100644 --- a/.github/workflows/cli.yml +++ b/.github/workflows/cli.yml @@ -261,34 +261,56 @@ jobs: go-version-file: cli/go.mod cache: false - - name: Run fuzz tests (30s per target) - id: fuzz_run - continue-on-error: true + # Two steps: discovery and execution are split so compile errors + # in fuzz targets fail the matrix cell loudly. The prior single + # step had a job-level `continue-on-error: true` that silenced + # both fuzzer-found crashes (intentional, advisory only) AND + # `go test -list` compile failures (unintentional, real bugs). + # Discovery runs without continue-on-error; execution keeps it. + - name: List fuzz targets + id: fuzz_list working-directory: cli + env: + PKG: ${{ matrix.package }} run: | set -euo pipefail - # Run each Fuzz* function in the package for 30s. - # go test -fuzz only supports one fuzz target at a time, - # so we discover and iterate. - # - # Discover fuzz targets. go test -list exits non-zero on compile - # errors; let that propagate so broken packages aren't silently - # skipped. - LIST_OUTPUT=$(go test -list 'Fuzz.*' ${{ matrix.package }} 2>&1) || { - echo "::error::go test -list failed for ${{ matrix.package }}" + # go test -list exits non-zero on compile errors; let that + # propagate so broken packages aren't silently skipped. + LIST_OUTPUT=$(go test -list 'Fuzz.*' "$PKG" 2>&1) || { + echo "::error::go test -list failed for $PKG (compile error?)" echo "$LIST_OUTPUT" exit 1 } FUZZ_TARGETS=$(echo "$LIST_OUTPUT" | grep '^Fuzz' || true) - if [ -z "$FUZZ_TARGETS" ]; then - echo "No fuzz targets found in ${{ matrix.package }}" - exit 0 - fi - for target in $FUZZ_TARGETS; do + # Multi-line GITHUB_OUTPUT requires a heredoc-style delimiter; + # see https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings + { + echo "targets<> "$GITHUB_OUTPUT" + + - name: Run fuzz tests (30s per target) + id: fuzz_run + if: steps.fuzz_list.outputs.targets != '' + continue-on-error: true + working-directory: cli + env: + # Pass via env (not direct ${{ }} interpolation) so multi-line + # values survive without GitHub Actions expression-syntax + # escaping pitfalls. + FUZZ_TARGETS: ${{ steps.fuzz_list.outputs.targets }} + PKG: ${{ matrix.package }} + run: | + set -euo pipefail + # Run each Fuzz* function in the package for 30s. go test + # -fuzz only supports one fuzz target at a time, so iterate. + while IFS= read -r target; do + [ -z "$target" ] && continue echo "::group::Fuzzing $target" - go test -fuzz="^${target}$" -fuzztime=30s ${{ matrix.package }} + go test -fuzz="^${target}$" -fuzztime=30s "$PKG" echo "::endgroup::" - done + done <<< "$FUZZ_TARGETS" - name: Report fuzz outcome if: always() diff --git a/.github/workflows/dev-release.yml b/.github/workflows/dev-release.yml index 6124b82c87..f70fe9a487 100644 --- a/.github/workflows/dev-release.yml +++ b/.github/workflows/dev-release.yml @@ -189,14 +189,25 @@ jobs: # 2. Create a draft pre-release pointing at the tag. The draft # follows the same lifecycle as stable releases: Docker + CLI # workflows attach assets, then finalize-release publishes - # once both succeed. Clean up the tag on failure. + # once both succeed. + # + # On failure we deliberately do NOT delete the tag. Step 1 + # already pushed the ref, which fired downstream `tags: v*` + # workflows (cli.yml, docker.yml). Deleting the ref now + # would race their actions/checkout step and 404 on + # `refs/tags/$DEV_TAG`, surfacing a false-positive red on + # main even when the squash content is clean (#1818). The + # orphan tag is harmless: the "Clean up old dev pre- + # releases" step below + finalize-release.yml's stable- + # release sweep garbage-collect it later, and finalize- + # release.yml correctly skip-publishes a tag with no draft. if ! gh release create "$DEV_TAG" \ --draft \ --prerelease \ --title "$DEV_TAG" \ --notes-file "$NOTES_FILE"; then - echo "::warning::Release creation failed, cleaning up tag" - gh api -X DELETE "repos/$GITHUB_REPOSITORY/git/refs/tags/$DEV_TAG" || true + echo "::error::Release creation failed for $DEV_TAG" + echo "::warning::Tag preserved to avoid racing downstream workflows (#1818)" exit 1 fi @@ -208,14 +219,51 @@ jobs: env: GH_TOKEN: ${{ github.token }} run: | - # Keep the 5 most recent dev pre-releases, delete the rest + # Keep the 5 most recent dev pre-releases, delete the rest. + # The "keep 5 newest" filter guarantees the JUST-MINTED tag is + # never selected for deletion -- the deleted tags are 5+ + # revisions old, and their downstream Docker / CLI workflows + # have long since completed. The race documented in #1818 + # requires deleting a tag whose downstream workflows are + # still in flight, which this bulk-prune doesn't do. gh release list --limit 50 --json tagName,isPrerelease,createdAt \ --jq '[.[] | select(.isPrerelease and (.tagName | contains("-dev.")))] | sort_by(.createdAt) | reverse | .[5:] | .[].tagName' \ | while read -r tag; do echo "Deleting old dev release: $tag" - gh release delete "$tag" --yes --cleanup-tag 2>/dev/null || true + gh release delete "$tag" --yes --cleanup-tag 2>/dev/null || true # lint-allow: workflow-tag-lifecycle -- bulk-deletes 5+-revision-old dev tags whose downstream workflows have completed; not the just-minted tag (#1818) done + # Regression guard for #1818. The dev tag was created near the + # top of this job; if anything in the run between then and now + # deleted it, downstream Docker / CLI workflows triggered by the + # initial tag push are about to 404 on actions/checkout. Surface + # the deletion as a hard job failure so the existing report- + # failure job opens the dev-release-regression tracking issue + # with the run URL, instead of debugging from downstream symptoms. + - name: Verify minted tag survived the run + # always() ensures this regression guard runs on the failure + # paths -- which is precisely when tag loss is most likely + # (release-create or cleanup step erroring out partway). Without + # always(), the implicit success() condition would skip the + # guard the moment any earlier step failed, defeating the + # canary's purpose. + if: >- + always() + && steps.check-tag.outputs.skip != 'true' + && steps.version.outputs.skip != 'true' + && steps.tag-exists.outputs.skip != 'true' + env: + GH_TOKEN: ${{ github.token }} + DEV_TAG: ${{ steps.version.outputs.dev_tag }} + run: | + set -euo pipefail + if ! gh api "repos/$GITHUB_REPOSITORY/git/refs/tags/$DEV_TAG" \ + >/dev/null 2>&1; then + echo "::error::Tag $DEV_TAG was deleted during this run -- downstream Docker/CLI workflows are about to 404 on actions/checkout. Investigate dev-release.yml." + exit 1 + fi + echo "Tag $DEV_TAG present at end-of-run." + report-failure: name: Open / update tracking issue on dev-release failure needs: dev-release diff --git a/.github/workflows/finalize-release.yml b/.github/workflows/finalize-release.yml index dfd24a7ffe..106ca2221e 100644 --- a/.github/workflows/finalize-release.yml +++ b/.github/workflows/finalize-release.yml @@ -146,13 +146,56 @@ jobs: exit 0 fi - # Both succeeded -- check if release exists and is still a draft - if ! IS_DRAFT=$(gh release view "$TAG" --repo "$GITHUB_REPOSITORY" \ - --json isDraft --jq '.isDraft' 2>/dev/null); then - echo "Release $TAG not found -- nothing to publish." - echo "proceed=false" >> "$GITHUB_OUTPUT" - exit 0 - fi + # Both upstream workflows succeeded -- now confirm the draft + # release exists and is still a draft. Three distinct outcomes + # to disambiguate: + # + # 1. Success: read isDraft and continue. + # 2. "Not found" 404: release-please may legitimately not have + # cut the release yet, OR a brand-new release page may be + # surfacing transient 404s while the API catches up. Poll + # for a bounded window before deciding which it is. With + # both upstream workflows already complete, there is no + # later `workflow_run` event to retry on, so a premature + # "skip on first 404" can leave the commit `pending` + # forever. After the retry budget is exhausted, treat the + # miss as a genuine skip (release-please simply has not run + # yet) and let the next push or finalize cycle pick it up. + # 3. Other failure (auth / rate limit / 5xx / DNS): fail the + # job non-zero so the run is visible and can be retried, + # rather than silently writing proceed=false. + IS_DRAFT="" + MISS_RETRIES=6 # 6 attempts + MISS_BACKOFF_BASE=5 # seconds: 5, 10, 15, 20, 25, 30 (105s total) + attempt=0 + while [ "$attempt" -lt "$MISS_RETRIES" ]; do + attempt=$((attempt + 1)) + release_err=$(mktemp) + if IS_DRAFT=$(gh release view "$TAG" --repo "$GITHUB_REPOSITORY" \ + --json isDraft --jq '.isDraft' 2>"$release_err"); then + rm -f "$release_err" + break + fi + if grep -qiE 'not found|could not resolve|HTTP 404' "$release_err"; then + rm -f "$release_err" + if [ "$attempt" -lt "$MISS_RETRIES" ]; then + sleep_for=$((MISS_BACKOFF_BASE * attempt)) + echo "Release $TAG not found yet (attempt $attempt/$MISS_RETRIES); sleeping ${sleep_for}s before retry." + sleep "$sleep_for" + continue + fi + # Retries exhausted -- release-please hasn't cut the + # release; legitimate skip (the next push or scheduled + # cycle will pick it up). + echo "Release $TAG not found after $MISS_RETRIES attempts -- nothing to publish." + echo "proceed=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + echo "::error::gh release view failed for $TAG (transient API/auth error)" + cat "$release_err" >&2 + rm -f "$release_err" + exit 1 + done if [ "$IS_DRAFT" != "true" ]; then echo "Release $TAG is not a draft (isDraft=$IS_DRAFT). Already published." @@ -374,7 +417,13 @@ jobs: # API once and filtering the JSON locally keeps the failure # signal observable while preserving best-effort semantics. PR_NUMBER="" - if PR_API_OUTPUT=$(gh api "repos/$GITHUB_REPOSITORY/commits/$HEAD_SHA/pulls" 2>/dev/null); then + # No `2>/dev/null` on either gh call below: stderr from + # auth / permission / rate-limit failures should surface in + # the run log so the upstream cause is debuggable. The + # explicit if/else branches handle the non-zero exit + # cleanly with a `::warning::`; we don't want to silently + # drop the explanation alongside. + if PR_API_OUTPUT=$(gh api "repos/$GITHUB_REPOSITORY/commits/$HEAD_SHA/pulls"); then PR_NUMBER=$(printf '%s' "$PR_API_OUTPUT" \ | jq -r '.[] | select(.head.ref | startswith("release-please--branches--main")) | .number' \ | head -n1) @@ -382,7 +431,26 @@ jobs: echo "::warning::gh api commits/${HEAD_SHA}/pulls failed; skipping Highlights propagation." fi if [ -n "$PR_NUMBER" ]; then - PR_BODY=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json body --jq '.body' 2>/dev/null || true) + # Split the call into capture + classify so an auth / + # rate-limit failure surfaces a `::warning::` distinct + # from "PR was deleted" (legitimate skip). The previous + # `|| true` swallowed all non-zero exits and conflated + # the two -- a regression that would silently lose + # Highlights every release until someone noticed. + if PR_VIEW_OUTPUT=$(gh pr view "$PR_NUMBER" --repo "$GITHUB_REPOSITORY" --json body --jq '.body' 2>&1); then + PR_BODY="$PR_VIEW_OUTPUT" + else + PR_BODY="" + # `gh pr view` returns non-zero on both 404 (PR + # genuinely deleted) and on auth / rate-limit / 5xx + # failures. Distinguish via stderr fingerprint. + if printf '%s' "$PR_VIEW_OUTPUT" | grep -qiE 'not found|could not resolve'; then + echo "::notice::release-please PR #${PR_NUMBER} no longer exists; skipping Highlights propagation." + else + echo "::warning::gh pr view #${PR_NUMBER} failed (auth / rate-limit / API error); skipping Highlights propagation." + echo "::warning::stderr: ${PR_VIEW_OUTPUT}" + fi + fi if [ -n "$PR_BODY" ]; then # Find marker line numbers via exact whole-line match # (`grep -nFx`): -n adds line numbers, -F treats the @@ -558,6 +626,12 @@ jobs: if gh release edit "$TAG" --repo "$GITHUB_REPOSITORY" --draft=false; then echo "Release $TAG published successfully." else + # `2>/dev/null` is intentional: this is the publish-race + # fallback. `|| echo "unknown"` provides an explicit + # sentinel for the not-found case; if the call fails for a + # different reason the surrounding logic still treats + # `unknown` as "could not confirm publish" and surfaces an + # error in the next branch. IS_DRAFT_NOW=$(gh release view "$TAG" --repo "$GITHUB_REPOSITORY" \ --json isDraft --jq '.isDraft' 2>/dev/null || echo "unknown") if [ "$IS_DRAFT_NOW" = "false" ]; then @@ -587,7 +661,11 @@ jobs: run: | set -euo pipefail - # Safety: verify the stable release is actually published + # Safety: verify the stable release is actually published. + # `2>/dev/null` + `|| echo "not_found"` is intentional: the + # next check requires `IS_DRAFT == "false"` to proceed, so + # ANY non-success outcome (genuine 404, transient API error, + # auth failure) safely falls through to the abort branch. IS_DRAFT=$(gh release view "$TAG" --repo "$GITHUB_REPOSITORY" \ --json isDraft --jq '.isDraft' 2>/dev/null || echo "not_found") if [ "$IS_DRAFT" != "false" ]; then @@ -600,21 +678,68 @@ jobs: # 1. Delete all dev releases (draft, pre-release, or published) + their tags. # Use the API directly to ensure drafts are included. # Strict regex: only match vX.Y.Z-dev.N (never stable tags). - gh api "repos/$GITHUB_REPOSITORY/releases" --paginate \ - --jq '.[] | select(.tag_name | test("^v[0-9]+\\.[0-9]+\\.[0-9]+-dev\\.[0-9]+$")) | .tag_name' \ - | while read -r dev_tag; do - echo "Deleting dev release + tag: $dev_tag" - gh release delete "$dev_tag" --repo "$GITHUB_REPOSITORY" --yes --cleanup-tag || true - done + # Capture the API output to a variable BEFORE feeding mapfile. + # `mapfile < <(cmd)` does NOT propagate cmd's exit code: a + # failed `gh api` (auth, rate-limit, 5xx) closes the FD with + # empty input and mapfile exits 0, leaving the failure- + # tracking array empty and the cleanup silently no-op. The + # explicit `|| { exit 1 }` here makes a real API failure loud. + # `mapfile <<< "$VAR"` (here-string, no subshell) keeps the + # failure-tracking array in scope, which was the original + # reason for moving away from `cmd | while read`. + if ! GH_RELEASES_OUTPUT=$(gh api "repos/$GITHUB_REPOSITORY/releases" --paginate \ + --jq '.[] | select(.tag_name | test("^v[0-9]+\\.[0-9]+\\.[0-9]+-dev\\.[0-9]+$")) | .tag_name'); then + echo "::error::Failed to list dev releases for cleanup (gh api non-zero exit)" + exit 1 + fi + if [ -z "$GH_RELEASES_OUTPUT" ]; then + dev_releases=() + else + mapfile -t dev_releases <<< "$GH_RELEASES_OUTPUT" + fi + release_failures=() + for dev_tag in "${dev_releases[@]}"; do + echo "Deleting dev release + tag: $dev_tag" + if ! gh release delete "$dev_tag" --repo "$GITHUB_REPOSITORY" --yes --cleanup-tag; then + echo "::warning::Failed to delete dev release $dev_tag" + release_failures+=("$dev_tag") + fi + done # 2. Delete orphan dev tags (tags without a release, e.g. from failed builds). - gh api "repos/$GITHUB_REPOSITORY/git/matching-refs/tags/v" --paginate --jq '.[].ref' \ - | sed 's|^refs/tags/||' \ - | { grep -E '^v[0-9]+\.[0-9]+\.[0-9]+-dev\.[0-9]+$' || true; } \ - | while read -r dev_tag; do - echo "Deleting orphan dev tag: $dev_tag" - gh api -X DELETE "repos/$GITHUB_REPOSITORY/git/refs/tags/$dev_tag" || true - done + # Same explicit-capture pattern as above. `set -o pipefail` + # (from the earlier `set -euo pipefail`) makes the `gh api | + # sed | grep` pipeline fail loudly if any stage other than + # the trailing `grep || true` (which is allowed to find no + # matches) fails. + if ! GH_TAGS_OUTPUT=$(gh api "repos/$GITHUB_REPOSITORY/git/matching-refs/tags/v" --paginate --jq '.[].ref' \ + | sed 's|^refs/tags/||' \ + | { grep -E '^v[0-9]+\.[0-9]+\.[0-9]+-dev\.[0-9]+$' || true; }); then + echo "::error::Failed to list orphan dev tags for cleanup (gh api or sed non-zero exit)" + exit 1 + fi + if [ -z "$GH_TAGS_OUTPUT" ]; then + orphan_tags=() + else + mapfile -t orphan_tags <<< "$GH_TAGS_OUTPUT" + fi + tag_failures=() + for dev_tag in "${orphan_tags[@]}"; do + echo "Deleting orphan dev tag: $dev_tag" + if ! gh api -X DELETE "repos/$GITHUB_REPOSITORY/git/refs/tags/$dev_tag"; then + echo "::warning::Failed to delete orphan dev tag $dev_tag" + tag_failures+=("$dev_tag") + fi + done + + # Per-tag warnings above preserve diagnostics for partial-success + # cleanups; a non-empty failure list fails the job so the + # existing report-failure tracking-issue lane fires (delete is + # idempotent so manual rerun is safe). + if [ ${#release_failures[@]} -gt 0 ] || [ ${#tag_failures[@]} -gt 0 ]; then + echo "::error::Dev cleanup had failures: releases=${release_failures[*]:-none} tags=${tag_failures[*]:-none}" + exit 1 + fi echo "Dev pre-release cleanup complete" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 976df1b92a..7ed248acc1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,9 +5,11 @@ ci: # Hooks listed here are skipped ONLY on pre-commit.ci. They still run # on every developer machine and in any CI job that invokes pre-commit # directly. Skip reasons, grouped by category: - # - commitizen / gitleaks / hadolint-docker / caddy-validate: + # - commitizen / gitleaks / hadolint-docker / caddy-validate / zizmor: # covered by dedicated CI jobs; re-running on pre-commit.ci adds - # latency without adding signal. + # latency without adding signal. (actionlint is NOT skipped: no + # dedicated CI job covers it, and pre-commit.ci is the only + # enforcement layer for contributors who skipped local hooks.) # - mypy / pytest-unit / go-* / eslint-web: # too slow / toolchain-heavy for the cloud runner. # - no-em-dashes / no-redundant-timeout / forbidden-literals / @@ -28,7 +30,7 @@ ci: # scripts with no CI counterpart, and letting pre-commit.ci enforce # them closes the gap where a PR from a contributor who skipped local # hooks would otherwise introduce a regression unchecked. - skip: [commitizen, gitleaks, hadolint-docker, caddy-validate, no-em-dashes, no-redundant-timeout, mypy, pytest-unit, golangci-lint, go-vet, go-test, eslint-web, check-push-rebased, check-single-migration-per-pr, check-no-modify-migration, forbidden-literals, persistence-boundary, persistence-protocol-uniformity, dependency-inversion, provider-complete-chokepoint, no-new-logger-exception-str-exc, otlp-span-redaction, orphan-fixtures, doc-drift-counts, boundary-typed, setting-to-startup-trace, list-pagination, domain-error-hierarchy, dead-api-endpoints, dual-backend-test-parity, schema-drift, no-magic-numbers, convention-gate-inventory, mcp-admin-guardrail] + skip: [commitizen, gitleaks, hadolint-docker, caddy-validate, zizmor, no-em-dashes, no-redundant-timeout, mypy, pytest-unit, golangci-lint, go-vet, go-test, eslint-web, check-push-rebased, check-single-migration-per-pr, check-no-modify-migration, forbidden-literals, persistence-boundary, persistence-protocol-uniformity, dependency-inversion, provider-complete-chokepoint, no-new-logger-exception-str-exc, otlp-span-redaction, orphan-fixtures, doc-drift-counts, boundary-typed, setting-to-startup-trace, list-pagination, domain-error-hierarchy, dead-api-endpoints, dual-backend-test-parity, schema-drift, no-magic-numbers, convention-gate-inventory, mcp-admin-guardrail] default_install_hook_types: [pre-commit, commit-msg, pre-push] @@ -134,6 +136,25 @@ repos: hooks: - id: hadolint-docker + # Workflow YAML safety gates. actionlint catches workflow-syntax + # errors, expression typos, and shellcheck-style issues in `run:` + # blocks. zizmor catches GitHub Actions security smells (dangerous + # workflow_run patterns, secret leakage, expression injection, + # outdated action SHAs). Scoped to .github/workflows/ + .zizmor.yml + # so unrelated commits do not pay the lint cost. + - repo: https://github.com/rhysd/actionlint + rev: v1.7.12 + hooks: + - id: actionlint + files: ^\.github/workflows/.*\.ya?ml$ + + - repo: https://github.com/zizmorcore/zizmor-pre-commit + rev: v1.24.1 + hooks: + - id: zizmor + files: ^(\.github/workflows/.*\.ya?ml|\.github/\.zizmor\.yml)$ + args: [--config=.github/.zizmor.yml] + - repo: local hooks: - id: caddy-validate @@ -202,6 +223,21 @@ repos: language: system files: ^\.github/workflows/.*\.ya?ml$ + - id: workflow-tag-lifecycle + name: block tag CREATE+conditional-DELETE in same workflow (#1818) + entry: uv + args: [run, python, scripts/check_workflow_tag_lifecycle.py, --scan-all] + language: system + files: ^(\.github/workflows/.*\.ya?ml|scripts/check_workflow_tag_lifecycle\.py|\.pre-commit-config\.yaml)$ + # pass_filenames: false so the hook always re-scans every + # workflow YAML regardless of which file in `files:` triggered + # it. Without this, an edit limited to the checker script or + # to .pre-commit-config.yaml would invoke the script with only + # those non-workflow paths; the script would skip them on the + # ``.yml/.yaml`` suffix filter and exit 0 with zero workflows + # actually scanned, letting a weakened gate land unchecked. + pass_filenames: false + - id: doc-drift-counts name: doc count floors (event modules) entry: uv diff --git a/CLAUDE.md b/CLAUDE.md index 5075e60d87..7c5ca91b76 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -114,6 +114,7 @@ Existing gate inventory (all under `scripts/`): - `check_schema_drift.py` - `check_setting_to_startup_trace.py` - `check_web_design_system.py` +- `check_workflow_tag_lifecycle.py` Wire each new gate into `.pre-commit-config.yaml` (pre-commit or pre-push stage as fits) so it runs locally and in CI; per-line opt-outs diff --git a/docs/reference/claude-reference.md b/docs/reference/claude-reference.md index c56fe41fd9..21b30625d1 100644 --- a/docs/reference/claude-reference.md +++ b/docs/reference/claude-reference.md @@ -98,8 +98,8 @@ data/ # Shared data files (competitors.yaml for comparison page) - **Release**: `release.yml`: Release Please creates draft release PR. Mints a `synthorg-repo-bot` App installation token via the `release-runner-setup` composite action (secrets documented in [docs/reference/github-environments.md](./github-environments.md#release_bot_app_)). Gated by the `release` deployment environment. Includes a Highlights step that calls GitHub Models (`openai/gpt-4.1-mini` via `actions/ai-inference`, Copilot Pro quota, no new secret) to prepend a three-section summary to the release PR body, wrapped in `...` markers. Total bullet count is dynamic (1-15) scaled to the changelog volume and distributed across three fixed headers: *What you'll notice* (user-facing fixes + UX / behaviour changes), *What's new* (newly-introduced capabilities and extensions), *Under the hood* (maintenance, deps, refactors, included only when notable). Empty sections are omitted. Opt out per-release by adding a `No-Highlights:` trailer (case-insensitive, anywhere on its own line) to the Release Please PR body before the workflow runs. `finalize-release.yml` then promotes the same marker block from the merged release-please PR body into the published release body (release-please builds release notes from `CHANGELOG.md` only, so without this promotion the Highlights block would stay stranded on the PR; see "Finalize Release" below). The CLI consumes the same Highlights block during `synthorg update` on stable channels: it walks every release in `(installed, target]` oldest-to-newest in batches of 3 and renders the styled summary by default, with `c` toggling between the AI summary and the Release Please commit-based changelog. Releases without a Highlights block (pre-rollout or `No-Highlights:` opt-out) fall back to the commit view automatically. Dev pre-releases have no Highlights block by design, so the CLI walk renders a single combined commit list via the GitHub compare API instead. Walk is gated to interactive TTY runs; `--quiet` / `--json` / `--yes` / non-TTY contexts skip the walk and print the terse "Update available" notice + release-notes URL. - **Auto Rollover**: `auto-rollover.yml`: detects when the last stable tag's patch meets the `__synthorg_rollover_at_patch` threshold in `.github/release-please-config.json` (default 9) and appends an empty commit with `Release-As: 0.(minor+1).0` trailer so Release Please targets the minor bump. Three skip guards: (1) Release Please release commits, (2) any commit already carrying a `Release-As:` trailer (including its own prior rollover commits), (3) any open Release Please release PR whose body already queues a `Release-As:` trailer. Gated by the `release` deployment environment. Commit is authored via the Git Data API (POST `/git/commits` + PATCH `/git/refs/heads/main`) using the App installation token so it ships with a verified signature (required by `main`'s signed-commits rule) and triggers downstream Release + Dev Release workflows. - **Graduate**: `graduate.yml`: `workflow_dispatch` one-click `Release-As:` trailer for target versions that skip the normal patch cadence (1.0 graduation, explicit minor jumps). Inputs: `target_version` + `reason`. Validates target is strictly above last stable (hard-blocks downgrades). Creates a signed empty commit on `main` with the trailer via the Git Data API under the App installation token. Gated by the `release` deployment environment. -- **Dev Release**: `dev-release.yml`: creates semver dev tags (e.g. `v0.4.7-dev.3`) and draft pre-releases on every push to main (skips Release Please version-bump commits). Tags trigger existing Docker + CLI workflows for full build/scan/sign pipeline. Gated by the `release` deployment environment. Uses the `release-runner-setup` composite for token mint. Pre-release body is built locally via `git log -1` on the head SHA and `gh release create --notes-file`: title `Dev build #N toward vX.Y.Z`, then `**Commit:** `, `**Subject:** `, the full commit body (squash-merges carry the PR description), the `**Full pipeline:**` disclaimer, and the channel opt-in tip. Variables go through `printf '%s'` placeholders (the `--notes-file` route avoids command substitution that bare `--notes "..."` would suffer if a commit subject contained backticks or `$(...)`). Incrementally prunes old dev pre-releases (keeps 5 most recent); finalize-release deletes all remaining when a stable release is published. -- **Finalize Release**: `finalize-release.yml`: assembles the release body and publishes the draft once both Docker + CLI workflows succeed for the tag. Body assembly: prepends the AI Highlights block (stable releases only) extracted from the merged release-please PR body via the head_sha → pulls association, then re-applies the Verification section from the per-image marker comments (``, ``, etc.). The strip step that prevents finalize re-runs from doubling sections gates EVERY marker-pair deletion on both START and END being present in the body; `sed '/START/,/END/d'` is greedy to EOF without an END, which would tank the entire CHANGELOG-derived body if a contributor's commit subject (now propagated verbatim into dev release bodies via `dev-release.yml`) happened to contain a literal opening marker. The gate applies to HIGHLIGHTS and to all five `CLI_*` / `CONTAINER_*` verification-data marker pairs. The `FINALIZE_VERIFICATION` marker is intentionally greedy-to-EOF: everything after it IS the verification section, rebuilt fresh on each finalize run. Posts a `finalize-release` commit status (`pending` at start, `success` / `failure` at finish) so workflow_run-triggered failures surface as a red ❌ on the commit row instead of disappearing into the Actions tab. Gated by the `release` deployment environment. Immutable releases enabled. Handles both stable and dev releases. Deletes all dev pre-releases and tags when a stable release is published. Artifact smoke testing happens at BUILD time in `cli.yml` and `docker.yml` via the `smoke-test-cli-binary` and `smoke-test-backend-image` composite actions; the finalize step does not re-test (Docker images are content-addressed and CLI archives are SHA-256-verified by the cosign-signed `checksums.txt`). +- **Dev Release**: `dev-release.yml`: creates semver dev tags (e.g. `v0.4.7-dev.3`) and draft pre-releases on every push to main (skips Release Please version-bump commits). Tags trigger existing Docker + CLI workflows for full build/scan/sign pipeline. Gated by the `release` deployment environment. Uses the `release-runner-setup` composite for token mint. Pre-release body is built locally via `git log -1` on the head SHA and `gh release create --notes-file`: title `$DEV_TAG` (e.g. `v0.7.2-dev.5`), then a `Dev build #N toward vX.Y.Z` line, `**Commit:** `, `**Subject:** `, the `**Full pipeline:**` disclaimer, and the channel opt-in tip. Only the short SHA and the commit subject are written into the notes file -- the full commit body (squash-merge PR descriptions of hundreds of lines, nested markdown, tables) is deliberately omitted because it renders poorly on the release page and buries what changed. Variables go through `printf '%s'` placeholders (the `--notes-file` route avoids command substitution that bare `--notes "..."` would suffer if a commit subject contained backticks or `$(...)`). Failure path: if `gh release create` returns non-zero (transient API error, 5xx, rate limit), the workflow exits 1 with the orphan tag preserved -- deleting the tag would race the downstream `tags: v*`-listening workflows that the tag-create push already triggered (cli.yml, docker.yml), 404'ing their `actions/checkout` step. The orphan tag is later garbage-collected by the same workflow's incremental sweep (keeps 5 most recent dev pre-releases) and by `finalize-release.yml`'s stable-release sweep. End-of-job regression guard `Verify minted tag survived the run` always re-resolves `refs/tags/$DEV_TAG` (via `if: always()` so the guard runs on failure paths where tag loss is most likely) and exits 1 if absent, routing through the existing `report-failure` job into the `dev-release regression` tracking issue. Workflow-tag-lifecycle pre-push gate (`scripts/check_workflow_tag_lifecycle.py`) statically prevents any future workflow from re-introducing the create-then-conditionally-delete shape. +- **Finalize Release**: `finalize-release.yml`: assembles the release body and publishes the draft once both Docker + CLI workflows succeed for the tag. Body assembly: prepends the AI Highlights block (stable releases only) extracted from the merged release-please PR body via the head_sha → pulls association, then re-applies the Verification section from the per-image marker comments (``, ``, etc.). The strip step that prevents finalize re-runs from doubling sections gates EVERY marker-pair deletion on both START and END being present in the body; `sed '/START/,/END/d'` is greedy to EOF without an END, which would tank the entire CHANGELOG-derived body if a contributor's commit subject (now propagated verbatim into dev release bodies via `dev-release.yml`) happened to contain a literal opening marker. The gate applies to HIGHLIGHTS and to all five `CLI_*` / `CONTAINER_*` verification-data marker pairs. The `FINALIZE_VERIFICATION` marker is intentionally greedy-to-EOF: everything after it IS the verification section, rebuilt fresh on each finalize run. Posts a `finalize-release` commit status (`pending` at start, `success` / `failure` at finish) so workflow_run-triggered failures surface as a red ❌ on the commit row instead of disappearing into the Actions tab. Gated by the `release` deployment environment. Immutable releases enabled. Handles both stable and dev releases. Stable-release dev-cleanup deletes every dev release + every orphan dev tag matching `vX.Y.Z-dev.N` -- the inner `gh api` calls are explicitly capture-and-checked (NOT `mapfile < <(...)`, which silently treats inner-process failures as empty input) and per-tag `gh release delete` / `gh api -X DELETE` failures accumulate into a final exit-on-failure check so partial-cleanup is loudly diagnosed. The Highlights propagation path that fetches the release-please PR body splits the `gh pr view` call into capture + classify so an auth / rate-limit failure surfaces a `::warning::` distinct from "PR was deleted" (legitimate skip with `::notice::`). Artifact smoke testing happens at BUILD time in `cli.yml` and `docker.yml` via the `smoke-test-cli-binary` and `smoke-test-backend-image` composite actions; the finalize step does not re-test (Docker images are content-addressed and CLI archives are SHA-256-verified by the cosign-signed `checksums.txt`). - **CI failure-surfacing policy**: every CI workflow must surface its outcome somewhere visible. Non-schedule failure paths (push / pull_request / workflow_run / release / dispatch) post a commit status or PR check; schedule failure paths open or update a tracking GitHub Issue labeled `automation:ci-health`. Schedule-triggered workflows have no commit context to attach to, hence the issue lane; manual `workflow_dispatch` runs surface failures in the run UI directly so they do not open issues. The shared composite is `.github/actions/post-tracking-issue`; it dedupes by title across all states (open + closed), so a regression that reappears reopens the same tracker rather than creating a duplicate; consumers that auto-close on success (e.g. `ci-preflight.yml`) should also unpin in the close path so a closed-and-resolved issue does not stay in the pinned row. Workflows currently using this pattern: `apko-lock.yml`, `ci-preflight.yml`, `dast.yml`, `python-audit.yml`, `evals.yml`, `scorecard.yml`, `secret-scan.yml`. Pinned tracking-issue label: `automation:ci-health`. Success events (stable release published, dev pre-release cut, auto-rollover success) deliberately do NOT generate notifications; the GitHub Releases tab and commit row already surface those, and posting them would just spam the tracker. - **SBOM Diff**: `sbom-diff.yml`: inform-only sticky PR comment on Release Please release PRs. Added / removed components + license category counts from the head backend SBOM vs last stable. `dependency-review.yml` remains the license gate; this comment is advisory. diff --git a/scripts/check_workflow_tag_lifecycle.py b/scripts/check_workflow_tag_lifecycle.py new file mode 100644 index 0000000000..46d5398f81 --- /dev/null +++ b/scripts/check_workflow_tag_lifecycle.py @@ -0,0 +1,275 @@ +#!/usr/bin/env python3 +"""Pre-push gate: block tag-vs-downstream-checkout races. + +A workflow that BOTH creates a tag via ``gh api .../git/refs`` POST AND +conditionally deletes that same tag via ``gh api -X DELETE +.../git/refs/tags/...`` (in the same step or across steps in the same +workflow file) reproduces the race documented in #1818: + +1. Step 1 creates ``refs/tags/$DEV_TAG`` via ``gh api .../git/refs``. + The ref-create fires ``push`` events to every ``tags: v*``-listening + workflow (currently ``cli.yml`` and ``docker.yml``); those workflows + begin running on hosted runners and ``actions/checkout`` the new + tag within seconds. +2. Step 2 (or a later step in the same workflow) attempts a follow-on + operation -- ``gh release create``, asset upload, etc. +3. On failure, a cleanup branch deletes the tag with ``gh api -X + DELETE``. +4. Downstream tag-triggered runs already in flight 404 on + ``actions/checkout`` while fetching the just-deleted ref. Result: + false-positive red on ``main`` even when the squash content is + clean. + +The convention this gate enforces: + + A workflow MAY create tags. A workflow MAY delete tags. It MAY NOT + do BOTH within the same workflow file -- the cleanup must happen in + a separate workflow that does not also produce the original tag, so + its delete cannot race a producer-side race window. + +Reference fix: ``dev-release.yml`` after #1818 (release-create failure +preserves the orphan tag; the existing stale-pre-release sweeper + +``finalize-release.yml``'s stable-release sweep garbage-collect it +later in separate workflow runs that do NOT also mint tags). + +This is a no-baseline gate: a NEW convention should pass clean from day +one. If this gate flags an existing workflow, fix the workflow -- do +NOT add a baseline. The MED severity audit pass for #1818 confirmed +the post-fix repo passes clean. + +Usage:: + + python scripts/check_workflow_tag_lifecycle.py ... # pre-commit + python scripts/check_workflow_tag_lifecycle.py --scan-all # CI / manual +""" + +import argparse +import re +import sys +from pathlib import Path +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Iterable + +_REPO_ROOT = Path(__file__).resolve().parent.parent +_WORKFLOWS_ROOT = _REPO_ROOT / ".github" / "workflows" + +# CREATE: ``gh api`` invocation hitting the ``/git/refs`` endpoint with +# a ``-f ref="refs/tags/...`` or ``-F ref=refs/tags/...`` flag. ``gh +# api`` infers POST when any field flag is present, so this captures +# the create-ref shape regardless of which field-flag form is used. +# +# Tolerance built into the pattern: +# +# - ``git/refs[^/\w]`` requires that ``git/refs`` be the leaf endpoint +# (followed by quote / whitespace / EOL), NOT a path segment as in +# ``git/refs/tags/...``. This excludes the DELETE shape, which always +# has a trailing path component. +# - ``-[fF]`` matches both ``-f`` (raw string field) and ``-F`` (typed +# field); both flags POST to the same endpoint and both produce a +# working tag-create call, so missing one would let any future +# workflow bypass the gate by switching flag form. +# - Backslash-newline continuations (``(?:[^\n]|\\\n)``) are tolerated +# between every pair of tokens so a multi-line invocation cannot +# bypass the gate just by wrapping. The 500 / 300 char windows cap +# pairing distance so an unrelated downstream ``-[fF]`` cannot bind +# to a distant ``git/refs`` reference in the same run block. +# - Argument order is not fixed: ``-[fF] ref=refs/tags/...`` may appear +# either AFTER the endpoint (the common form) or BEFORE it. Both +# orderings hit the same POST endpoint, so both are matched. +# - Anchored on ``refs/tags/`` so a heads-ref create (``refs/heads/...``) +# does not falsely trip this gate. +_TAG_CREATE_RE = re.compile( + r"gh\s+api(?:[^\n]|\\\n){0,500}?" + r"(?:" + r"git/refs[^/\w](?:[^\n]|\\\n){0,300}?-[fF]\s+ref=[\"']?refs/tags/" + r"|" + r"-[fF]\s+ref=[\"']?refs/tags/(?:[^\n]|\\\n){0,300}?git/refs[^/\w]" + r")", + re.MULTILINE, +) + +# DELETE: any of the three ways a workflow can drop a tag the same job +# (or an earlier job in the same workflow) just minted: +# +# 1. ``gh api -X DELETE .../git/refs/tags/...`` (the empirical #1818 +# reproducer; the App-token authenticated call that fires the +# downstream-cancelling ref-delete event). +# 2. ``gh api --method DELETE`` -- semantically identical to ``-X DELETE``. +# 3. ``gh release delete --cleanup-tag`` -- the GitHub CLI's release +# command, which deletes both the release AND its tag in one call. +# +# Backslash-newline continuations are tolerated between every pair of +# tokens so a multi-line invocation cannot bypass the gate by wrapping; +# the 300 / 200 char windows cap pairing distance so unrelated calls +# in the same run block do not bind together. +# +# Per-line opt-out: a ``# lint-allow: workflow-tag-lifecycle -- `` +# comment on the line containing the offending pattern (or any line the +# multi-line match spans) suppresses the report. Use this only when the +# delete provably targets tags whose downstream workflows have already +# completed (e.g. bulk-pruning of N-revisions-old dev tags), not the +# just-minted tag. +_TAG_DELETE_RE = re.compile( + r"(?:" + r"gh\s+api(?:[^\n]|\\\n){0,300}?" + r"(?:-X|--method)\s+DELETE(?:[^\n]|\\\n){0,200}?" + r"git/refs/tags/" + r"|" + r"gh\s+release\s+delete(?:[^\n]|\\\n){0,200}?--cleanup-tag" + r")", + re.MULTILINE, +) + +# Strip full-line shell comments (``^[ \t]*#...``) before regex +# matching so a documented example in a ``run:`` block (``# example: +# gh api -X DELETE git/refs/tags/foo``) does not trip the gate. In-line +# ``#`` is left alone -- it is almost always part of a quoted string, +# not a comment. The leading character class is ``[ \t]*`` (NOT +# ``\s*``) on purpose: ``\s`` includes ``\n``, which would let the +# scrubber greedy-match a preceding blank line's newline along with +# the comment line's content, dropping a row from the file and +# silently shifting every later line number reported by the gate. +_SHELL_COMMENT_RE = re.compile(r"(?m)^[ \t]*#[^\n]*$") + +# Per-line opt-out marker. Mandatory non-empty justification after +# ``--`` (whitespace-only is rejected). Detected on any of the line(s) +# the matched pattern spans; one opt-out is enough to suppress the +# report for that match. +_OPT_OUT_RE = re.compile( + r"#\s*lint-allow:\s*workflow-tag-lifecycle\s*--\s*\S", +) + +_STEERING_MESSAGE = ( + "Tag CREATE + conditional DELETE within a single workflow races " + "downstream `tags: v*`-listening workflows on actions/checkout (#1818). " + "Move the cleanup to a separate workflow that does not also produce " + "the original tag, or leave orphan tags for the existing dev-pre-" + "release sweeper to collect. Reference fix: dev-release.yml after " + "#1818." +) + + +def _iter_workflow_files() -> Iterable[Path]: + """Walk ``.github/workflows/`` for YAML files.""" + if not _WORKFLOWS_ROOT.exists(): + return + for pattern in ("*.yml", "*.yaml"): + yield from sorted(_WORKFLOWS_ROOT.rglob(pattern)) + + +def _match_is_opted_out( + scrubbed: str, raw_lines: list[str], match: re.Match[str] +) -> bool: + """Return True if any line the match spans carries the opt-out marker. + + The opt-out check runs against the ORIGINAL source (``raw_lines``), + not the scrubbed copy, so a per-line ``# lint-allow:`` comment is + visible to the check even though the scrubber would normally strip + full-line comments. The match's start/end are still computed from + the scrubbed copy, but since the scrubber preserves newlines (it + zeroes the text only) the 1-indexed line numbers align across both + copies. + """ + start_line = scrubbed[: match.start()].count("\n") + 1 + span_lines = scrubbed[match.start() : match.end()].count("\n") + 1 + end_line = start_line + span_lines - 1 + return any( + _OPT_OUT_RE.search(raw_lines[line_no - 1]) + for line_no in range(start_line, end_line + 1) + if 0 < line_no <= len(raw_lines) + ) + + +def _scan_file(path: Path) -> tuple[list[int], list[int]]: + """Return ``(create_lines, delete_lines)`` 1-indexed line numbers. + + Caller flags the workflow when both lists are non-empty. + + ``_SHELL_COMMENT_RE`` substitution preserves newlines (it only + zeros the comment text), so line numbers in the scrubbed source + stay aligned with the original file. + """ + raw = path.read_text(encoding="utf-8") + raw_lines = raw.splitlines() + scrubbed = _SHELL_COMMENT_RE.sub("", raw) + creates: list[int] = [] + for match in _TAG_CREATE_RE.finditer(scrubbed): + if _match_is_opted_out(scrubbed, raw_lines, match): + continue + line_no = scrubbed[: match.start()].count("\n") + 1 + creates.append(line_no) + deletes: list[int] = [] + for match in _TAG_DELETE_RE.finditer(scrubbed): + if _match_is_opted_out(scrubbed, raw_lines, match): + continue + line_no = scrubbed[: match.start()].count("\n") + 1 + deletes.append(line_no) + return creates, deletes + + +def _scan_paths(paths: Iterable[Path]) -> int: + """Scan each path; return shell exit code.""" + violations: list[tuple[Path, list[int], list[int]]] = [] + for path in paths: + if not path.exists() or path.suffix not in (".yml", ".yaml"): + continue + creates, deletes = _scan_file(path) + if creates and deletes: + violations.append((path, creates, deletes)) + if not violations: + return 0 + for path, creates, deletes in violations: + resolved = path.resolve() + try: + rel = resolved.relative_to(_REPO_ROOT).as_posix() + except ValueError: + # Path is outside the repo root (e.g. an ad-hoc test + # invocation against a tempfile). Fall back to the + # absolute path so the reporter still produces a + # navigable file:line citation. + rel = resolved.as_posix() + print( + f"\n{rel}: tag CREATE and conditional DELETE in same workflow", + file=sys.stderr, + ) + for line in creates: + print(f" CREATE at {rel}:{line}", file=sys.stderr) + for line in deletes: + print(f" DELETE at {rel}:{line}", file=sys.stderr) + print(f"\n{_STEERING_MESSAGE}", file=sys.stderr) + return 1 + + +def main(argv: list[str] | None = None) -> int: + """CLI entry point.""" + parser = argparse.ArgumentParser( + description="Block tag-vs-downstream-checkout races in workflows.", + ) + parser.add_argument( + "paths", + nargs="*", + help="Files to check (pre-commit supplies these).", + ) + parser.add_argument( + "--scan-all", + action="store_true", + help="Scan every workflow file (CI / manual mode).", + ) + args = parser.parse_args(argv) + + if args.scan_all: + targets = list(_iter_workflow_files()) + elif args.paths: + targets = [Path(p).resolve() for p in args.paths] + else: + # Default to scan-all when invoked without args, matching the + # convention of the sibling check_workflow_shell_git_commits.py + # gate. + targets = list(_iter_workflow_files()) + return _scan_paths(targets) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/unit/scripts/test_check_workflow_tag_lifecycle.py b/tests/unit/scripts/test_check_workflow_tag_lifecycle.py new file mode 100644 index 0000000000..4e153458f8 --- /dev/null +++ b/tests/unit/scripts/test_check_workflow_tag_lifecycle.py @@ -0,0 +1,358 @@ +"""Unit tests for ``scripts/check_workflow_tag_lifecycle.py``. + +Loads the script as a module so its private helpers are callable +without spawning subprocesses. + +Covers: + +* Positive matches: ``gh api -X DELETE``, ``gh api --method DELETE``, + ``gh release delete --cleanup-tag``, multi-line line continuations + on both create and delete sides, and reversed ``-f ref=`` / endpoint + argument order. +* Negative cases: heads-ref creates (``refs/heads/...``), pure + create-only or delete-only workflows, full-line shell comments that + contain delete-shaped strings. +* Per-line opt-out (``# lint-allow: workflow-tag-lifecycle -- + ``) with mandatory non-empty justification. +* The ``_SHELL_COMMENT_RE`` line-number-preservation fix (using + ``[ \t]*`` instead of ``\\s*`` so blank lines preceding comment + blocks don't get coalesced). +""" + +import importlib.util +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.unit + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent.parent +_SCRIPT_PATH = _REPO_ROOT / "scripts" / "check_workflow_tag_lifecycle.py" + + +def _load_script_module() -> object: + """Import the script as a module so private helpers are callable.""" + spec = importlib.util.spec_from_file_location( + "_check_workflow_tag_lifecycle", + _SCRIPT_PATH, + ) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +_MODULE = _load_script_module() + + +def _scan(tmp_path: Path, content: str) -> tuple[list[int], list[int]]: + """Write content to a tmp .yml file and return ``(creates, deletes)``.""" + target = tmp_path / "wf.yml" + target.write_text(content, encoding="utf-8") + creates, deletes = _MODULE._scan_file(target) # type: ignore[attr-defined] + return creates, deletes + + +# --------------------------------------------------------------------------- # +# Positive create matches +# --------------------------------------------------------------------------- # + + +class TestCreateMatching: + """The CREATE regex catches tag-create shapes.""" + + def test_single_line_create(self, tmp_path: Path) -> None: + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: gh api repos/foo/bar/git/refs -f ref=refs/tags/v1\n" + ) + creates, deletes = _scan(tmp_path, content) + assert creates == [4] + assert deletes == [] + + def test_multi_line_create_continuation(self, tmp_path: Path) -> None: + """Backslash-newline continuations between args don't bypass.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: |\n" + ' gh api "repos/foo/bar/git/refs" \\\n' + ' -f ref="refs/tags/v1" \\\n' + ' -f sha="$SHA"\n' + ) + creates, deletes = _scan(tmp_path, content) + assert len(creates) == 1 + assert deletes == [] + + def test_reversed_argument_order(self, tmp_path: Path) -> None: + """``-f ref=`` BEFORE the endpoint still hits the same POST.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: |\n" + ' gh api -f ref="refs/tags/v1" \\\n' + ' "repos/foo/bar/git/refs"\n' + ) + creates, _deletes = _scan(tmp_path, content) + assert len(creates) == 1 + + def test_typed_field_capital_f_flag(self, tmp_path: Path) -> None: + """``-F ref=`` (typed field) is the same POST; gate must catch it.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: |\n" + " gh api repos/foo/bar/git/refs \\\n" + " -F ref=refs/tags/v1 \\\n" + " -F sha=$GITHUB_SHA\n" + ) + creates, _deletes = _scan(tmp_path, content) + assert len(creates) == 1 + + def test_typed_field_capital_f_reversed_order(self, tmp_path: Path) -> None: + """``-F ref=`` BEFORE the endpoint still matches.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: gh api -F ref=refs/tags/v1 repos/foo/bar/git/refs\n" + ) + creates, _deletes = _scan(tmp_path, content) + assert len(creates) == 1 + + +class TestCreateNonMatches: + """The CREATE regex doesn't trip on look-alikes.""" + + def test_heads_ref_create_is_not_a_tag_create(self, tmp_path: Path) -> None: + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: gh api repos/foo/bar/git/refs -f ref=refs/heads/main\n" + ) + creates, deletes = _scan(tmp_path, content) + assert creates == [] + assert deletes == [] + + def test_path_segment_not_endpoint(self, tmp_path: Path) -> None: + """``git/refs/heads/...`` is not the create endpoint.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + ' - run: echo "git/refs/tags/v1 is just a string"\n' + ) + creates, _deletes = _scan(tmp_path, content) + assert creates == [] + + +# --------------------------------------------------------------------------- # +# Positive delete matches +# --------------------------------------------------------------------------- # + + +class TestDeleteMatching: + """The DELETE regex catches every tag-delete shape.""" + + def test_gh_api_x_delete_single_line(self, tmp_path: Path) -> None: + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + ' - run: gh api -X DELETE "repos/foo/bar/git/refs/tags/v1"\n' + ) + _creates, deletes = _scan(tmp_path, content) + assert deletes == [4] + + def test_gh_api_method_delete(self, tmp_path: Path) -> None: + """``--method DELETE`` is semantically identical to ``-X DELETE``.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + ' - run: gh api --method DELETE "repos/foo/bar/git/refs/tags/v1"\n' + ) + _creates, deletes = _scan(tmp_path, content) + assert len(deletes) == 1 + + def test_gh_release_delete_cleanup_tag(self, tmp_path: Path) -> None: + """``gh release delete --cleanup-tag`` deletes both release + tag.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: gh release delete v1 --yes --cleanup-tag\n" + ) + _creates, deletes = _scan(tmp_path, content) + assert len(deletes) == 1 + + def test_multi_line_delete_continuation(self, tmp_path: Path) -> None: + """Backslash-newline continuations between args don't bypass.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: |\n" + " gh api \\\n" + " -X DELETE \\\n" + ' "repos/foo/bar/git/refs/tags/v1"\n' + ) + _creates, deletes = _scan(tmp_path, content) + assert len(deletes) == 1 + + +class TestDeleteNonMatches: + """The DELETE regex doesn't trip on look-alikes.""" + + def test_release_delete_without_cleanup_tag(self, tmp_path: Path) -> None: + """Release-only delete (no ``--cleanup-tag``) leaves the tag alone.""" + content = "jobs:\n x:\n steps:\n - run: gh release delete v1 --yes\n" + _creates, deletes = _scan(tmp_path, content) + assert deletes == [] + + +# --------------------------------------------------------------------------- # +# Combined CREATE + DELETE reports both +# --------------------------------------------------------------------------- # + + +class TestCombined: + """When both shapes are present in the same file, both line lists fill.""" + + def test_create_and_delete_both_reported(self, tmp_path: Path) -> None: + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: |\n" + " gh api repos/foo/bar/git/refs -f ref=refs/tags/v1\n" + ' gh api -X DELETE "repos/foo/bar/git/refs/tags/v1"\n' + ) + creates, deletes = _scan(tmp_path, content) + assert len(creates) == 1 + assert len(deletes) == 1 + + +# --------------------------------------------------------------------------- # +# Shell-comment scrubbing (regression: line-number preservation) +# --------------------------------------------------------------------------- # + + +class TestShellCommentScrub: + """Full-line shell comments are stripped without shifting line numbers.""" + + def test_documented_example_in_comment_does_not_trip(self, tmp_path: Path) -> None: + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: |\n" + " # example: gh api -X DELETE git/refs/tags/foo\n" + " echo no-op\n" + ) + _creates, deletes = _scan(tmp_path, content) + assert deletes == [] + + def test_blank_line_before_comment_block_preserves_line_numbers( + self, tmp_path: Path + ) -> None: + """Regression: ``\\s*`` previously ate the blank-line newline.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: |\n" + "\n" + " # comment block line 1\n" + " # comment block line 2\n" + " gh api repos/foo/bar/git/refs -f ref=refs/tags/v1\n" + ) + creates, _deletes = _scan(tmp_path, content) + # The create is on raw line 8; the line number reported must + # match the source-of-truth line, not a coalesced offset. + assert creates == [8] + + +# --------------------------------------------------------------------------- # +# Per-line opt-out +# --------------------------------------------------------------------------- # + + +class TestOptOut: + """The ``# lint-allow: workflow-tag-lifecycle -- `` opt-out.""" + + def test_opt_out_with_reason_suppresses_match(self, tmp_path: Path) -> None: + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: gh release delete v1 --yes --cleanup-tag " + "# lint-allow: workflow-tag-lifecycle -- bulk cleanup of old tags\n" + ) + _creates, deletes = _scan(tmp_path, content) + assert deletes == [] + + def test_opt_out_without_reason_rejected(self, tmp_path: Path) -> None: + """Whitespace-only reason after ``--`` doesn't suppress.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: gh release delete v1 --yes --cleanup-tag " + "# lint-allow: workflow-tag-lifecycle -- \n" + ) + _creates, deletes = _scan(tmp_path, content) + assert len(deletes) == 1 + + def test_opt_out_for_unrelated_gate_does_not_suppress(self, tmp_path: Path) -> None: + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: gh release delete v1 --yes --cleanup-tag " + "# lint-allow: some-other-gate -- not this one\n" + ) + _creates, deletes = _scan(tmp_path, content) + assert len(deletes) == 1 + + def test_opt_out_spans_multi_line_match(self, tmp_path: Path) -> None: + """Marker on any line the match spans is enough.""" + content = ( + "jobs:\n" + " x:\n" + " steps:\n" + " - run: |\n" + " gh api \\\n" + " -X DELETE \\\n" + ' "repos/foo/bar/git/refs/tags/v1" ' + "# lint-allow: workflow-tag-lifecycle -- documented exception\n" + ) + _creates, deletes = _scan(tmp_path, content) + assert deletes == [] + + +# --------------------------------------------------------------------------- # +# Repo-level smoke: every shipped workflow passes the gate +# --------------------------------------------------------------------------- # + + +class TestRepoSmoke: + """The repository's own workflows must pass the gate.""" + + def test_all_repo_workflows_pass(self) -> None: + workflows = list((_REPO_ROOT / ".github" / "workflows").rglob("*.yml")) + list( + (_REPO_ROOT / ".github" / "workflows").rglob("*.yaml") + ) + offenders: list[tuple[str, list[int], list[int]]] = [] + for wf in workflows: + creates, deletes = _MODULE._scan_file(wf) # type: ignore[attr-defined] + if creates and deletes: + offenders.append((wf.name, creates, deletes)) + assert offenders == [], f"Workflows with create+delete pattern: {offenders}"