From 31569c8cc8affda01f5d6f6e7d75d0f660d8ecca Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Thu, 23 Apr 2026 12:40:16 -0400
Subject: [PATCH] ci: explicit rebase check + fail-fast SDK typecheck in
 install-smoke (#2631)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* ci: explicit rebase check + fail-fast SDK typecheck in install-smoke

Stale-base regression guard. Root cause: GitHub's `refs/pull/N/merge`
is cached against the PR's recorded merge-base, not current main. When
main advances after a PR is opened, the cache stays stale and CI runs
against the pre-advance tree. PRs hit this whenever a type error lands
on main and gets patched shortly after (e.g. #2611 + #2622) — stale
branches replay the broken intermediate state and report confusing
downstream failures for hours.

Observed failure mode: install-smoke's "Assert gsd-sdk resolves on PATH"
step fires with "installSdkIfNeeded() regression" even when the real
cause is `npm run build` failing in sdk/ due to a TypeScript cast
mismatch already fixed on main.

Fix:
- Explicit `git merge origin/main` step in both `install-smoke.yml` and
  `test.yml`. If the merge conflicts, emit a clear "rebase onto main"
  diagnostic and fail early, rather than let conflicts produce unrelated
  downstream errors.
- Dedicated `npm run build:sdk` typecheck step in install-smoke with a
  remediation hint ("rebase onto main — the error may already be fixed
  on trunk"). Fails fast with the actual tsc output instead of masking
  it behind a PATH assertion.
- Drop the `|| true` on `get-shit-done-cc --claude --local` so installer
  failures surface at the install step with install.js's own error
  message, not at the downstream PATH assertion where the message
  misleadingly blames "shim regression".
- `fetch-depth: 0` on checkout so the merge-base check has history.

* ci: address CodeRabbit — add rebase check to smoke-unpacked, fix fetch flag

Two findings from CodeRabbit's review on #2631:

1. `smoke-unpacked` job was missing the same rebase check applied to the
   `smoke` job. It ran on the cached `refs/pull/N/merge` and could hit
   the same stale-base failure mode the PR was designed to prevent. Added
   the identical rebase-check step.

2. `git fetch origin main --depth=0` is an invalid flag — git rejects it
   with "depth 0 is not a positive number". The intent was "fetch with
   full depth", but the right way is just `git fetch origin main` (no
   --depth). Removed the invalid flag and the `||` fallback that was
   papering over the error.
---
 .github/workflows/install-smoke.yml | 73 +++++++++++++++++++++++++++--
 .github/workflows/test.yml          | 25 ++++++++++
 2 files changed, 93 insertions(+), 5 deletions(-)

diff --git a/.github/workflows/install-smoke.yml b/.github/workflows/install-smoke.yml
index 4e8c3033..d90813a6 100644
--- a/.github/workflows/install-smoke.yml
+++ b/.github/workflows/install-smoke.yml
@@ -85,6 +85,31 @@ jobs:
         if: steps.skip.outputs.skip != 'true'
         with:
           ref: ${{ inputs.ref || github.ref }}
+          # Need enough history to merge origin/main for stale-base detection.
+          fetch-depth: 0
+
+      # The default `refs/pull/N/merge` ref GitHub produces for PRs is cached
+      # against the recorded merge-base, not current main. When main advances
+      # after the PR was opened, the merge ref stays stale and CI can fail on
+      # issues that were already fixed upstream. Explicitly merge current
+      # origin/main into the PR head so smoke always tests the PR against the
+      # latest trunk. If the merge conflicts, emit a clear "rebase onto main"
+      # diagnostic instead of a downstream build error that looks unrelated.
+      - name: Rebase check — merge origin/main into PR head
+        if: steps.skip.outputs.skip != 'true' && github.event_name == 'pull_request'
+        shell: bash
+        run: |
+          set -euo pipefail
+          git config user.email "ci@gsd-build"
+          git config user.name "CI Rebase Check"
+          git fetch origin main
+          if ! git merge --no-edit --no-ff origin/main; then
+            echo "::error::This PR cannot cleanly merge origin/main. Rebase your branch onto current main and push again."
+            echo "::error::Conflicting files:"
+            git diff --name-only --diff-filter=U
+            git merge --abort
+            exit 1
+          fi
 
       - name: Set up Node.js ${{ matrix.node-version }}
         if: steps.skip.outputs.skip != 'true'
@@ -97,9 +122,22 @@ jobs:
         if: steps.skip.outputs.skip != 'true'
         run: npm ci
 
-      - name: Build SDK dist (required in tarball — sdk/dist is gitignored)
+      # Isolated SDK typecheck — if the build fails, emit a clear "stale base
+      # or real type error" diagnostic instead of letting the failure cascade
+      # into the tarball install step, where the downstream PATH assertion
+      # misreports it as "gsd-sdk not on PATH — installSdkIfNeeded regression".
+      - name: SDK typecheck (fails fast on type regressions)
         if: steps.skip.outputs.skip != 'true'
-        run: npm run build:sdk
+        shell: bash
+        run: |
+          set -euo pipefail
+          if ! npm run build:sdk; then
+            echo "::error::SDK build (npm run build:sdk) failed."
+            echo "::error::Common cause: your PR base is behind main and picks up intermediate type errors that are already fixed on trunk."
+            echo "::error::Fix: git fetch origin main && git rebase origin/main && git push --force-with-lease"
+            echo "::error::If the error persists on a fresh rebase, the type error is real — fix it in sdk/src/ and push."
+            exit 1
+          fi
 
       - name: Pack root tarball
         if: steps.skip.outputs.skip != 'true'
@@ -132,9 +170,14 @@ jobs:
           cd "$TMPDIR_ROOT"
           npm install -g "$WORKSPACE/$TARBALL"
           command -v get-shit-done-cc
-          # `--claude --local` is the non-interactive code path.
-          # Tolerate non-zero: the authoritative assertion is the next step.
-          get-shit-done-cc --claude --local || true
+          # `--claude --local` is the non-interactive code path. Don't swallow
+          # non-zero exit — if the installer fails, that IS the CI failure, and
+          # its own error message is more useful than the downstream "shim
+          # regression" assertion masking the real cause.
+          if ! get-shit-done-cc --claude --local; then
+            echo "::error::get-shit-done-cc --claude --local failed. See the install.js output above for the real error (SDK build, PATH resolution, chmod, etc.)."
+            exit 1
+          fi
 
       - name: Assert gsd-sdk resolves on PATH
         if: steps.skip.outputs.skip != 'true'
@@ -173,6 +216,26 @@ jobs:
       - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2
         with:
           ref: ${{ inputs.ref || github.ref }}
+          fetch-depth: 0
+
+      # See the `smoke` job above for rationale — refs/pull/N/merge is cached
+      # against the recorded merge-base, not current main. Explicitly merge
+      # origin/main so smoke-unpacked also runs against the latest trunk.
+      - name: Rebase check — merge origin/main into PR head
+        if: github.event_name == 'pull_request'
+        shell: bash
+        run: |
+          set -euo pipefail
+          git config user.email "ci@gsd-build"
+          git config user.name "CI Rebase Check"
+          git fetch origin main
+          if ! git merge --no-edit --no-ff origin/main; then
+            echo "::error::This PR cannot cleanly merge origin/main. Rebase your branch onto current main and push again."
+            echo "::error::Conflicting files:"
+            git diff --name-only --diff-filter=U
+            git merge --abort
+            exit 1
+          fi
 
       - name: Set up Node.js 22
         uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f  # v6.3.0
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 6068112c..6098fad6 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -35,6 +35,31 @@ jobs:
 
     steps:
       - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2
+        with:
+          # Fetch full history so we can merge origin/main for stale-base detection.
+          fetch-depth: 0
+
+      # GitHub's `refs/pull/N/merge` is cached against the recorded merge-base.
+      # When main advances after a PR is opened, the cache stays stale and CI
+      # runs against the pre-advance state — hiding bugs that are already fixed
+      # on trunk and surfacing type errors that were introduced and then patched
+      # on main in between. Explicitly merge current origin/main here so tests
+      # always run against the latest trunk.
+      - name: Rebase check — merge origin/main into PR head
+        if: github.event_name == 'pull_request'
+        shell: bash
+        run: |
+          set -euo pipefail
+          git config user.email "ci@gsd-build"
+          git config user.name "CI Rebase Check"
+          git fetch origin main
+          if ! git merge --no-edit --no-ff origin/main; then
+            echo "::error::This PR cannot cleanly merge origin/main. Rebase your branch onto current main and push again."
+            echo "::error::Conflicting files:"
+            git diff --name-only --diff-filter=U
+            git merge --abort
+            exit 1
+          fi
 
       - name: Set up Node.js ${{ matrix.node-version }}
         uses: actions/setup-node@53b83947a5a98c8d113130e565377fae1a50d02f  # v6.3.0