fix(ci): classify pull request diffs against the merge parent (#165)

A stale event base SHA was unioning commits already on main into the
isolation diff, so a clean PR failed as a mixed app and Terraform change.

.github/actions/app-terraform-isolation/action.yml

@@ -45,8 +45,15 @@ runs:
               echo "FAIL: pull_request base SHA is empty" >&2
               exit 1
             fi
-            merge_base="$(git merge-base "${PR_BASE_SHA}" HEAD)"
-            git diff --name-only --diff-filter=ACMRD "${merge_base}" HEAD | classify
+            # pull_request checkout is the merge commit. Diff that commit
+            # against its first parent so commits already on the base are
+            # not classified with this PR. Fall back when HEAD is not a merge.
+            if git rev-parse --verify --quiet HEAD^2 >/dev/null; then
+              git diff --name-only --diff-filter=ACMRD HEAD^1 HEAD | classify
+            else
+              merge_base="$(git merge-base "${PR_BASE_SHA}" HEAD)"
+              git diff --name-only --diff-filter=ACMRD "${merge_base}" HEAD | classify
+            fi
             ;;
           merge_group)
             if [[ -z "${MERGE_GROUP_BASE_SHA}" ]]; then

.github/workflows/ci-terraform.yaml

@@ -17,9 +17,10 @@ name: CI — Terraform
 #
 # app-paths is optional. Empty skips isolation and leaves fmt/init/validate
 # unchanged. A trailing slash is a directory prefix. Any other line is an
-# exact file. pull_request classifies the merge-base of the base SHA to HEAD.
-# merge_group classifies each first-parent commit against its parent, so a
-# Terraform-only PR stacked with an app-only PR still passes. The classified
+# exact file. pull_request classifies the merge commit against its first
+# parent, not an older event base SHA. merge_group classifies each
+# first-parent commit against its parent, so a Terraform-only PR stacked
+# with an app-only PR still passes. The classified
 # diff includes deletions. Terraform paths are those under working-directory.
 # The checker is a composite action in this repository. `$/` resolves that
 # action at this workflow's commit, so callers do not clone this private

scripts/test_check_app_terraform_isolation.py

@@ -134,7 +134,43 @@ class IsolationTests(unittest.TestCase):
 
 class WorkflowDiffTests(unittest.TestCase):
     def test_classified_diffs_include_deletions(self) -> None:
-        self.assertEqual(_workflow_diff_filters(), ["ACMRD", "ACMRD"])
+        self.assertEqual(_workflow_diff_filters(), ["ACMRD", "ACMRD", "ACMRD"])
+
+    def test_pull_request_diffs_merge_commit_against_first_parent(self) -> None:
+        action = (ACTION / "action.yml").read_text()
+        self.assertIn("git rev-parse --verify --quiet HEAD^2", action)
+        self.assertIn(
+            "git diff --name-only --diff-filter=ACMRD HEAD^1 HEAD | classify",
+            action,
+        )
+        self.assertIn('git merge-base "${PR_BASE_SHA}" HEAD', action)
+
+    def test_merge_commit_excludes_commits_already_on_the_base(self) -> None:
+        with tempfile.TemporaryDirectory() as tmp:
+            repo = Path(tmp)
+            _commit_base(repo)
+            old_base = _git(repo, "rev-parse", "HEAD").strip()
+            (repo / "terraform" / "versions.tf").write_text("aws\n")
+            _git(repo, "add", "-A")
+            _git(repo, "commit", "-m", "terraform provider")
+            (repo / "package.json").write_text("{}\n")
+            _git(repo, "add", "-A")
+            _git(repo, "commit", "-m", "npm bump")
+            _git(repo, "checkout", "-b", "pr", old_base)
+            workflows = repo / ".github" / "workflows"
+            workflows.mkdir(parents=True)
+            (workflows / "ci.yaml").write_text("name: CI\n")
+            _git(repo, "add", "-A")
+            _git(repo, "commit", "-m", "pin bump")
+            _git(repo, "checkout", "main")
+            _git(repo, "merge", "--no-ff", "pr", "-m", "Merge pr into main")
+            pr_only = _changed_paths(repo, "HEAD^1", "HEAD", "ACMRD")
+            stale = _changed_paths(repo, old_base, "HEAD", "ACMRD")
+        self.assertEqual(pr_only, [".github/workflows/ci.yaml"])
+        self.assertIn("package.json", stale)
+        self.assertIn("terraform/versions.tf", stale)
+        self.assertIsNone(isolation_violation(pr_only, APP_PATHS))
+        self.assertIsNotNone(isolation_violation(stale, APP_PATHS))
 
     def test_workflow_passes_working_directory(self) -> None:
         self.assertIn(