From 7987c8702a1f97f134ad923380b0839bfb590686 Mon Sep 17 00:00:00 2001 From: Ethan Davidson <31261035+EthanThatOneKid@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:02:49 -0700 Subject: [PATCH] fix: stop treating a blank line as a job boundary in contract lint scan_jobs() ended a job block at the first line indented less than two spaces, and indent("") is 0, so a blank line ended the block. Blank lines are ordinary inside `run: |` bodies, so every rule violation after the first one in a job was invisible. wazoo-console's cleanup-preview job lost its R5 finding exactly that way, and its deploy-preview job lost R1 and R5. Add a credential-free bash self-test that pins the boundary, run it from CI, and record why it is not part of `deno task ci`. Closes #163 --- .github/workflows/ci.yml | 2 + skills/contract-lint/README.md | 18 ++++ skills/contract-lint/lint.sh | 5 + skills/contract-lint/lint_test.sh | 154 ++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+) create mode 100644 skills/contract-lint/lint_test.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e7a8337..90e6dfe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,3 +17,5 @@ jobs: with: deno-version: v2.x - run: deno task ci + - name: Contract lint self-test + run: bash skills/contract-lint/lint_test.sh diff --git a/skills/contract-lint/README.md b/skills/contract-lint/README.md index fd0325d..82c558f 100644 --- a/skills/contract-lint/README.md +++ b/skills/contract-lint/README.md @@ -38,3 +38,21 @@ Local run: ``` Exits 0 when clean, 1 on any violation. + +## Tests + +```sh +bash skills/contract-lint/lint_test.sh +``` + +`lint_test.sh` is a credential-free bash self-test: it writes workflow fixtures +into a temp dir and asserts the exit code, the rule, and the job attribution for +each case. One case pins the job-block boundary — a blank line inside a `run: |` +body is not the end of a job. Treating it as one hid every later violation in +that job, which is what +[workspace-cli#163](https://github.com/wazootech/workspace-cli/issues/163) +reported. + +CI runs it as its own step rather than through `deno task ci`, because the +documented `deno task ci` is expected to work on a bare Windows checkout and +this test is bash. diff --git a/skills/contract-lint/lint.sh b/skills/contract-lint/lint.sh index 7f16820..fc4189b 100644 --- a/skills/contract-lint/lint.sh +++ b/skills/contract-lint/lint.sh @@ -101,6 +101,11 @@ scan_jobs() { next } if (jname != "") { + # A blank line is not a job boundary. indent("") is 0, so the check + # below used to end the block on one, and run: | bodies contain them. + # Treating one as the boundary hid every later violation in that job + # (#163). + if ($0 ~ /^[[:space:]]*$/) next if (indent($0) < 2) { finish_job(); next } lines[n++] = $0 } diff --git a/skills/contract-lint/lint_test.sh b/skills/contract-lint/lint_test.sh new file mode 100644 index 0000000..fd7c804 --- /dev/null +++ b/skills/contract-lint/lint_test.sh @@ -0,0 +1,154 @@ +#!/usr/bin/env bash +# Regression tests for lint.sh. Credential-free: writes fixtures to a temp dir. +# +# bash skills/contract-lint/lint_test.sh +# +# Background (issue #163): scan_jobs() used to end a job block at the first line +# indented less than two spaces, and indent("") is 0, so a blank line counted as +# a job boundary. Blank lines are ordinary inside `run: |` bodies, so every rule +# violation after the first one in a job was invisible — wazoo-console's +# cleanup-preview job lost its R5 finding exactly that way. +set -euo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +lint="$here/lint.sh" +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT + +passed=0 +failed=0 + +# check ... — the YAML fixture arrives on +# stdin. A wanted substring prefixed with "!" must be absent from the output. +check() { + local name="$1" want_rc="$2" + shift 2 + local dir="$tmp/case.$((passed + failed))" + mkdir -p "$dir" + cat > "$dir/ci.yml" + + local out rc + set +e + out="$(bash "$lint" --dir "$dir" 2>&1)" + rc=$? + set -e + + local bad="" + [[ "$rc" == "$want_rc" ]] || bad="exit $rc, want $want_rc" + local want + for want in "$@"; do + if [[ "$want" == "!"* ]]; then + [[ "$out" == *"${want#!}"* ]] && bad="${bad:+$bad; }unexpected: ${want#!}" + else + [[ "$out" == *"$want"* ]] || bad="${bad:+$bad; }missing: $want" + fi + done + + if [[ -n "$bad" ]]; then + printf 'not ok - %s (%s)\n%s\n' "$name" "$bad" "$out" >&2 + failed=$((failed + 1)) + else + printf 'ok - %s\n' "$name" + passed=$((passed + 1)) + fi +} + +check "R5 survives a blank line inside a PR job" 1 \ + "FAIL [R5]" "[cleanup-preview]" "1 violation(s)" <<'YAML' +name: fixture +on: pull_request +jobs: + cleanup-preview: + if: github.event_name == 'pull_request' && github.event.action == 'closed' + runs-on: ubuntu-latest + steps: + - run: | + echo "deleting the preview worker" + + echo "done" + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} + CLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }} +YAML + +check "R5 still fires without a blank line" 1 \ + "FAIL [R5]" "[cleanup-preview]" "1 violation(s)" <<'YAML' +name: fixture +on: pull_request +jobs: + cleanup-preview: + if: github.event_name == 'pull_request' && github.event.action == 'closed' + runs-on: ubuntu-latest + steps: + - run: | + echo "deleting the preview worker" + echo "done" + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} +YAML + +check "a blank line does not merge the next job into the previous one" 1 \ + "[cleanup-preview]" "1 violation(s)" "![verify]" <<'YAML' +name: fixture +on: pull_request +jobs: + verify: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + steps: + - run: | + echo "verify" + + echo "more verification" + cleanup-preview: + if: github.event_name == 'pull_request' && github.event.action == 'closed' + runs-on: ubuntu-latest + steps: + - run: echo cleanup + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} +YAML + +check "R1 and R5 both survive a blank line in a deploy job" 1 \ + "FAIL [R1]" "FAIL [R5]" "[deploy-preview]" "2 violation(s)" <<'YAML' +name: fixture +on: pull_request +jobs: + deploy-preview: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + steps: + - run: | + echo "deploying the preview" + + echo "done" + env: + CLOUDFLARE_API_TOKEN: ${{ secrets.CLOUDFLARE_API_TOKEN }} +YAML + +check "a conformed deploy job passes" 0 "0 violation(s)" <<'YAML' +name: fixture +on: + push: + branches: [main] +jobs: + deploy-qa: + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + runs-on: ubuntu-latest + steps: + - name: Fetch Cloudflare deploy token from Infisical (qa) + uses: Infisical/secrets-action@v1.0.17 + with: + method: oidc + secret-name: CLOUDFLARE_API_TOKEN + + - run: pnpm exec wrangler deploy --env qa + env: + CLOUDFLARE_ACCOUNT_ID: ${{ vars.CLOUDFLARE_ACCOUNT_ID }} +YAML + +if ((failed)); then + printf '%d of %d cases failed\n' "$failed" "$((passed + failed))" >&2 + exit 1 +fi +printf 'all %d cases passed\n' "$passed"