From e188ee03a4b64be1f6105b24d395438afe71f7a7 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Wed, 17 Jun 2026 16:30:48 -0400 Subject: [PATCH] fix(scripts/check_emdash.sh): skip emdash check when no diff base is available (#26489) ## Problem `scripts/check_emdash.sh` is a diff gate for pull requests: it resolves the merge-base against the target branch and only inspects added lines. When it cannot resolve a base ref, it fell back to scanning **every tracked file**. Push builds on release branches hit exactly this case: the `lint` job checks out with `fetch-depth: 1`, so `origin/main` is absent, and `GITHUB_BASE_REF` is only set for `pull_request` events. With no base ref, the whole-tree scan flags the many pre-existing emdash/endash characters already in the repo and fails `make lint` (`lint/emdash`), even though the build introduced none of them. Observed on `release/2.34` CI (run [27704528068](https://github.com/coder/coder/actions/runs/27704528068/job/81949529546)). ## Fix When no base ref can be determined (i.e. outside a pull request), skip the check instead of scanning the entire tree. A full scan remains available on demand via `scripts/check_emdash.sh --all`. ## Testing - **No base ref** (release-push simulation, no `GITHUB_BASE_REF`, no `origin/main`): old script scans all files and fails on a pre-existing emdash; new script skips and exits 0. - **PR path** (diff vs merge-base): `OK: no emdash or endash characters found.` - **`--all`**: still scans the full tree (flags pre-existing characters as before). - `shellcheck` and `shfmt` clean. ## Backports Backport PRs target `release/2.33` and `release/2.34` (same bug, older script variant). `release/2.29` and `release/2.32` do **not** contain `scripts/check_emdash.sh`, so there is nothing to backport there.
Decision log Considered alternatives to the skip: 1. **Compare against `github.event.before`** on push events. Rejected: the before-SHA is frequently unreachable in a `fetch-depth: 1` clone, and wiring it in requires per-workflow env changes that complicate backports. 2. **Fetch `origin/main` / deepen history** in the release lint job. Rejected for the same backport-surface reason and because it only masks the design intent. The check exists to stop *new* emdashes from landing via PRs; that gate already ran on the originating PRs. On non-PR builds there is no meaningful diff base, so skipping is correct and self-contained in the script (clean to backport). The explicit `--all` mode is preserved for intentional full-tree audits.
--- Generated by Coder Agents on behalf of @f0ssel. --- scripts/check_emdash.sh | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/scripts/check_emdash.sh b/scripts/check_emdash.sh index 2b95fd4584..71d1728637 100755 --- a/scripts/check_emdash.sh +++ b/scripts/check_emdash.sh @@ -175,12 +175,19 @@ else exit 1 } if [[ -z "$base" ]]; then - echo "WARNING: no base ref found, scanning all tracked files." >&2 - scan_all_files - else - found=0 - scan_diff "$base" + # No base ref is available outside of pull requests, for + # example push builds on release branches with a shallow clone + # where neither GITHUB_BASE_REF nor origin/main is present. + # The diff check has nothing to compare against, and scanning + # every tracked file would flag pre-existing characters that + # are unrelated to the change under test. Skip instead; pass + # --all to force a full-tree scan. + echo "OK: no base ref found (not a pull request); skipping emdash check." + echo " Pass --all to scan every tracked file." + exit 0 fi + found=0 + scan_diff "$base" fi if [[ "$found" -ne 0 ]]; then