Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion .github/workflows/cmux-next.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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: |
Expand Down
19 changes: 19 additions & 0 deletions docs/ci/cmux-next-tiers.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
52 changes: 52 additions & 0 deletions scripts/ci/cmux_next_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment thread
cursor[bot] marked this conversation as resolved.


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:
Expand Down Expand Up @@ -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))
Expand Down
130 changes: 130 additions & 0 deletions scripts/ci/cmux_next_route_replay.py
Original file line number Diff line number Diff line change
@@ -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("][", ","))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replay pagination breaks on large PRs

Medium Severity

recent_prs asks gh api --paginate for filenames as one JSON array per page, then joins pages with a ][ replace. Paginated gh --jq output is newline-separated arrays, so json.loads throws on any merged PR with more than 100 files and the routing replay fails closed. The rest of this repo already fetches those names as one path per line.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit e6de287. Configure here.

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())
4 changes: 4 additions & 0 deletions tests/test-execution.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
56 changes: 52 additions & 4 deletions tests/test_cmux_next_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
Expand Down
Loading
Loading