From f242e4c0538f437df175633b754dc2f12aea0303 Mon Sep 17 00:00:00 2001 From: teamleaderleo Date: Fri, 25 Sep 2026 05:26:28 -0400 Subject: [PATCH 1/3] ci: weigh a package source change when an owned Mac picks its starting build A changed source in a local package (Packages/, vendor/, Examples/) recompiles every file of the cmux module, however few inputs changed. prefer compared starts by changed-input count only, so a warm Mac cloned a kept seed 9 to 20 commits behind (fewer changed inputs than its kept build) and compiled for 533 to 958 s behind a package change that a nearer bucket seed had already built (jobs 108004619872, 107981633810). prefer now ranks a start that recompiles the app below one that does not. When the kept build and the kept seed both would, it asks GitHub's compare API whether the nearest bucket seed's commits up to the checkout change a package source, and downloads that seed at any distance if not. Unknown answers keep today's choice. Co-Authored-By: Claude Opus 5.5 --- scripts/ci/owned_build_state.py | 102 ++++++++++++++++++++++++----- tests/test_ci_owned_build_state.py | 81 +++++++++++++++++++++++ 2 files changed, 168 insertions(+), 15 deletions(-) diff --git a/scripts/ci/owned_build_state.py b/scripts/ci/owned_build_state.py index 4a4156e439f0..157088727ddf 100644 --- a/scripts/ci/owned_build_state.py +++ b/scripts/ci/owned_build_state.py @@ -82,6 +82,18 @@ 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 958 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,9 +331,17 @@ 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 { @@ -329,7 +349,45 @@ def files(entries: dict[str, list]) -> dict[str, str]: 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]"], + 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 + return rebuilds_app(files) def nearest_kept_seed(prefix: str, revision: str) -> tuple[str, int] | None: @@ -357,22 +415,28 @@ 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 @@ -380,12 +444,20 @@ def prefer(store: Path, workspace: Path, prefix: str, revision: str, max_distanc 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 from the kept DerivedData (365 to 482 s 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 diff --git a/tests/test_ci_owned_build_state.py b/tests/test_ci_owned_build_state.py index 51660085e2c4..2fc36e9a0894 100644 --- a/tests/test_ci_owned_build_state.py +++ b/tests/test_ci_owned_build_state.py @@ -382,6 +382,87 @@ 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_any_error_keeps_the_warm_path(self): output = Path(self.tmp.name) / "output" with unittest.mock.patch.dict(os.environ, {"GITHUB_OUTPUT": str(output)}), \ From 619309a78c8e5eace87f509fa4181dc03b9d1dc8 Mon Sep 17 00:00:00 2001 From: teamleaderleo Date: Fri, 25 Sep 2026 05:30:38 -0400 Subject: [PATCH 2/3] ci: count a bumped package submodule as an app rebuild in prefer GitHub's compare lists a submodule bump (vendor/bonsplit) as the bare submodule path, so the bucket-seed check missed it while the local records saw the .swift files under it. Read the submodule paths from .gitmodules, include renamed files' previous names, and bring the workflow comment and the timing comment in line with the new ranking. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci-macos.yml | 13 ++++++++----- scripts/ci/owned_build_state.py | 22 +++++++++++++++++++--- tests/test_ci_owned_build_state.py | 22 ++++++++++++++++++++++ 3 files changed, 49 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci-macos.yml b/.github/workflows/ci-macos.yml index 2d0d8eefad82..3e03280985a2 100644 --- a/.github/workflows/ci-macos.yml +++ b/.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' diff --git a/scripts/ci/owned_build_state.py b/scripts/ci/owned_build_state.py index 157088727ddf..6558a2f1f0bb 100644 --- a/scripts/ci/owned_build_state.py +++ b/scripts/ci/owned_build_state.py @@ -380,14 +380,30 @@ def bucket_seed_rebuilds_app(key: str, workspace: Path) -> bool | None: 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]"], + "--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 - return rebuilds_app(files) + # A submodule bump (vendor/bonsplit) is listed as the bare submodule + # path, while the local records see the .swift files under it. + return rebuilds_app(files) or any( + path in submodules(workspace) and path.startswith(PACKAGE_SOURCES) for path in files + ) + + +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: @@ -456,7 +472,7 @@ def prefer(store: Path, workspace: Path, prefix: str, revision: str, max_distanc + ("" 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 from the kept DerivedData (365 to 482 s on 2026-09-25). + # 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}") diff --git a/tests/test_ci_owned_build_state.py b/tests/test_ci_owned_build_state.py index 2fc36e9a0894..e8b41f3965c1 100644 --- a/tests/test_ci_owned_build_state.py +++ b/tests/test_ci_owned_build_state.py @@ -463,6 +463,28 @@ def fake(argv, **_): 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)}), \ From bef1de725cb4066c4878d18911618de6e6262351 Mon Sep 17 00:00:00 2001 From: teamleaderleo Date: Fri, 25 Sep 2026 05:33:06 -0400 Subject: [PATCH 3/3] ci: read .gitmodules once in the bucket seed check; align timing notes Co-Authored-By: Claude Opus 5.5 --- scripts/ci/owned_build_state.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/scripts/ci/owned_build_state.py b/scripts/ci/owned_build_state.py index 6558a2f1f0bb..c3be42462d95 100644 --- a/scripts/ci/owned_build_state.py +++ b/scripts/ci/owned_build_state.py @@ -77,7 +77,7 @@ 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. @@ -92,7 +92,7 @@ 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 958 s). +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 @@ -389,9 +389,10 @@ def bucket_seed_rebuilds_app(key: str, workspace: Path) -> bool | None: return None # A submodule bump (vendor/bonsplit) is listed as the bare submodule # path, while the local records see the .swift files under it. - return rebuilds_app(files) or any( - path in submodules(workspace) and path.startswith(PACKAGE_SOURCES) for path in files - ) + 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]: