ci: run every app-host unit shard when a PR changes the shard layout

CLAUDE.md

@@ -144,7 +144,9 @@ declares or extends, and an app-source diff runs the suites whose tests mention
 what it changed (`reverse_test_impact.py`, #14418), in one changed-suites batch
 with no label (edited suites over its budget take all seven shards). `unit-ci` runs
 every app-host suite across all seven workers; `full-ci` adds the other lanes on
-top. Neither is needed to test the suites you edited. No PR job runs
+top. Neither is needed to test the suites you edited. A change to how the suites
+are laid out over the workers (the timings file, the sharder, the batch runner, or
+the job's matrix and shard env) runs every app-host suite on its own. No PR job runs
 `cmuxUITests/`; `no-full-ci` records a deliberate skip for `suite-coverage`. The label permits eligible app-host shards,
 lag builds, and other full-suite lanes; path routing, release routing, and job
 dependencies still apply. It does not request every repository test. Inspect

scripts/ci/choose_ci_suite.py

@@ -104,6 +104,20 @@
 # executed and were accounted for, which is what a consumer edit can break.
 CONSUMER_CANARY_SELECTOR = "cmuxTests/CmuxSSHURLRequestTests"
 
+# What decides which suites share a worker and in what order they run. A new
+# layout can put one suite after another that leaves state behind, and only
+# running every shard shows that: #14393 took the canary, merged, and main
+# failed four suites that only fail in the new order. These run every unit
+# suite, as `unit-ci` does, but not the rest of the full suite.
+# generate_test_timings.py is left out: no CI job runs it, and its layout
+# change arrives as the timings file it writes.
+SHARD_LAYOUT_PATHS = (
+    MACOS_WORKFLOW_PATH,  # only shard_layout_lines() count
+    "scripts/ci/cmux-unit-test-timings.json",
+    "scripts/ci/cmux_unit_test_shard.py",
+    "scripts/ci/run-app-host-unit-batches.sh",
+)
+
 
 def diff_needs_the_suite(paths: Iterable[str] | None) -> bool:
     """True when the diff contains changes compile admission cannot judge.
@@ -361,6 +375,79 @@ def consumer_canary_selectors(
     return []
 
 
+SHARD_LAYOUT_SETTING_RE = re.compile(r"^      CMUX_APP_HOST_[A-Z_]*(SHARD|RESERVED_WALL_SECONDS):")
+SHARD_MATRIX_ENTRY_RE = re.compile(r'^\s*\{"shard":')
+
+
+def shard_layout_lines(workflow: str) -> set[int]:
+    """1-based lines of `app-host unit tests` that lay out its shards.
+
+    Its `strategy:` block (the shard matrix) and the job env that places a
+    strict step on a shard or reserves its time there.
+    """
+    job = job_lines(workflow, APP_HOST_CONSUMER_JOB)
+    if job is None:
+        return set()
+    lines = workflow.splitlines()
+    layout: set[int] = set()
+    in_strategy = False
+    for number in job:
+        text = lines[number - 1]
+        if re.match(r"^    [A-Za-z_-]+:", text):
+            in_strategy = text.startswith("    strategy:")
+        if in_strategy or SHARD_LAYOUT_SETTING_RE.match(text):
+            layout.add(number)
+    return layout
+
+
+def removed_shard_layout_setting(diff: str) -> bool:
+    """True when a ci-macos.yml hunk removes a line that set the shard layout.
+
+    changed_lines() reports new-side lines only, so a shard setting that an
+    edit deletes or renames to another key would not show up in
+    shard_layout_lines() of the new workflow.
+    """
+    path: str | None = None
+    for line in diff.splitlines():
+        if line.startswith("+++ "):
+            target = line[4:].strip()
+            path = target[2:] if target.startswith("b/") else None
+            continue
+        if line.startswith("--- "):
+            continue
+        if path == MACOS_WORKFLOW_PATH and line.startswith("-"):
+            removed = line[1:]
+            if SHARD_LAYOUT_SETTING_RE.match(removed) or SHARD_MATRIX_ENTRY_RE.match(removed):
+                return True
+    return False
+
+
+def shard_layout_changed(root: Path, paths: Iterable[str] | None, diff: str | None) -> bool:
+    """True when the diff changes how app-host unit suites are laid out over shards.
+
+    A ci-macos.yml edit counts only when a hunk touches shard_layout_lines(),
+    or when its hunks are missing and the edit cannot be placed. An unreadable
+    file list returns False because the caller already runs every unit suite.
+    """
+    if paths is None:
+        return False
+    stripped = {path.strip() for path in paths}
+    if stripped & set(SHARD_LAYOUT_PATHS[1:]):
+        return True
+    if MACOS_WORKFLOW_PATH not in stripped:
+        return False
+    hunks = changed_lines(diff).get(MACOS_WORKFLOW_PATH) if diff else None
+    if not hunks:
+        return True
+    if removed_shard_layout_setting(diff):
+        return True
+    try:
+        workflow = (root / MACOS_WORKFLOW_PATH).read_text(encoding="utf-8")
+    except (OSError, UnicodeError):
+        return True
+    return bool(hunks & shard_layout_lines(workflow))
+
+
 def runs_in_admission(
     root: Path,
     paths: Iterable[str] | None,
@@ -497,11 +584,13 @@ def main(argv: list[str]) -> int:
             app_diff = None
 
     full = wants_full_suite(args.event_name, args.pull_request_policy, labels)
-    unit = wants_unit_suite(args.event_name, args.pull_request_policy, labels, paths)
+    layout = shard_layout_changed(args.root, paths, diff)
+    unit = layout or wants_unit_suite(args.event_name, args.pull_request_policy, labels, paths)
     gap = coverage_gap(args.event_name, full, paths, labels, unit_suite=unit)
     # Only a unit run the diff asked for narrows. `full-ci` and `unit-ci` are
-    # explicit requests for every suite.
-    asked_for_every_suite = full or UNIT_SUITE_LABEL in {label.strip() for label in labels or ()}
+    # explicit requests for every suite, and a shard layout change needs every
+    # suite in its new order.
+    asked_for_every_suite = full or layout or UNIT_SUITE_LABEL in {label.strip() for label in labels or ()}
     selectors = [] if not unit or asked_for_every_suite else changed_unit_selectors(args.root, paths, diff)
     canary = False
     reached: list[str] = []

tests/test_ci_change_areas.py

@@ -5114,6 +5114,98 @@ def outputs(paths: list[str], label: str = "") -> dict[str, str]:
         f"${{{{ !({dropped}) && steps.suite.outputs.unit_selectors || '' }}}}", changes["outputs"]["unit_selectors"]
 
 
+def test_a_shard_layout_edit_runs_every_app_host_unit_shard() -> None:
+    """A new shard layout puts suites in a new order, and only running all of it shows that.
+
+    #14393 rebalanced the shards from measured timings, took the one-suite
+    consumer canary, and merged; main then failed four suites that only fail
+    in the new order (run 36101756298).
+    """
+    sys.path.insert(0, str(ROOT / "scripts/ci"))
+    from choose_ci_suite import SHARD_LAYOUT_PATHS, shard_layout_changed
+
+    workflow_path = ".github/workflows/ci-macos.yml"
+    lines = MACOS_WORKFLOW.read_text(encoding="utf-8").splitlines()
+    job_start = lines.index("  app-host-unit-tests:") + 1
+
+    def line_of(prefix: str) -> int:
+        return next(number for number, text in enumerate(lines[job_start:], start=job_start + 1)
+                    if text.startswith(prefix))
+
+    def hunk(line: int, count: int = 1) -> str:
+        return f"--- a/{workflow_path}\n+++ b/{workflow_path}\n@@ -{line},{count} +{line},{count} @@\n"
+
+    for path in SHARD_LAYOUT_PATHS[1:]:
+        assert (ROOT / path).is_file(), path
+        assert shard_layout_changed(ROOT, [path], None), path
+    # The generator runs in no CI job; its layout change arrives as the JSON.
+    assert "scripts/ci/generate_test_timings.py" not in SHARD_LAYOUT_PATHS
+    # Other consumer scripts keep the canary.
+    assert not shard_layout_changed(ROOT, ["scripts/ci/app_host_test_products.py"], None)
+    assert not shard_layout_changed(ROOT, ["Sources/Workspace.swift"], None)
+    assert not shard_layout_changed(ROOT, None, None)
+    # The shards' matrix, and the env that places strict steps and reserves
+    # their time on a worker, are the layout.
+    for prefix in ('          {"shard": 3},', "    strategy:", "      CMUX_APP_HOST_RESERVED_WALL_SECONDS:",
+                   "      CMUX_APP_HOST_GLOBAL_SEARCH_SHARD:", "      CMUX_APP_HOST_FOCUSED_REGRESSION_B_SHARD:"):
+        assert shard_layout_changed(ROOT, [workflow_path], hunk(line_of(prefix))), prefix
+    # The rest of the job, and other jobs, are not.
+    for prefix in ("      CMUX_UNIT_TEST_TIMEOUT_SECONDS:", "    timeout-minutes:", "    runs-on:"):
+        assert not shard_layout_changed(ROOT, [workflow_path], hunk(line_of(prefix))), prefix
+    compile_line = lines.index("  macos-compile-admission:") + 2
+    assert not shard_layout_changed(ROOT, [workflow_path], hunk(compile_line))
+    # An edit nothing can place counts.
+    assert shard_layout_changed(ROOT, [workflow_path], None)
+    # A shard setting deleted or renamed to another key counts, although the
+    # new-side line it leaves behind is not a layout line.
+    timeout_line = line_of("      CMUX_UNIT_TEST_TIMEOUT_SECONDS:")
+    for removed in ('      CMUX_APP_HOST_GLOBAL_SEARCH_SHARD: "3"', '          {"shard": 7},'):
+        renamed = (f"--- a/{workflow_path}\n+++ b/{workflow_path}\n@@ -{timeout_line},1 +{timeout_line},1 @@\n"
+                   f"-{removed}\n+      CMUX_APP_HOST_RENAMED: \"3\"\n")
+        assert shard_layout_changed(ROOT, [workflow_path], renamed), removed
+    unrelated = (f"--- a/{workflow_path}\n+++ b/{workflow_path}\n@@ -{timeout_line},1 +{timeout_line},1 @@\n"
+                 "-      CMUX_UNIT_TEST_TIMEOUT_SECONDS: \"1\"\n+      CMUX_UNIT_TEST_TIMEOUT_SECONDS: \"2\"\n")
+    assert not shard_layout_changed(ROOT, [workflow_path], unrelated)
+
+    script = ROOT / "scripts/ci/choose_ci_suite.py"
+    with tempfile.TemporaryDirectory() as directory:
+        changed = Path(directory) / "changed.txt"
+        labels = Path(directory) / "labels.txt"
+        diff = Path(directory) / "tests.diff"
+
+        def outputs(paths: list[str], hunks: str = "", label: str = "") -> dict[str, str]:
+            changed.write_text("".join(f"{path}\n" for path in paths))
+            labels.write_text(label)
+            diff.write_text(hunks)
+            run = subprocess.run(
+                [sys.executable, str(script), "--event-name", "pull_request",
+                 "--pull-request-policy", "compile-only", "--labels-file", str(labels),
+                 "--files-from", str(changed), "--diff-from", str(diff), "--root", str(ROOT)],
+                capture_output=True, text=True, check=True,
+            )
+            return dict(line.split("=", 1) for line in run.stdout.splitlines())
+
+        every_shard = {"full_suite": "false", "unit_suite": "true", "unit_selectors": "",
+                       "unit_strict_steps": "", "unit_canary": "false", "unit_in_admission": "false"}
+        # #14393's files, with and without the workflow hunk.
+        rebalance = ["scripts/ci/cmux-unit-test-timings.json", "scripts/ci/cmux_unit_test_shard.py",
+                     "scripts/ci/generate_test_timings.py", "scripts/ci/run-app-host-unit-batches.sh"]
+        reserved = hunk(line_of("      CMUX_APP_HOST_RESERVED_WALL_SECONDS:"))
+        for paths, hunks in (
+            (rebalance, ""),
+            (rebalance + [workflow_path], reserved),
+            ([workflow_path], reserved),
+            (["scripts/ci/cmux-unit-test-timings.json"], ""),
+            # An edited suite does not narrow a layout change to that suite.
+            (["scripts/ci/cmux-unit-test-timings.json", "cmuxTests/TerminalTabIconRegressionTests.swift"], ""),
+        ):
+            result = outputs(paths, hunks)
+            assert {key: result[key] for key in every_shard} == every_shard, (paths, result)
+        # A consumer hunk elsewhere in the job keeps the one-suite canary.
+        other = outputs([workflow_path], hunk(line_of("      CMUX_UNIT_TEST_TIMEOUT_SECONDS:")))
+        assert (other["unit_selectors"], other["unit_canary"]) == ("cmuxTests/CmuxSSHURLRequestTests", "true"), other
+
+
 def test_the_unit_tier_closes_only_the_gap_its_job_can_judge() -> None:
     sys.path.insert(0, str(ROOT / "scripts/ci"))
     from choose_ci_suite import coverage_gap