From 49240439df7b2570b50150a99e580c72ff163af8 Mon Sep 17 00:00:00 2001 From: Hanno Blankenstein Date: Mon, 15 Jun 2026 13:31:32 +1000 Subject: [PATCH] ci: make typecheck-ratchet base collection robust (fix false positive) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The base-baseline step ran `bun turbo typecheck` inside a detached `git worktree` with symlinked node_modules. turbo/tsgo produced no task output in that linked worktree (it has a `.git` *file*, not dir), so the base set came back empty and the ratchet flagged every real head error as "new" — failing Dockerfile-only / no-TS PRs (e.g. #48) with a misleading "introduces N new errors". The base step itself also died because collect-type-errors.sh's final grep exits 1 on zero matches. - Collect the base baseline by checking BASE out IN PLACE in the same working tree (reusing head's node_modules) instead of a linked worktree, so turbo behaves identically for base and head. Head's collector is copied to a temp path before checkout so the up-to-date collector runs even when BASE predates this fix. - collect-type-errors.sh: honor its documented "always exits 0" contract (the final grep exited 1 on no matches and, as the last command, failed the step under `bash -e`). Warn to stderr when the typecheck emitted no turbo task lines (i.e. it did not actually run). - Sanity gate: base=0 signatures while head>0 means base collection did not run — fail with a clear diagnostic instead of misreporting N "new" errors. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/scripts/collect-type-errors.sh | 26 +++++++++++++-- .github/workflows/typecheck.yml | 45 ++++++++++++++++---------- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/.github/scripts/collect-type-errors.sh b/.github/scripts/collect-type-errors.sh index c6e850a12f6b..8636508180ee 100755 --- a/.github/scripts/collect-type-errors.sh +++ b/.github/scripts/collect-type-errors.sh @@ -20,12 +20,34 @@ raw="$(mktemp)" trap 'rm -f "$raw"' EXIT # `bun run typecheck` → `bun turbo typecheck` → each package's `tsgo --noEmit`. -TURBO_FORCE=true bun run typecheck > "$raw" 2>&1 || true +TURBO_FORCE=true bun run typecheck > "$raw" 2>&1 +rc=$? + +# Diagnostics to stderr (shows in the step log, never touches the stdout the +# workflow parses). A non-zero rc is EXPECTED when there are type errors — +# tsgo exits non-zero on errors. But if rc is non-zero AND the output carries +# no turbo task lines at all, the typecheck never actually ran (tooling/setup +# failure) and the caller is about to mistake "0 errors" for "didn't run" — +# make that loud. +if ! grep -qE ':typecheck:|Tasks:[[:space:]]+[0-9]' "$raw"; then + echo "WARNING(collect-type-errors): typecheck output has no turbo task lines (rc=$rc) — the type checker likely did NOT run in this checkout." >&2 + echo "---- first 20 lines of raw typecheck output ----" >&2 + head -20 "$raw" >&2 + echo "------------------------------------------------" >&2 +fi # tsgo lines: path/to/file.ts(12,34): error TS2741: Property 'x' is missing ... # turbo prefixes them with "pkg:typecheck: "; the -oE match starts at the path, # dropping the prefix, so signatures compare cleanly across the two checkouts. # Strip the (line,col) so a signature is { file + TS code + message }. +# +# The trailing `|| true` keeps the script's "always exits 0" contract: grep +# exits 1 when it finds NO matching lines (a clean checkout, or a checkout where +# typecheck didn't run), and without the guard that non-zero — being the script's +# last command — would propagate out and fail the step under `bash -e`. grep -oE '[^ ]+\.(ts|tsx)\([0-9]+,[0-9]+\): error TS[0-9]+:.*' "$raw" \ | sed -E 's/\([0-9]+,[0-9]+\)//' \ - | sort -u + | sort -u \ + || true + +exit 0 diff --git a/.github/workflows/typecheck.yml b/.github/workflows/typecheck.yml index 8dba861b02cb..9673be7196b2 100644 --- a/.github/workflows/typecheck.yml +++ b/.github/workflows/typecheck.yml @@ -47,30 +47,41 @@ jobs: BASE_SHA: ${{ github.event.pull_request.base.sha }} run: | set -euo pipefail - # Fetch just the base commit and check it out in a separate worktree, - # reusing the already-installed node_modules (type defs) via symlink - # so we don't pay a second multi-minute `bun install`. Base + head - # share a lockfile in the overwhelming majority of PRs; if a PR - # changes dependencies, the base typecheck runs against head's - # node_modules — worst case a spurious new/missing signature the - # author can see in the log. + # Collect the base baseline by checking BASE out IN PLACE in this same + # working tree, then running the collection script here — NOT in a + # detached `git worktree` with symlinked node_modules. The worktree + # approach (the original design) made `bun turbo typecheck` fast-fail + # on the base: a linked worktree has a `.git` *file* (not dir) and a + # symlinked node_modules, and turbo/tsgo produced zero task output + # there, so the base baseline came back empty and the ratchet flagged + # every real head error as "new". Running in place reuses the exact + # node_modules + tree head just typechecked against, so turbo behaves + # identically for base and head. + # + # Copy head's (this PR's) collection script to a temp path BEFORE the + # checkout, so we always run the up-to-date collector even when BASE + # predates a fix to it. + cp .github/scripts/collect-type-errors.sh "$RUNNER_TEMP/collect.sh" git fetch --no-tags --depth=1 origin "$BASE_SHA" - git worktree add --detach "$RUNNER_TEMP/base" "$BASE_SHA" - ln -s "$GITHUB_WORKSPACE/node_modules" "$RUNNER_TEMP/base/node_modules" - for d in packages/*; do - if [ -d "$GITHUB_WORKSPACE/$d/node_modules" ]; then - ln -s "$GITHUB_WORKSPACE/$d/node_modules" "$RUNNER_TEMP/base/$d/node_modules" || true - fi - done - ( cd "$RUNNER_TEMP/base" && bash "$GITHUB_WORKSPACE/.github/scripts/collect-type-errors.sh" ) > "$RUNNER_TEMP/base.txt" + git checkout --detach --force "$BASE_SHA" + bash "$RUNNER_TEMP/collect.sh" > "$RUNNER_TEMP/base.txt" - name: Fail on NEW type errors if: github.event_name == 'pull_request' run: | set -uo pipefail - base_n=$(wc -l < "$RUNNER_TEMP/base.txt" | tr -d ' ') - head_n=$(wc -l < "$RUNNER_TEMP/head.txt" | tr -d ' ') + base_n=$(grep -c . "$RUNNER_TEMP/base.txt" || true) + head_n=$(grep -c . "$RUNNER_TEMP/head.txt" || true) echo "type-error signatures — base: ${base_n}, head: ${head_n}" + # Sanity gate: this fork carries ~20 known baseline TS errors, so the + # base set is never legitimately empty while head has errors. A base + # count of 0 with a non-empty head almost always means the base + # collection didn't actually run (tooling/setup failure) — fail with a + # clear diagnostic instead of misreporting every head error as "new". + if [ "$base_n" -eq 0 ] && [ "$head_n" -gt 0 ]; then + echo "::error::Base produced 0 type-error signatures while head has ${head_n}. The base typecheck almost certainly failed to run (not a real diff) — inspect the 'Collect type errors — base' step log." + exit 1 + fi new="$(comm -13 <(sort -u "$RUNNER_TEMP/base.txt") <(sort -u "$RUNNER_TEMP/head.txt") || true)" if [ -n "$new" ]; then echo "::error::This PR introduces NEW TypeScript errors not present on the base branch:"