diff --git a/.github/workflows/cmux-next.yml b/.github/workflows/cmux-next.yml index f79ecdd94e06..7c9c7712b25b 100644 --- a/.github/workflows/cmux-next.yml +++ b/.github/workflows/cmux-next.yml @@ -691,7 +691,29 @@ jobs: - name: Tier routing id: tier-routing continue-on-error: true - run: python3 tests/test_cmux_next_route.py + run: | + python3 tests/test_cmux_next_route.py + python3 tests/test_cmux_next_route_replay.py + + # A pull request that changes the router replays the last 30 merged PRs' changed files + # through the base router (the merge commit's HEAD^1) and its own: a tier the new router + # adds, or drops without naming a fast tier, fails (scripts/ci/cmux_next_route_replay.py). + - name: Routing replay (router changes) + id: routing-replay + continue-on-error: true + env: + GH_TOKEN: ${{ github.token }} + EVENT_NAME: ${{ github.event_name }} + run: | + if [[ "$EVENT_NAME" != pull_request ]]; then + echo "Pushes run every tier; the replay checks pull request routing only." + exit 0 + fi + if git diff --quiet HEAD^1 HEAD -- scripts/ci/cmux_next_route.py scripts/ci/select_package_tests.py; then + echo "The router did not change." + exit 0 + fi + python3 scripts/ci/cmux_next_route_replay.py --base HEAD^1 --limit 30 | tee -a "$GITHUB_STEP_SUMMARY" # Package.swift links the app FFI from one pinned release; the pin must # match the FFI sources (every crate cmux-app-ffi reaches through Cargo @@ -732,6 +754,7 @@ jobs: ${{ steps.package-conventions.outcome }}|Package conventions lint ${{ steps.checks-structure.outcome }}|Checks job structure ${{ steps.tier-routing.outcome }}|Tier routing + ${{ steps.routing-replay.outcome }}|Routing replay (router changes) ${{ steps.app-ffi-pin.outcome }}|Check the app FFI pin ${{ steps.daemon-capabilities-fresh.outcome }}|Daemon capabilities export is fresh run: | diff --git a/docs/ci/cmux-next-tiers.md b/docs/ci/cmux-next-tiers.md index bfece10c39a2..169348583055 100644 --- a/docs/ci/cmux-next-tiers.md +++ b/docs/ci/cmux-next-tiers.md @@ -14,6 +14,25 @@ job's summary lists each tier and the reason for it. | swift | `cmux-next swift test` | the test targets the target graph reaches | mini | | daemon | `cmux-next daemon tests` (`same-tree cmux-tui` reports a tree that nothing will publish) | cmux-tui tree inputs, daemon capabilities, `pin-cmux-tui.sh`, CmuxNextDaemon or CmuxNextMobile and their dependencies, CmuxNextControl | a mini, once the tree is published | +A web nit runs no Mac tier. When every changed file is under `webviews/`, the +webviews app, the agent-pane bundle (`CmuxNextAgentPane/Resources/agent-pane/`) +or docs, and no Swift test reads it, only `cmux-next checks` runs here. ci-web +type-checks, lints and tests the sources, and `build-agent-pane-web.sh --check` +proves the committed bundle matches them; the bundle is a `.copy` resource, so +the app takes it without a compile. gallery-pr diffs the touched entries. A +`dev-build` PR still compiles its dogfood app. + +A CI-only change runs no Mac tier either: workflows, `scripts/ci/`, the router, +`scripts/gh-merge-green` and their Python tests (alone or with web files). ci.yml's +guards run actionlint and the CI unit tests. When the router itself changes, +`Routing replay (router changes)` replays the last 30 merged PRs' changed files +through the base router and the PR's own (`scripts/ci/cmux_next_route_replay.py`) +and fails on any tier the new router adds, or drops without naming a fast tier. +`gh-merge-green --revert OWNER/REPO#N` undoes a bad one in a command. A file +that a Mac tier reads keeps its tier whatever its name: the Xcode pins, +`.github/actions/setup-cmux-tui-rust/`, daemon and tree inputs, generated-file +inputs such as `plans/cmux-next/`, and every file a Swift test reads. + Every tier runs on: - a pull request labeled `full-ci`. Use it on a batch integration PR, so the diff --git a/scripts/ci/cmux_next_route.py b/scripts/ci/cmux_next_route.py index d6f66e05bd9a..2acd331627b5 100755 --- a/scripts/ci/cmux_next_route.py +++ b/scripts/ci/cmux_next_route.py @@ -112,6 +112,47 @@ "scripts/build-webviews-app.sh", "scripts/check-webviews-react-compiler.mjs", ) +# Committed web bundles that ci-web rebuilds and compares (`--check`) on every pull request, +# and that the app takes as a `.copy` resource: a change to them needs no compile. +CHECKED_WEB_BUNDLES = ( + PACKAGE + "Sources/CmuxNextAgentPane/Resources/agent-pane/*", +) + +# CI-only files: workflows, CI scripts, the router and gh-merge-green, and their tests. actionlint +# and the CI unit tests (ci.yml's guards) and the routing replay (the checks job) cover them on +# Linux, and `gh-merge-green --revert` undoes one in a command. +CI_ONLY = ( + ".github/workflows/*", "scripts/ci/*", "scripts/gh-merge-green", + "tests/test_cmux_next_route.py", "tests/test_cmux_next_route_replay.py", + "tests/test_cmux_next_checks_workflow.py", "tests/test_gh_merge_green*.py", + "tests/test_base_red_excuse.py", "tests/test_ci_*.py", +) +# The CI files that route themselves (the router reads them; the replay checks them). +CI_SELF = (".github/workflows/cmux-next.yml", "scripts/ci/cmux_next_route.py", "scripts/ci/select_package_tests.py") + + +def web_fast(path: str) -> bool: + """Web sources, their checked bundles, docs: ci-web and the checks job cover them all.""" + if any(fnmatch.fnmatch(path, pattern) for pattern in WEBVIEW + CHECKED_WEB_BUNDLES): + return True + return any(fnmatch.fnmatch(path, pattern) for pattern in WEB_ONLY) and not path.startswith(PACKAGE) + + +def ci_only(path: str) -> bool: + return any(fnmatch.fnmatch(path, pattern) for pattern in CI_ONLY) + + +def consumed(path: str, tree_inputs: list[str], read) -> bool: + """A Mac tier reads this file (a toolchain pin, a job script, a daemon or tree input, a + generated-file input or a test fixture), whatever its name looks like.""" + if path in FULL_INPUTS and path not in CI_SELF: + return True + if path in DAEMON_PATHS or any(matches(path, p) for p in SWIFT_JOB_INPUTS + tuple(tree_inputs)): + return True + if any(matches(path, p) for p in GENERATED_INPUTS) and not any(fnmatch.fnmatch(path, b) for b in CHECKED_WEB_BUNDLES): + return True + return read(path) + @dataclass class Route: @@ -195,6 +236,17 @@ def route(root: Path, event: str, changed: list[str] | None, labels: set[str]) - graph = load_graph(root) tests = {name for name, target in graph["targets"].items() if target["kind"] == "test"} + read = lambda path: any(matches(path, r) for name in tests for r in graph["targets"][name].get("reads", [])) # noqa: E731 + # The fast tiers: a web nit or a CI change lands in minutes. ci-web's bundle --check stands in for + # the app compile; actionlint, the CI tests and the routing replay check CI changes. A file a Mac + # tier reads keeps its tier, and a dev-build PR still compiles its dogfood app. + tree_inputs_early = tree_input_paths(root) + if "dev-build" not in labels and all( + (web_fast(path) or ci_only(path)) and not consumed(path, tree_inputs_early, read) for path in changed + ): + kinds = sorted({"CI" if ci_only(path) else "web" for path in changed}) + result.reasons.append(f"only {' and '.join(kinds)} files changed: Linux checks cover them (no Mac tier)") + return result packages = package_inputs(root, graph) tree_inputs = tree_input_paths(root) daemon_closure = closure(graph, set(LIVE_DAEMON_SUBJECTS)) diff --git a/scripts/ci/cmux_next_route_replay.py b/scripts/ci/cmux_next_route_replay.py new file mode 100644 index 000000000000..e51069c2bd40 --- /dev/null +++ b/scripts/ci/cmux_next_route_replay.py @@ -0,0 +1,130 @@ +#!/usr/bin/env python3 +"""Replays recent pull requests' changed files through the base router and this tree's router. + +The checks job runs it when a pull request changes the router: the old router is the merge +commit's base parent (HEAD^1), the new one is the working tree. A tier change is explained only +when the new router drops tiers and names the change as a fast tier (its "no Mac tier" reason); +any other change (an added tier, a dropped tier without that reason, different Swift test +targets) is unexplained and fails. + +Usage: cmux_next_route_replay.py [--base HEAD^1] [--limit 30] [--repo manaflow-ai/cmux] +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import subprocess +import sys +import tempfile +from dataclasses import dataclass +from pathlib import Path +from typing import Callable + +ROOT = Path(__file__).resolve().parents[2] +TIERS = ("native", "macos", "scheme", "generated", "swift", "daemon", "full") +FAST_REASON = "(no Mac tier)" +# The router imports these from scripts/ci; the old router gets its own copies. +ROUTER_FILES = ("cmux_next_route.py", "select_package_tests.py") + +Router = Callable[[list[str]], tuple[dict[str, str], list[str]]] + + +@dataclass +class Verdict: + kind: str # same, explained, unexplained + detail: str = "" + + +def compare(old: dict[str, str], new: dict[str, str], new_reasons: list[str]) -> Verdict: + added = [t for t in TIERS if old[t] == "false" and new[t] == "true"] + dropped = [t for t in TIERS if old[t] == "true" and new[t] == "false"] + targets = old.get("swift_targets", "") != new.get("swift_targets", "") and not dropped + if not added and not dropped and not targets: + return Verdict("same") + if added or targets: + return Verdict("unexplained", f"added {', '.join(added) or 'none'}; swift targets " + f"{old.get('swift_targets') or '-'} -> {new.get('swift_targets') or '-'}") + if any(FAST_REASON in reason for reason in new_reasons): + return Verdict("explained", f"dropped {', '.join(dropped)}: {next(r for r in new_reasons if FAST_REASON in r)}") + return Verdict("unexplained", f"dropped {', '.join(dropped)} with no fast-tier reason") + + +def replay(prs: list[tuple[int, list[str]]], old: Router, new: Router) -> tuple[list[str], bool]: + lines, failed = [], False + for number, files in prs: + before, _ = old(files) + after, reasons = new(files) + verdict = compare(before, after, reasons) + failed |= verdict.kind == "unexplained" + label = "UNEXPLAINED" if verdict.kind == "unexplained" else verdict.kind + lines.append(f"#{number} {label} ({len(files)} files){': ' + verdict.detail if verdict.detail else ''}") + return lines, failed + + +def _load(directory: Path, name: str): + spec = importlib.util.spec_from_file_location(name, directory / "cmux_next_route.py") + module = sys.modules[name] = importlib.util.module_from_spec(spec) + sys.path.insert(0, str(directory)) + try: + sys.modules.pop("select_package_tests", None) + spec.loader.exec_module(module) + finally: + sys.path.remove(str(directory)) + sys.modules.pop("select_package_tests", None) + return module + + +def _router(module, root: Path) -> Router: + def route(files: list[str]) -> tuple[dict[str, str], list[str]]: + result = module.route(root, "pull_request", files, set()) + return module.outputs(result), list(result.reasons) + return route + + +def load_router(root: Path, ref: str | None) -> Router: + """The router at a git ref (copied out of git), or the working tree's when ref is None.""" + if ref is None: + return _router(_load(root / "scripts" / "ci", "cmux_next_route_new"), root) + directory = Path(tempfile.mkdtemp(prefix="route-")) + for name in ROUTER_FILES: + text = subprocess.run(["git", "show", f"{ref}:scripts/ci/{name}"], cwd=root, check=True, + capture_output=True, text=True).stdout + (directory / name).write_text(text) + return _router(_load(directory, "cmux_next_route_old"), root) + + +def recent_prs(repo: str, limit: int) -> list[tuple[int, list[str]]]: + """The last `limit` merged pull requests into feat-cmux-next and their changed files.""" + pulls = json.loads(subprocess.run( + ["gh", "api", f"repos/{repo}/pulls?state=closed&base=feat-cmux-next&sort=updated&direction=desc&per_page=100"], + check=True, capture_output=True, text=True).stdout) + merged = [p["number"] for p in pulls if p.get("merged_at")][:limit] + out = [] + for number in merged: + files = json.loads(subprocess.run( + ["gh", "api", "--paginate", f"repos/{repo}/pulls/{number}/files?per_page=100", "--jq", "[.[].filename]"], + check=True, capture_output=True, text=True).stdout.replace("][", ",")) + if files and len(files) < 3000: + out.append((number, files)) + return out + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--base", default="HEAD^1", help="git ref of the old router") + parser.add_argument("--limit", type=int, default=30) + parser.add_argument("--repo", default="manaflow-ai/cmux") + a = parser.parse_args(argv) + prs = recent_prs(a.repo, a.limit) + lines, failed = replay(prs, load_router(ROOT, a.base), load_router(ROOT, None)) + print(f"Routing replay: {len(prs)} merged PRs, base router {a.base} vs this tree") + print("\n".join(lines)) + if failed: + print("::error::The router changes tiers it does not explain; see the UNEXPLAINED lines.") + return 1 if failed else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test-execution.toml b/tests/test-execution.toml index 0dc1e659bed0..9aea91be7563 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -1084,6 +1084,10 @@ lane = "linux-guard" path = "tests/test_ci_macho_parity.py" lane = "linux-guard" +[[test]] +path = "tests/test_cmux_next_route_replay.py" +lane = "linux-guard" + [[test]] path = "tests/test_ci_macos_cross_shadow_jobs.py" lane = "linux-guard" diff --git a/tests/test_cmux_next_route.py b/tests/test_cmux_next_route.py index 1a36e33014c3..c7764335ee2f 100644 --- a/tests/test_cmux_next_route.py +++ b/tests/test_cmux_next_route.py @@ -158,11 +158,59 @@ def test_web_and_docs_need_no_mac(self): self.assertEqual(result["macos"], "false") self.assertEqual(result["swift"], "false") - def test_webviews_keep_one_scheme_compile(self): - result = tiers(["webviews/src/agent-session/pane.tsx"]) + def test_a_web_nit_runs_no_mac_tier(self): + """A webviews change and its regenerated agent-pane bundle (#18296, one CSS line) need no Mac. + + ci-web type-checks, lints and tests the sources and proves with build-agent-pane-web.sh --check + that the committed bundle matches them; the bundle is a `.copy` resource, so the app takes any + file set without a compile. + """ + for changed in ( + ["webviews/src/agent-session/pane.tsx"], + ["webviews/src/agent-session/acpmux/composerAttachments.css", + f"{PACKAGE}/Sources/CmuxNextAgentPane/Resources/agent-pane/index.html"], + ["Resources/markdown-viewer/webviews-app/index.js", "docs/cmux-next.md"], + ): + with self.subTest(changed=changed): + result = tiers(changed) + for tier in ("macos", "scheme", "native", "swift", "generated", "daemon"): + self.assertEqual(result[tier], "false", tier) + + def test_a_web_change_beside_native_code_keeps_the_mac_tiers(self): + result = tiers(["webviews/src/agent-session/pane.tsx", f"{PACKAGE}/Sources/CmuxNextAgentPane/AgentPaneView.swift"]) + self.assertEqual(result["native"], "true") + self.assertEqual(result["scheme"], "true") + + def test_a_web_file_a_swift_test_reads_keeps_that_test(self): + reads = [read for target in GRAPH["targets"].values() for read in target.get("reads", []) + if read.startswith("webviews/") and not read.endswith("/")] + self.assertTrue(reads, "no Swift test reads a webviews file; pick another fixture") + self.assertEqual(tiers([reads[0]])["swift"], "true") + + def test_a_ci_only_change_runs_no_mac_tier(self): + """Workflows, CI scripts, the router and gh-merge-green: actionlint, the CI unit tests and the + routing replay check them on Linux, and one command reverts them.""" + for changed in ( + [".github/workflows/cmux-next.yml", "scripts/ci/cmux_next_route.py", "tests/test_cmux_next_route.py"], + ["scripts/gh-merge-green", "scripts/ci/revert_pr.py", "tests/test_gh_merge_green_revert.py"], + ["scripts/ci/select_package_tests.py", "webviews/src/agent-session/pane.tsx"], + ): + with self.subTest(changed=changed): + result = tiers(changed) + for tier in ("macos", "scheme", "native", "swift", "generated", "daemon", "full"): + self.assertEqual(result[tier], "false", tier) + + def test_ci_files_a_mac_job_consumes_keep_their_tier(self): + self.assertEqual(tiers(["scripts/ci/xcode-pins.txt"])["full"], "true") + self.assertEqual(tiers([".github/actions/setup-cmux-tui-rust/action.yml"])["swift"], "true") + + def test_a_doc_a_generator_reads_keeps_the_generated_tier(self): + """`*.md` looks like docs, but plans/cmux-next/ feeds the action contracts.""" + self.assertEqual(tiers(["plans/cmux-next/actions.md"])["generated"], "true") + + def test_dev_build_label_keeps_the_scheme_compile_for_a_web_change(self): + result = tiers(["webviews/src/agent-session/pane.tsx"], labels=frozenset({"dev-build"})) self.assertEqual(result["scheme"], "true") - self.assertEqual(result["native"], "false") - self.assertEqual(result["swift"], "false") def test_the_app_host_compiles_the_scheme_without_package_tests(self): result = tiers(["App/main.swift"]) diff --git a/tests/test_cmux_next_route_replay.py b/tests/test_cmux_next_route_replay.py new file mode 100644 index 000000000000..cd5157bbf2d7 --- /dev/null +++ b/tests/test_cmux_next_route_replay.py @@ -0,0 +1,80 @@ +#!/usr/bin/env python3 +"""The routing replay: recent PRs' changed files through the base router and the PR's router. + +A router change may only drop tiers for files the new router names as cheap (its fast tiers). +Any other tier change, added or dropped, is unexplained and fails the checks job. +""" + +from __future__ import annotations + +import sys +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "scripts" / "ci")) + +import cmux_next_route_replay as replay # noqa: E402 + +FAST = "only web files changed: Linux checks cover them (no Mac tier)" + + +def tiers(**on: bool) -> dict[str, str]: + keys = ("native", "macos", "scheme", "generated", "swift", "daemon", "full") + return {key: "true" if on.get(key) else "false" for key in keys} | {"swift_targets": on.get("targets", "")} + + +class Compare(unittest.TestCase): + def test_the_same_tiers_are_unchanged(self): + verdict = replay.compare(tiers(native=True, macos=True), tiers(native=True, macos=True), []) + self.assertEqual(verdict.kind, "same") + + def test_tiers_dropped_by_a_fast_tier_are_explained(self): + verdict = replay.compare(tiers(scheme=True, macos=True), tiers(), [FAST]) + self.assertEqual(verdict.kind, "explained") + self.assertIn("scheme", verdict.detail) + + def test_a_dropped_tier_without_a_fast_reason_is_unexplained(self): + verdict = replay.compare(tiers(swift=True, macos=True, targets="A"), tiers(), ["x is read by A"]) + self.assertEqual(verdict.kind, "unexplained") + + def test_an_added_tier_is_unexplained(self): + verdict = replay.compare(tiers(), tiers(native=True, macos=True), []) + self.assertEqual(verdict.kind, "unexplained") + + def test_changed_swift_targets_are_unexplained(self): + verdict = replay.compare(tiers(swift=True, targets="A"), tiers(swift=True, targets="A B"), []) + self.assertEqual(verdict.kind, "unexplained") + + +class Replay(unittest.TestCase): + def test_the_report_fails_on_any_unexplained_pr(self): + def old(files): + return tiers(scheme=True, macos=True), [] + + def new(files): + return (tiers(), [FAST]) if files == ["webviews/a.ts"] else (tiers(native=True, macos=True), []) + + prs = [(1, ["webviews/a.ts"]), (2, ["App/main.swift"])] + lines, failed = replay.replay(prs, old, new) + self.assertTrue(failed) + self.assertTrue(any(line.startswith("#1 explained") for line in lines), lines) + self.assertTrue(any(line.startswith("#2 UNEXPLAINED") for line in lines), lines) + + def test_identical_routers_pass(self): + def same(files): + return tiers(native=True, macos=True), [] + + _, failed = replay.replay([(1, ["a"]), (2, ["b"])], same, same) + self.assertFalse(failed) + + def test_the_base_router_loads_from_a_git_ref(self): + """The old router is the PR's base parent (HEAD^1 of the merge commit); here, HEAD.""" + route = replay.load_router(ROOT, "HEAD") + result, reasons = route(["docs/cmux-next.md"]) + self.assertEqual(result["macos"], "false") + self.assertIsInstance(reasons, list) + + +if __name__ == "__main__": + unittest.main()