ci: weigh a package source change when an owned Mac picks its starting build

.github/workflows/ci-macos.yml

@@ -501,11 +501,14 @@ jobs:
       # A warm Mac's kept DerivedData is the previous pull request's build, so
       # its compile also undoes that diff (365-428 s on 2026-09-25), while a
       # seed a few commits behind compiled in 50-160 s. With
-      # CI_OWNED_PREFER_SEED set, adopt the seed instead when it rebuilds
-      # fewer inputs: `local` compares only with a seed this Mac keeps (no
-      # download), a number N also takes a downloaded seed at most N commits
-      # behind. owned_build_state.py `prefer` has the rules; unset, nothing
-      # changes. If the seed then misses, the kept DerivedData is adopted.
+      # CI_OWNED_PREFER_SEED set, adopt the seed instead when it compiles
+      # less: first whether a local package source changed (that recompiles
+      # the whole app), then how many inputs changed. `local` compares only
+      # with a seed this Mac keeps (no download); a number N also takes a
+      # downloaded seed at most N commits behind, or at any distance when it
+      # alone spares the app recompile. owned_build_state.py `prefer` has the
+      # rules; unset, nothing changes. If the seed then misses, the kept
+      # DerivedData is adopted.
       - name: Prefer a near seed over this owned Mac's DerivedData
         id: prefer-seed
         if: steps.reuse-products.outputs.hit != 'true' && steps.owned-state.outputs.warm == 'true' && vars.CI_OWNED_PREFER_SEED != '' && (vars.CI_ADMISSION_SEED_DERIVED_DATA || '1') != '0' && (inputs.cache_backend != 'default' && inputs.cache_backend || vars.CI_CACHE_BACKEND || 'r2') == 'r2'

scripts/ci/owned_build_state.py

@@ -77,11 +77,23 @@
 MANIFEST of the nearest seed in this commit's history that this Mac keeps
 (seed_derived_data.py CMUX_SEED_LOCAL_CACHE). Both are a local clone, so the
 one with fewer changed inputs wins, and the adopt that follows clones exactly
-that seed (CMUX_SEED_EXACT). A seed this Mac does not keep costs a download of about 190 s,
+that seed (CMUX_SEED_EXACT). A seed this Mac does not keep costs a download of about 250 s,
 so it wins only within MAX_DISTANCE commits, and only when MAX_DISTANCE is
 given. A kept DerivedData without a record replays nothing and rebuilds the
 whole `cmux` module, so any seed beats it. Every error keeps the warm path.
 
+The count alone misleads when a local package's source changed (Packages/,
+vendor/, Examples/): every `cmux` file imports those modules and recompiles,
+whether 19 or 1,000 inputs changed. On 2026-09-25 three warm jobs cloned a
+kept seed 9 to 20 commits behind because it had fewer changed inputs than the
+kept DerivedData, and compiled for 533 to 958 s. In two of them the seed sat
+behind a package change that a nearer bucket seed had already built (jobs
+108004619872, 107981633810). So a start that recompiles the app loses
+to one that does not, and when both the kept DerivedData and the kept seed
+would, a nearer bucket seed wins at any distance if GitHub's compare of its
+commit with the checkout shows no package source change: its download (about
+250 s on a mini) costs less than recompiling the app (365 to 1,053 s).
+
 Clones are APFS clones: the canonical root (/private/tmp/cmux-ci) and STORE
 sit on the same volume, so nothing is copied. Kept state is replaced by
 renaming the new copy into place after the old one is out of the way, so an
@@ -319,17 +331,80 @@ def save(store: Path, source_packages: Path, workspace: Path, package_store: Pat
 
 # Not build inputs of the compile, and not present at every recording.
 UNCOMPARED = (".ci-source-packages/", "GhosttyKit.xcframework/")
+# Local Swift packages the app target imports. A changed source in any of them
+# changes a module the `cmux` target imports, so every `cmux` file recompiles
+# (2,300 to 2,700 SwiftCompile tasks, 365 to 1,053 s on an owned mini on
+# 2026-09-25), however few inputs changed: 115 changed inputs cost 958 s in
+# job 108004619872, 19 changed app inputs 222 s in job 107986723124.
+PACKAGE_SOURCES = ("Packages/", "vendor/", "Examples/")
+# GitHub's compare API lists at most this many files; a longer diff is unknown.
+COMPARE_FILE_LIMIT = 300
 
 
-def changed_inputs(current: dict[str, list], recorded: dict[str, list]) -> int:
+def changed_paths(current: dict[str, list], recorded: dict[str, list]) -> set[str]:
     """Files whose content differs between two records, or that only one has."""
     def files(entries: dict[str, list]) -> dict[str, str]:
         return {
             path: entry[0] for path, entry in entries.items()
             if not path.endswith("/") and not path.startswith(UNCOMPARED) and isinstance(entry, list) and entry
         }
     now, then = files(current), files(recorded)
-    return sum(1 for path in now.keys() | then.keys() if now.get(path) != then.get(path))
+    return {path for path in now.keys() | then.keys() if now.get(path) != then.get(path)}
+
+
+def changed_inputs(current: dict[str, list], recorded: dict[str, list]) -> int:
+    return len(changed_paths(current, recorded))
+
+
+def rebuilds_app(paths) -> bool:
+    """Whether these changed files recompile the whole `cmux` module."""
+    return any(path.startswith(PACKAGE_SOURCES) and path.endswith(".swift") for path in paths)
+
+
+def cost(paths: set[str]) -> tuple[bool, int]:
+    """What a start with these changed inputs compiles: an app rebuild first, then the count."""
+    return rebuilds_app(paths), len(paths)
+
+
+def bucket_seed_rebuilds_app(key: str, workspace: Path) -> bool | None:
+    """Whether the commits from KEY's revision to the checkout change a package source.
+
+    None when GitHub cannot say (no repository, an error, or a diff past the
+    compare API's file limit); the caller then treats the seed as no better.
+    """
+    repository = os.environ.get("GITHUB_REPOSITORY", "")
+    if not repository:
+        return None
+    try:
+        head = subprocess.run(["git", "-C", str(workspace), "rev-parse", "HEAD"],
+                              check=True, capture_output=True, text=True, timeout=30).stdout.strip()
+        files = json.loads(subprocess.run(
+            ["gh", "api", f"repos/{repository}/compare/{key.rsplit('-', 1)[-1]}...{head}",
+             "--jq", "[.files[]? | .filename, (.previous_filename // empty)]"],
+            check=True, capture_output=True, text=True, timeout=60,
+        ).stdout)
+    except (OSError, subprocess.SubprocessError, ValueError):
+        return None
+    if len(files) >= COMPARE_FILE_LIMIT:
+        return None
+    # A submodule bump (vendor/bonsplit) is listed as the bare submodule
+    # path, while the local records see the .swift files under it.
+    if rebuilds_app(files):
+        return True
+    bumped = [path for path in files if path.startswith(PACKAGE_SOURCES)]
+    return bool(bumped) and not submodules(workspace).isdisjoint(bumped)
+
+
+def submodules(workspace: Path) -> set[str]:
+    """The submodule paths .gitmodules declares, or none if it cannot be read."""
+    try:
+        listed = subprocess.run(
+            ["git", "config", "-f", str(workspace / ".gitmodules"), "--get-regexp", r"\.path$"],
+            check=True, capture_output=True, text=True, timeout=30,
+        ).stdout
+    except (OSError, subprocess.SubprocessError):
+        return set()
+    return {line.split(None, 1)[1].strip() for line in listed.splitlines() if len(line.split(None, 1)) == 2}
 
 
 def nearest_kept_seed(prefix: str, revision: str) -> tuple[str, int] | None:
@@ -357,35 +432,49 @@ def prefer(store: Path, workspace: Path, prefix: str, revision: str, max_distanc
     manifest = store / DERIVED / RECORD
     kept_record = json.loads(manifest.read_text()) if manifest.is_file() else None
     current = seed.warm.record(workspace) if kept_record is not None else {}
-    kept_changed = changed_inputs(current, kept_record) if kept_record is not None else None
-    if kept_changed is not None:
-        result["kept_changed"] = str(kept_changed)
+    kept_cost = cost(changed_paths(current, kept_record)) if kept_record is not None else None
+    if kept_cost is not None:
+        result.update(kept_changed=str(kept_cost[1]), kept_rebuilds_app=str(kept_cost[0]).lower())
     kept_seed = nearest_kept_seed(prefix, revision)
+    local_rebuilds_app = False
     if kept_seed:
         key, distance = kept_seed
         result.update(seed_key=key, seed_distance=str(distance), local="true")
-        if kept_changed is None:
+        if kept_cost is None:
             result.update(prefer="true", reason="kept DerivedData has no input record")
             return result
-        seed_changed = changed_inputs(current, json.loads((seed.cached(key) / seed.MANIFEST).read_text()))
-        result["seed_changed"] = str(seed_changed)
-        if seed_changed < kept_changed:
+        seed_cost = cost(changed_paths(current, json.loads((seed.cached(key) / seed.MANIFEST).read_text())))
+        result.update(seed_changed=str(seed_cost[1]), seed_rebuilds_app=str(seed_cost[0]).lower())
+        if seed_cost < kept_cost:
             result.update(prefer="true", reason="this Mac keeps a seed with fewer changed inputs")
-            return result
-        result["reason"] = "the kept DerivedData has no more changed inputs than the seed this Mac keeps"
+            if not seed_cost[0] or max_distance is None:
+                return result
+            # It still recompiles the whole app. A newer seed in the bucket
+            # may not; checked below.
+            local_rebuilds_app = True
+        else:
+            result["reason"] = "the kept DerivedData has no more changed inputs than the seed this Mac keeps"
     if max_distance is None:
         result.setdefault("reason", "this Mac keeps no seed in this commit's history")
         return result
     exact, distance = seed.locate(prefix, revision)
     if distance is None:
         result.setdefault("reason", "no seed in this commit's history")
         return result
-    if kept_changed == 0:
+    downloaded = {"prefer": "true", "seed_key": exact, "seed_distance": str(distance), "local": "false"}
+    if local_rebuilds_app:
+        if distance < int(result["seed_distance"]) and bucket_seed_rebuilds_app(exact, workspace) is False:
+            result.update(downloaded, reason=f"the seed this Mac keeps recompiles the app; the seed {distance} commits behind does not")
+        return result
+    if kept_cost is not None and kept_cost[1] == 0:
         result["reason"] = "the kept DerivedData has no changed inputs"
     elif distance <= max_distance:
-        result.update(prefer="true", seed_key=exact, seed_distance=str(distance), local="false",
-                      reason=f"seed {distance} commits behind, within {max_distance}"
-                      + ("" if kept_changed is not None else "; kept DerivedData has no input record"))
+        result.update(downloaded, reason=f"seed {distance} commits behind, within {max_distance}"
+                      + ("" if kept_cost is not None else "; kept DerivedData has no input record"))
+    elif kept_cost is not None and kept_cost[0] and bucket_seed_rebuilds_app(exact, workspace) is False:
+        # A download (about 250 s on a mini) costs less than recompiling the
+        # whole app (365 to 1,053 s on an owned mini on 2026-09-25).
+        result.update(downloaded, reason=f"the kept DerivedData recompiles the app; the seed {distance} commits behind does not")
     else:
         result.setdefault("reason", f"the nearest seed is {distance} commits behind, past {max_distance}")
     return result

tests/test_ci_owned_build_state.py

@@ -382,6 +382,109 @@ def test_the_nearest_kept_seed_counts_not_only_the_newest_in_the_bucket(self):
         with unittest.mock.patch.dict(os.environ, {"CMUX_SEED_EXACT": "p-j14-gone"}):
             self.assertIsNone(state.seed.chosen())
 
+    def recorded_with_package_change(self, changed):
+        record = self.recorded(changed)
+        record["Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/F.swift"] = ["stale", 1]
+        return record
+
+    def test_rebuilds_app_only_for_a_package_swift_source(self):
+        self.assertTrue(state.rebuilds_app({"Packages/macOS/CmuxCloud/Sources/CmuxCloud/A.swift"}))
+        self.assertTrue(state.rebuilds_app({"vendor/bonsplit/Package.swift"}))
+        self.assertFalse(state.rebuilds_app({"Sources/AppDelegate.swift", "cmuxTests/ATests.swift",
+                                             "Packages/macOS/CmuxCloud/README.md"}))
+
+    def test_a_seed_without_a_package_change_beats_more_changed_inputs_with_one(self):
+        """115 changed inputs across a package change cost 958 s (job 108004619872)."""
+        self.kept(changed=5)
+        (self.cache / "p-j14-base").mkdir(parents=True)
+        (self.cache / "p-j14-base" / state.seed.MANIFEST).write_text(
+            json.dumps(self.recorded_with_package_change(changed=1)))
+        result = self.prefer()
+        self.assertEqual((result["prefer"], result["seed_rebuilds_app"], result["kept_rebuilds_app"]),
+                         ("false", "true", "false"))
+
+    def test_a_nearer_bucket_seed_replaces_a_kept_seed_that_recompiles_the_app(self):
+        self.kept(changed=6)
+        (self.store / "derived-data" / state.RECORD).write_text(
+            json.dumps(self.recorded_with_package_change(changed=6)))
+        (self.cache / "p-j14-oldest").mkdir(parents=True)
+        (self.cache / "p-j14-oldest" / state.seed.MANIFEST).write_text(
+            json.dumps(self.recorded_with_package_change(changed=1)))
+        with unittest.mock.patch.object(state, "bucket_seed_rebuilds_app", return_value=False) as compare:
+            result = self.prefer(("p-j14-base", 0), max_distance=2)
+        compare.assert_called_once_with("p-j14-base", self.workspace)
+        self.assertEqual((result["prefer"], result["seed_key"], result["local"]), ("true", "p-j14-base", "false"))
+        # When the bucket seed recompiles the app too, or GitHub cannot say,
+        # the clone stays: it is the cheaper start.
+        for answer in (True, None):
+            with unittest.mock.patch.object(state, "bucket_seed_rebuilds_app", return_value=answer):
+                result = self.prefer(("p-j14-base", 0), max_distance=2)
+            self.assertEqual((result["prefer"], result["seed_key"], result["local"]), ("true", "p-j14-oldest", "true"))
+        # Without downloads, the kept seed is all there is.
+        with unittest.mock.patch.object(state, "bucket_seed_rebuilds_app") as compare:
+            result = self.prefer(("p-j14-base", 0))
+        compare.assert_not_called()
+        self.assertEqual((result["prefer"], result["seed_key"]), ("true", "p-j14-oldest"))
+
+    def test_a_far_bucket_seed_replaces_a_kept_build_that_recompiles_the_app(self):
+        (self.store / "derived-data").mkdir(parents=True)
+        (self.store / "derived-data" / state.RECORD).write_text(
+            json.dumps(self.recorded_with_package_change(changed=3)))
+        with unittest.mock.patch.object(state, "bucket_seed_rebuilds_app", return_value=False):
+            result = self.prefer(("p-j14-base", 6), max_distance=2)
+        self.assertEqual((result["prefer"], result["seed_key"], result["seed_distance"], result["local"]),
+                         ("true", "p-j14-base", "6", "false"))
+        for answer in (True, None):
+            with unittest.mock.patch.object(state, "bucket_seed_rebuilds_app", return_value=answer):
+                self.assertEqual(self.prefer(("p-j14-base", 6), max_distance=2)["prefer"], "false")
+
+    def test_a_far_bucket_seed_never_replaces_a_kept_build_without_a_package_change(self):
+        self.kept(changed=3)
+        with unittest.mock.patch.object(state, "bucket_seed_rebuilds_app") as compare:
+            self.assertEqual(self.prefer(("p-j14-base", 6), max_distance=2)["prefer"], "false")
+        compare.assert_not_called()
+
+    def test_the_bucket_compare_reads_github_and_gives_up_past_its_file_limit(self):
+        def run(files):
+            def fake(argv, **_):
+                out = "abc123\n" if argv[0] == "git" else json.dumps(files)
+                return unittest.mock.Mock(stdout=out)
+            return fake
+        with unittest.mock.patch.dict(os.environ, {"GITHUB_REPOSITORY": "o/r"}):
+            with unittest.mock.patch.object(state.subprocess, "run", side_effect=run(["Sources/A.swift"])) as ran:
+                self.assertIs(state.bucket_seed_rebuilds_app("p-j14-seedsha", self.workspace), False)
+            self.assertEqual(ran.call_args_list[1].args[0][:3], ["gh", "api", "repos/o/r/compare/seedsha...abc123"])
+            with unittest.mock.patch.object(state.subprocess, "run", side_effect=run(["Packages/X/Sources/X/A.swift"])):
+                self.assertIs(state.bucket_seed_rebuilds_app("p-j14-seedsha", self.workspace), True)
+            with unittest.mock.patch.object(state.subprocess, "run", side_effect=run(["Sources/A.swift"] * 300)):
+                self.assertIsNone(state.bucket_seed_rebuilds_app("p-j14-seedsha", self.workspace))
+            with unittest.mock.patch.object(state.subprocess, "run", side_effect=OSError("no gh")):
+                self.assertIsNone(state.bucket_seed_rebuilds_app("p-j14-seedsha", self.workspace))
+        with unittest.mock.patch.dict(os.environ, {"GITHUB_REPOSITORY": ""}):
+            self.assertIsNone(state.bucket_seed_rebuilds_app("p-j14-seedsha", self.workspace))
+
+    def test_a_submodule_bump_under_a_package_root_rebuilds_the_app(self):
+        """Compare lists a submodule bump as the bare path (bonsplit, 4 bumps this month)."""
+        (self.workspace / ".gitmodules").write_text(
+            '[submodule "vendor/bonsplit"]\n\tpath = vendor/bonsplit\n\turl = x\n'
+            '[submodule "ghostty"]\n\tpath = ghostty\n\turl = y\n')
+        self.assertEqual(state.submodules(self.workspace), {"vendor/bonsplit", "ghostty"})
+        real = state.subprocess.run
+
+        def fake(argv, **kwargs):
+            if argv[:2] == ["git", "-C"]:
+                return unittest.mock.Mock(stdout="abc123\n")
+            if argv[0] == "gh":
+                return unittest.mock.Mock(stdout=json.dumps(self.compared))
+            return real(argv, **kwargs)
+        with unittest.mock.patch.dict(os.environ, {"GITHUB_REPOSITORY": "o/r"}), \
+             unittest.mock.patch.object(state.subprocess, "run", side_effect=fake):
+            self.compared = ["vendor/bonsplit"]
+            self.assertIs(state.bucket_seed_rebuilds_app("p-j14-seedsha", self.workspace), True)
+            # ghostty ships as a prebuilt xcframework, not a package root.
+            self.compared = ["ghostty", "Sources/A.swift"]
+            self.assertIs(state.bucket_seed_rebuilds_app("p-j14-seedsha", self.workspace), False)
+
     def test_any_error_keeps_the_warm_path(self):
         output = Path(self.tmp.name) / "output"
         with unittest.mock.patch.dict(os.environ, {"GITHUB_OUTPUT": str(output)}), \