From 9f211ce5aecc93bd9edf3c923b7614998c7c51a8 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 26 Jun 2026 14:34:49 +0200 Subject: [PATCH] fix: use `HEAD` instead of fetching base branch for `emdash` linter (#26733) ## Problem The `lint/emdash` check fails on Graphite-stacked PRs. See [this failed run](https://github.com/coder/coder/actions/runs/28225390084/job/83616080375?pr=26650): ``` Base ref origin/graphite-base/26650 not found locally, fetching graphite-base/26650... ERROR: could not fetch base ref origin/graphite-base/26650. ERROR: could not determine base ref. make: *** [Makefile:768: lint/emdash] Error 1 ``` `scripts/check_emdash.sh` resolved its diff base by fetching `origin/$GITHUB_BASE_REF` and computing a merge-base. Graphite sets `GITHUB_BASE_REF` to a `graphite-base/` ref that is ephemeral (it is not reliably present on origin), so the fetch fails and the check errors out instead of running. ## Fix `actions/checkout` checks out the PR **merge commit** (`refs/pull//merge`), whose **first parent (`HEAD^1`) is the exact base commit GitHub merged against**. Diffing `HEAD^1` against the checkout yields every change the PR makes against its base branch, for normal and Graphite-stacked PRs alike. No base-branch fetch, no merge-base computation, no `gh`-based deepen dance. - `scripts/check_emdash.sh`: use `HEAD^1` (the PR base commit) as the diff base in CI. Drops `resolve_merge_base` and `fetch_base_ref`. Emits a clear error if `HEAD^1` is missing (checkout too shallow). - `.github/workflows/ci.yaml`: bump the `lint` job checkout to `fetch-depth: 2` so `HEAD^1` is present with no runtime fetch. Local dev behavior (merge-base against `origin/main`) is unchanged. ## Verification - `make lint/emdash`, `make lint/shellcheck`, `make lint/actions/actionlint` pass. - Simulated the CI path with `GITHUB_BASE_REF` set: the check resolves to `HEAD^1` without fetching and still flags an added line containing an emdash.
Why the merge commit's first parent For a `pull_request` checkout of `refs/pull//merge`: - `HEAD` = GitHub's synthetic PR merge commit - `HEAD^1` = the exact base commit used for the merge - `HEAD^2` = the PR head commit `git diff HEAD^1 HEAD` is the full-tree diff from the base snapshot to the merged result, i.e. all of the PR's changes against its base. This is immutable and always local (given depth >= 2), unlike base branch refs which are mutable and, for Graphite stacks, ephemeral.
--- This PR was generated by Coder Agents on behalf of @dannykopping. --- .github/workflows/ci.yaml | 5 +- .github/workflows/dogfood.yaml | 5 ++ scripts/check_emdash.sh | 93 ++++++---------------------------- 3 files changed, 24 insertions(+), 79 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 1f7de0f684..4a900bc2bf 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -318,7 +318,10 @@ jobs: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: - fetch-depth: 1 + # Depth 2 makes the PR merge commit's first parent (the base + # branch tip) available so lint/emdash can diff against HEAD^ + # without fetching the base branch at runtime. + fetch-depth: 2 persist-credentials: false - name: Set up mise tools diff --git a/.github/workflows/dogfood.yaml b/.github/workflows/dogfood.yaml index 9eef88cf9b..7b61d41e92 100644 --- a/.github/workflows/dogfood.yaml +++ b/.github/workflows/dogfood.yaml @@ -77,6 +77,11 @@ jobs: - name: Checkout uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 with: + # Depth 2 makes the PR merge commit's first parent (the base + # branch tip) available so lint/emdash, run inside the image via + # scripts/dogfood_test_image.sh, can diff against HEAD^1 without + # fetching the base branch at runtime. + fetch-depth: 2 persist-credentials: false - name: Get branch name diff --git a/scripts/check_emdash.sh b/scripts/check_emdash.sh index 71d1728637..4433a6d6b9 100755 --- a/scripts/check_emdash.sh +++ b/scripts/check_emdash.sh @@ -39,87 +39,24 @@ scan_all_files() { fi } -# resolve_merge_base finds the merge-base between HEAD and the given ref. -# In shallow CI clones the merge-base is not directly reachable, so we -# query the PR commit count via `gh`, deepen HEAD by count+1, and -# resolve HEAD~N which is the parent of the first PR commit. -resolve_merge_base() { - local base_ref="$1" - - # Fast path: merge-base already reachable (full clone or sufficient depth). - local mb - mb=$(git merge-base HEAD "$base_ref" 2>/dev/null || true) - if [[ -n "$mb" ]]; then - echo "$mb" - return - fi - - if ! command -v gh >/dev/null 2>&1; then - echo "gh CLI not found, cannot determine PR commit count." >&2 - return - fi - - # Use the PR commit count to deepen HEAD past the PR commits. - # HEAD~N is the parent of the oldest PR commit, i.e. the merge-base. - local count - count=$(gh pr view --json commits --jq '.commits | length' 2>/dev/null || true) - if [[ -z "$count" || "$count" -le 0 ]]; then - echo "Could not determine PR commit count from gh." >&2 - return - fi - - echo "Deepening HEAD by $((count + 1)) to reach PR base..." >&2 - git fetch --deepen="$((count + 1))" 2>/dev/null || true - - # Retry merge-base now that we have more history. - mb=$(git merge-base HEAD "$base_ref" 2>/dev/null || true) - if [[ -n "$mb" ]]; then - echo "$mb" - return - fi - - # Last resort: walk first-parent history. This is correct for - # linear PRs but may traverse the wrong branch for merge-commit - # checkouts. - git rev-parse --verify "HEAD~${count}" 2>/dev/null || true -} - -# fetch_base_ref ensures origin/$GITHUB_BASE_REF is available locally. -# CI shallow clones (fetch-depth: 1) typically omit the base branch. -fetch_base_ref() { - local base_ref="$1" - - if git rev-parse --verify "$base_ref" >/dev/null 2>&1; then - return 0 - fi - - local ref="${base_ref#origin/}" - echo "Base ref $base_ref not found locally, fetching $ref..." >&2 - git fetch origin "$ref" --depth=1 2>/dev/null || true - - if ! git rev-parse --verify "$base_ref" >/dev/null 2>&1; then - echo "ERROR: could not fetch base ref $base_ref." >&2 - return 1 - fi -} - -# resolve_diff_base determines the base ref to diff against. +# resolve_diff_base determines the base commit to diff against. resolve_diff_base() { - # CI pull requests: use merge-base against the target branch. + # CI pull requests: actions/checkout checks out the PR merge commit + # (refs/pull//merge). Its first parent (HEAD^1) is the exact base + # commit GitHub merged against, so diffing HEAD^1 against the checkout + # yields every change the PR makes against its base branch. We rely on + # this commit rather than fetching the base branch by name: branch + # names are mutable and Graphite stacks target an ephemeral + # graphite-base/ ref that may not exist on origin. Requires the + # checkout to use fetch-depth >= 2 so HEAD^1 is present. if [[ -n "${GITHUB_BASE_REF:-}" ]]; then - local base_ref="origin/${GITHUB_BASE_REF}" - fetch_base_ref "$base_ref" || return 1 - - local base - base=$(resolve_merge_base "$base_ref") - if [[ -n "$base" ]]; then - echo "$base" - return + if ! git rev-parse --verify --quiet "HEAD^1" >/dev/null; then + echo "ERROR: the PR base commit (HEAD^1) is missing. Check out" >&2 + echo " the PR with fetch-depth >= 2 so the merge commit's" >&2 + echo " base parent is available." >&2 + return 1 fi - - # Could not determine merge-base; fall back to branch tip. - echo "WARNING: could not find merge-base with $base_ref, using branch tip (diff may include non-PR changes)." >&2 - echo "$base_ref" + git rev-parse "HEAD^1" return fi