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
9 changes: 9 additions & 0 deletions .github/workflows/ci-artifact-transport.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,19 @@ on:
branches: [main]
paths:
- .github/workflows/ci-artifact-transport.yml
- .github/workflows/ci.yml
- .github/workflows/ci-macos.yml
- workers/ci-artifacts/**
- scripts/ci/restore-r2-artifact.py
- scripts/ci/node_product_cache.py
- scripts/ci/peer_product_source.py
- scripts/ci/app-host-products-archive.sh
- scripts/ci/app_host_layer_transport.py
- scripts/ci/app_host_layered_products.py
- tests/test_app_host_layer_transport.py
- tests/test_app_host_layered_products.py
- tests/test_app_host_products_archive.sh
- tests/test_ci_selective_layer_wiring.py
- scripts/ci/restore-app-host-test-product.sh
- tests/test_node_product_cache.py
- scripts/ci/verify-r2-canary.py
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,7 @@ jobs:
PY

suite-coverage:
timeout-minutes: 5
# Compile admission builds the test bundle and stops, so a pull request
# that edits cmuxTests/ and skips the full suite runs none of the code it
# changed. The policy that allows the skip is written for a merge queue
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/cmux-skill-contract.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ on:
- "skills.sh"
- "scripts/validate-cmux-browser-skill.py"
- "tests/test_cmux_browser_skill.py"
- "tests/test_cmux_settings_jsonc.py"
- ".github/workflows/cmux-skill-contract.yml"

permissions:
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/testbox-broker-guard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ jobs:
- name: Check that every workflow is linted
run: python3 tests/test_ci_actionlint_covers_every_workflow.py

- name: Check that push and pull_request path filters agree
run: |
"$TESTBOX_GUARD_PYTHON" tests/test_ci_workflow_path_filter_parity.py

- name: Lint every workflow
env:
ACTIONLINT_VERSION: "1.7.7"
Expand Down
4 changes: 4 additions & 0 deletions tests/test-execution.toml
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,10 @@ lane = "linux-guard"
path = "tests/test_ci_selective_layer_wiring.py"
lane = "linux-guard"

[[test]]
path = "tests/test_ci_workflow_path_filter_parity.py"
lane = "linux-guard"

[[test]]
path = "tests/test_ci_ubuntu_apt_https.py"
lane = "legacy"
Expand Down
159 changes: 159 additions & 0 deletions tests/test_ci_workflow_path_filter_parity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
#!/usr/bin/env python3
"""A workflow that filters both `push` and `pull_request` must filter them alike.

A workflow that names its inputs twice has two lists that can disagree, and
neither pull request that made them disagree can see it. When the `push` list is
the smaller one the failure is silent and expensive: a file is guarded on pull
requests, passes, merges, and is then unguarded on main forever.

Divergence that a maintainer genuinely wants goes in EXEMPTIONS with a reason,
so it is written down rather than implied by a list that looks like a typo.
"""

from pathlib import Path
from tempfile import TemporaryDirectory
import yaml

ROOT = Path(__file__).resolve().parents[1]
WORKFLOWS = ROOT / ".github/workflows"

# name -> why the two filters are deliberately different. Keep this empty unless
# a maintainer has chosen the divergence; an entry that is no longer needed is an
# error, so a synced workflow cannot keep a stale exemption.
EXEMPTIONS: dict[str, str] = {
# The candidate complexity job runs contributor-controlled package install
# scripts. A pull request that edits this workflow must not be able to queue
# the job that would run its own edit, so the pull_request filter omits the
# workflow's own path while push -- trusted and post-merge -- keeps it.
# tests/test_web_complexity_trusted_workflow.py enforces the same boundary.
"web-complexity.yml": "a pull request must not self-queue the candidate job",
}

FILTERS = ("paths", "paths-ignore")


def workflow_files():
"""List every workflow in deterministic order."""
return sorted(
[*WORKFLOWS.glob("*.yml"), *WORKFLOWS.glob("*.yaml")],
key=lambda path: path.name,
)


def triggers(path):
"""Return the workflow's `on:` mapping, or None if it has no usable one."""
document = yaml.safe_load(path.read_text(encoding="utf-8"))
if not isinstance(document, dict):
return None
# YAML 1.1 reads a bare `on:` key as the boolean True.
events = document.get("on", document.get(True))
return events if isinstance(events, dict) else None


def divergence(path):
"""Return {filter: (push_only, pull_request_only)} for the lists that differ."""
events = triggers(path)
if events is None:
return {}
push = events.get("push")
pull_request = events.get("pull_request")
if not isinstance(push, dict) or not isinstance(pull_request, dict):
return {}

differences = {}
for name in FILTERS:
push_patterns = push.get(name) or []
pull_request_patterns = pull_request.get(name) or []
on_push = set(push_patterns)
on_pull_request = set(pull_request_patterns)
# Negated paths can exclude and later re-include a match; their order
# is meaningful. Exclusion-only paths-ignore has no such ordering.
order_sensitive = name == "paths" and any(
pattern.startswith("!") for pattern in (*push_patterns, *pull_request_patterns)
)
differs = (
push_patterns != pull_request_patterns
if order_sensitive
else on_push != on_pull_request
)
if differs:
differences[name] = (
sorted(on_push - on_pull_request),
sorted(on_pull_request - on_push),
)
return differences


def describe(name, differences):
"""Explain membership or ordering differences for a workflow."""
lines = [f"{name}: push and pull_request filter different files"]
for filter_name, (push_only, pull_request_only) in sorted(differences.items()):
if not push_only and not pull_request_only:
lines.append(f" {filter_name}: patterns differ in order or repetition")
if pull_request_only:
lines.append(
f" {filter_name}: guarded on pull requests, not on push: "
+ ", ".join(pull_request_only)
)
if push_only:
lines.append(
f" {filter_name}: guarded on push, not on pull requests: "
+ ", ".join(push_only)
)
return "\n".join(lines)


def test_push_and_pull_request_filter_the_same_files():
"""Reject unapproved differences between trigger path filters."""
drifted = []
for path in workflow_files():
if path.name in EXEMPTIONS:
continue
differences = divergence(path)
if differences:
drifted.append(describe(path.name, differences))
assert not drifted, (
"Every workflow that filters both events must filter them identically, "
"or declare the difference in EXEMPTIONS with a reason.\n"
+ "\n".join(drifted)
)


def test_every_exemption_is_still_needed():
"""Reject obsolete or unexplained parity exemptions."""
names = {path.name for path in workflow_files()}
for name, reason in sorted(EXEMPTIONS.items()):
assert name in names, f"EXEMPTIONS names {name}, which no longer exists"
assert reason.strip(), f"{name}: exemption needs a reason"
assert divergence(WORKFLOWS / name), (
f"{name}: exempted from filter parity but its filters now agree. "
"Remove the EXEMPTIONS entry."
)


def test_filter_pattern_order():
"""Catch negation reordering without making paths-ignore order-sensitive."""
with TemporaryDirectory() as directory:
path = Path(directory) / "workflow.yml"
for filter_name, push, pull_request, expected in (
("paths", ["**", "!docs/**"], ["!docs/**", "**"], {"paths": ([], [])}),
("paths", ["**", "!docs/**"], ["**", "!docs/**"], {}),
("paths", ["docs/**", "tests/**"], ["tests/**", "docs/**"], {}),
("paths-ignore", ["docs/**", "tests/**"], ["tests/**", "docs/**"], {}),
("paths", ["src/**"], ["tests/**"], {"paths": (["src/**"], ["tests/**"])}),
):
path.write_text(yaml.safe_dump({"on": {
"push": {filter_name: push},
"pull_request": {filter_name: pull_request},
}}), encoding="utf-8")
assert divergence(path) == expected, (filter_name, push, pull_request)
assert "patterns differ in order or repetition" in describe(
"workflow.yml", {"paths": ([], [])}
)


if __name__ == "__main__":
test_filter_pattern_order()
test_push_and_pull_request_filter_the_same_files()
test_every_exemption_is_still_needed()
print("ok")
Loading