From 8aa77d6049c1c76499f14d0f300fd52c05076c3a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 18:51:08 +0000 Subject: [PATCH 1/3] test: red tests for a preflight matrix spanning every CI workflow preflight reads one workflow file, `.github/workflows/conventions.yml`, which #861 deleted when it split that file into one workflow per domain. `just preflight` -- the command AGENTS.md mandates before every push -- has been dying with FileNotFoundError ever since. These assert the shape the fix needs: parse_gate_matrix takes the set of workflow documents rather than one, and the CLI globs the workflow directory (sorted, so the report does not depend on readdir order). skip-changelog: internals/ is not a published package root Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KcNqe44kvgP5mBRr9LjQcE --- .../checks/src/checks/preflight/cli_test.py | 52 +++++++++++------ .../src/checks/preflight/matrix_test.py | 57 ++++++++++++++----- 2 files changed, 80 insertions(+), 29 deletions(-) diff --git a/internals/checks/src/checks/preflight/cli_test.py b/internals/checks/src/checks/preflight/cli_test.py index e6fdaf9a7..960a2b19f 100644 --- a/internals/checks/src/checks/preflight/cli_test.py +++ b/internals/checks/src/checks/preflight/cli_test.py @@ -1,6 +1,6 @@ """Colocated unit tests for the preflight command (isolation -- no `CliRunner`). -Driven through `.callback`; `run` and `open` are mocked at their import site. +Driven through `.callback`; `run`, `glob` and `open` are mocked at their import site. """ from unittest import mock @@ -9,35 +9,55 @@ from checks.preflight.cli import cli +WORKFLOWS = [".github/workflows/b-ci.yml", ".github/workflows/a-ci.yml"] -def invoke(gates=(), dry_run=False, **kwargs): - with mock.patch("checks.preflight.cli.open", mock.mock_open(read_data="jobs: {}")) as opened: - with mock.patch("checks.preflight.cli.run", **kwargs) as run: - with pytest.raises(SystemExit) as exc_info: - cli.callback(conventions="wf.yml", base="origin/x", gates=gates, dry_run=dry_run) - return opened, run, exc_info.value.code +def invoke(gates=(), dry_run=False, found=WORKFLOWS, **kwargs): + opened = mock.mock_open(read_data="jobs: {}") + with mock.patch("checks.preflight.cli.open", opened): + with mock.patch("checks.preflight.cli.glob", return_value=list(found)) as globbed: + with mock.patch("checks.preflight.cli.run", **kwargs) as run: + with pytest.raises(SystemExit) as exc_info: + cli.callback(workflows="wf-dir", base="origin/x", gates=gates, dry_run=dry_run) + return globbed, opened, run, exc_info.value.code -def test_reads_the_workflow_and_exits_with_runs_return_code(): - opened, run, code = invoke(return_value=0) - opened.assert_called_once_with("wf.yml", encoding="utf-8") - assert run.call_args.args == ("jobs: {}", "origin/x") + +def test_globs_the_workflow_directory_for_yaml_files(): + globbed, _opened, _run, _code = invoke(return_value=0) + globbed.assert_called_once_with("wf-dir/*.yml") + + +def test_reads_every_workflow_and_exits_with_runs_return_code(): + _globbed, opened, run, code = invoke(return_value=0) + assert opened.call_args_list == [ + mock.call(".github/workflows/a-ci.yml", encoding="utf-8"), + mock.call(".github/workflows/b-ci.yml", encoding="utf-8"), + ] + assert run.call_args.args == (["jobs: {}", "jobs: {}"], "origin/x") assert code == 0 +def test_reads_the_workflows_in_sorted_order(): + # Glob order is filesystem-dependent; the matrix (and so the report) must not be. + _globbed, opened, _run, _code = invoke(return_value=0) + assert [call.args[0] for call in opened.call_args_list] == sorted(WORKFLOWS) + + def test_propagates_a_failing_matrix_as_exit_one(): - assert invoke(return_value=1)[2] == 1 + assert invoke(return_value=1)[3] == 1 def test_forwards_the_gate_filter_and_dry_run_flag(): - _opened, run, _code = invoke(gates=("unit-lint",), dry_run=True, return_value=0) + _globbed, _opened, run, _code = invoke( + gates=("unit-lint",), dry_run=True, return_value=0 + ) assert run.call_args.kwargs["only"] == ("unit-lint",) assert run.call_args.kwargs["dry_run"] is True -def test_defaults_to_the_ci_workflow_and_main(): - conventions, base, gates, dry_run = cli.params - assert conventions.default == ".github/workflows/conventions.yml" +def test_defaults_to_the_workflow_directory_and_main(): + workflows, base, gates, dry_run = cli.params + assert workflows.default == ".github/workflows" assert base.default == "origin/main" assert gates.multiple is True assert dry_run.is_flag is True diff --git a/internals/checks/src/checks/preflight/matrix_test.py b/internals/checks/src/checks/preflight/matrix_test.py index b60a4ada6..6f31473b1 100644 --- a/internals/checks/src/checks/preflight/matrix_test.py +++ b/internals/checks/src/checks/preflight/matrix_test.py @@ -2,7 +2,7 @@ from checks.preflight.matrix import GATES, ROOT_GATES, parse_gate_matrix, pairs -CONVENTIONS = """ +PYTHON_CI = """ jobs: python-sdk: uses: x/y/.github/workflows/testing-conventions.yml@v0 @@ -12,6 +12,10 @@ config: testing-conventions.toml unrelated: runs-on: ubuntu-latest +""" + +RUST_CI = """ +jobs: rust: uses: x/y/.github/workflows/testing-conventions.yml@v0 with: @@ -20,34 +24,56 @@ gates: '["colocated-test", "packaging"]' """ +WORKFLOWS = [PYTHON_CI, RUST_CI] + def describe_parse_gate_matrix(): def it_reads_each_callers_source_gates_and_config(): - entries = parse_gate_matrix(CONVENTIONS) - assert [e.source for e in entries] == ["packages/python/dirsql", "packages/rust"] + entries = parse_gate_matrix(WORKFLOWS) + assert [e.source for e in entries] == [ + "packages/python/dirsql", + "packages/rust", + ] assert entries[0].config == "testing-conventions.toml" assert entries[0].languages == ["python"] assert entries[1].gates == ["colocated-test", "packaging"] + def it_collects_callers_across_every_workflow_in_document_order(): + # The matrix spans the per-domain workflows (#861 split `conventions.yml` + # into one file per domain), so a caller in the last file is a pair too. + assert [e.job for e in parse_gate_matrix(WORKFLOWS)] == ["python-sdk", "rust"] + assert [e.job for e in parse_gate_matrix([RUST_CI, PYTHON_CI])] == [ + "rust", + "python-sdk", + ] + + def it_ignores_a_workflow_that_declares_no_jobs(): + # `.github/workflows/` holds non-gate workflows too; one without a `jobs:` + # mapping must contribute nothing rather than raise. + assert parse_gate_matrix(["on: push\n"]) == [] + + def it_ignores_an_empty_workflow_document(): + assert parse_gate_matrix([""]) == [] + def it_defaults_gates_to_the_full_set_when_a_caller_omits_them(): # An omitted `gates:` means the reusable workflow's default -- the full # set. Treating it as "no gates" would silently skip a root. - (python, _rust) = parse_gate_matrix(CONVENTIONS) + (python, _rust) = parse_gate_matrix(WORKFLOWS) assert python.gates == ROOT_GATES def it_leaves_config_empty_when_a_caller_omits_it(): - (_python, rust) = parse_gate_matrix(CONVENTIONS) + (_python, rust) = parse_gate_matrix(WORKFLOWS) assert rust.config == "" def it_defaults_languages_to_empty_when_a_caller_omits_them(): - text = CONVENTIONS.replace(" languages: '[\"rust\"]'\n", "") - (_python, rust) = parse_gate_matrix(text) + text = RUST_CI.replace(" languages: '[\"rust\"]'\n", "") + (_python, rust) = parse_gate_matrix([PYTHON_CI, text]) assert rust.languages == [] def it_ignores_jobs_that_do_not_call_the_reusable_workflow(): - # `unrelated` sits BETWEEN two callers in the fixture, so skipping it must - # not stop the walk at the first non-caller. - assert [e.job for e in parse_gate_matrix(CONVENTIONS)] == ["python-sdk", "rust"] + # `unrelated` sits AFTER a caller in its file, so skipping it must not + # stop the walk at the first non-caller. + assert [e.job for e in parse_gate_matrix([PYTHON_CI])] == ["python-sdk"] def describe_GATES(): @@ -85,7 +111,9 @@ def it_maps_each_gate_to_its_cli_subcommand(): def describe_pairs(): def it_crosses_every_root_language_and_gate(): - flat = [(r.job, lang, gate) for r, lang, gate in pairs(parse_gate_matrix(CONVENTIONS))] + flat = [ + (r.job, lang, gate) for r, lang, gate in pairs(parse_gate_matrix(WORKFLOWS)) + ] assert flat == [ *(("python-sdk", "python", gate) for gate in ROOT_GATES), ("rust", "rust", "colocated-test"), @@ -93,5 +121,8 @@ def it_crosses_every_root_language_and_gate(): ] def it_drops_a_root_that_declares_no_languages(): - text = CONVENTIONS.replace("'[\"rust\"]'", "'[]'") - assert all(lang != "rust" for _root, lang, _gate in pairs(parse_gate_matrix(text))) + text = RUST_CI.replace("'[\"rust\"]'", "'[]'") + assert all( + lang != "rust" + for _root, lang, _gate in pairs(parse_gate_matrix([PYTHON_CI, text])) + ) From f388f328670f9d1634926c00c2319de470f9b1d1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 18:53:25 +0000 Subject: [PATCH 2/3] fix: derive the preflight matrix from every CI workflow #861 split conventions.yml into one workflow per domain, but preflight still opened that single deleted file, so `just preflight` raised FileNotFoundError instead of running a gate. parse_gate_matrix now folds the callers across every document, and the CLI globs .github/workflows for them. Documents that call the reusable workflow nowhere -- release, docs, the gate itself -- contribute nothing rather than raising on a missing `jobs:` key. Restores all 8 scan roots and 40 pairs. skip-changelog: internals/ is not a published package root Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01KcNqe44kvgP5mBRr9LjQcE --- internals/checks/src/checks/preflight/cli.py | 21 +++++---- internals/checks/src/checks/preflight/gate.py | 6 +-- .../checks/src/checks/preflight/gate_test.py | 19 ++++---- .../checks/src/checks/preflight/matrix.py | 43 +++++++++++-------- .../src/checks/preflight/matrix_test.py | 14 +++++- 5 files changed, 64 insertions(+), 39 deletions(-) diff --git a/internals/checks/src/checks/preflight/cli.py b/internals/checks/src/checks/preflight/cli.py index 347da8e32..dc5c6e074 100644 --- a/internals/checks/src/checks/preflight/cli.py +++ b/internals/checks/src/checks/preflight/cli.py @@ -1,12 +1,13 @@ """The preflight check -- repo-only (#781). Backs `dirsql-checks preflight`: derive the testing-conventions gate matrix from -`.github/workflows/conventions.yml` and run every pair locally. +the CI workflows and run every pair locally. """ from __future__ import annotations import os.path +from glob import glob import click @@ -15,9 +16,9 @@ @click.command() @click.option( - "--conventions", - default=".github/workflows/conventions.yml", - help="Workflow whose reusable-workflow callers define the gate matrix.", + "--workflows", + default=".github/workflows", + help="Directory whose reusable-workflow callers define the gate matrix.", ) @click.option("--base", default="origin/main", help="Base ref the diff-scoped gates measure against.") @click.option( @@ -27,12 +28,16 @@ help="Run only these gates (repeatable). Default: every gate each root declares.", ) @click.option("--dry-run", is_flag=True, help="Print the derived matrix without running it.") -def cli(conventions: str, base: str, gates: tuple[str, ...], dry_run: bool) -> None: - with open(conventions, encoding="utf-8") as handle: - text = handle.read() +def cli(workflows: str, base: str, gates: tuple[str, ...], dry_run: bool) -> None: + texts = [] + # Sorted, so the matrix -- and the report naming a failing pair -- does not + # depend on the order the filesystem happens to hand back. + for path in sorted(glob(f"{workflows}/*.yml")): + with open(path, encoding="utf-8") as handle: + texts.append(handle.read()) raise SystemExit( run( - text, + texts, base, runner=default_runner, exists=os.path.exists, diff --git a/internals/checks/src/checks/preflight/gate.py b/internals/checks/src/checks/preflight/gate.py index d478864c5..47109fe2c 100644 --- a/internals/checks/src/checks/preflight/gate.py +++ b/internals/checks/src/checks/preflight/gate.py @@ -1,7 +1,7 @@ """Run every (root, language, gate) pair CI declares, and report real exit codes (#781). Two failure modes this exists to remove: a hand-maintained pair list that drifts -from `conventions.yml`, and hand-transcribed shell one-liners whose non-zero exit +from the CI workflows, and hand-transcribed shell one-liners whose non-zero exit gets masked (`... ; echo ok`). Here the argv is derived and the exit code is the return value. @@ -153,7 +153,7 @@ def prepare(roots: list[Root], exists: Callable[[str], bool]) -> list[tuple[str, def run( - conventions: str, + workflows: list[str], base: str, *, runner: Callable[[Sequence[str], str], int], @@ -164,7 +164,7 @@ def run( dry_run: bool = False, ) -> int: """Run the whole matrix; return 0 only when every pair passed.""" - roots = parse_gate_matrix(conventions) + roots = parse_gate_matrix(workflows) failures = [] skipped = [] diff --git a/internals/checks/src/checks/preflight/gate_test.py b/internals/checks/src/checks/preflight/gate_test.py index 9d9b82919..fcd850559 100644 --- a/internals/checks/src/checks/preflight/gate_test.py +++ b/internals/checks/src/checks/preflight/gate_test.py @@ -33,7 +33,7 @@ def __init__(self, job, source, languages, gates, config=None): config="testing-conventions.toml", ) RUST = Root(job="rust", source="packages/rust", languages=["rust"], gates=["packaging"]) -CONVENTIONS = """ +PYTHON_CI = """ jobs: python-sdk: uses: x/.github/workflows/testing-conventions.yml@v0 @@ -42,9 +42,10 @@ def __init__(self, job, source, languages, gates, config=None): source: packages/python/dirsql gates: '["unit-lint", "mutation"]' """ +WORKFLOWS = [PYTHON_CI] # `packaging` first, so a `break` in place of the skip's `continue` would drop # the pair after it. -ARTIFACT_FIRST = CONVENTIONS.replace('"unit-lint", "mutation"', '"packaging", "unit-lint"') +ARTIFACT_FIRST = [PYTHON_CI.replace('"unit-lint", "mutation"', '"packaging", "unit-lint"')] def has_manifest(path: str) -> bool: @@ -212,14 +213,14 @@ def it_keeps_going_past_a_non_python_root_to_the_ones_after_it(): ] -def drive(conventions=None, **kwargs): +def drive(workflows=None, **kwargs): defaults = { "runner": lambda _argv, _cwd: 0, "exists": has_manifest, "e2e_config": lambda _config: {}, "echo": lambda _line: None, } - return run(conventions or CONVENTIONS, "origin/main", **{**defaults, **kwargs}) + return run(workflows or WORKFLOWS, "origin/main", **{**defaults, **kwargs}) def describe_run(): @@ -246,7 +247,7 @@ def it_counts_any_non_zero_exit_as_a_failure_including_a_negative_one(): def it_skips_an_artifact_gate_without_failing_and_says_so(): lines = [] code = drive( - conventions=ARTIFACT_FIRST, + workflows=ARTIFACT_FIRST, runner=lambda _argv, _cwd: 0, echo=lines.append, ) @@ -260,7 +261,7 @@ def it_skips_an_artifact_gate_without_failing_and_says_so(): def it_keeps_going_past_a_skipped_gate_to_the_pairs_after_it(): lines = [] drive( - conventions=ARTIFACT_FIRST, + workflows=ARTIFACT_FIRST, only=["packaging", "unit-lint"], echo=lines.append, ) @@ -305,11 +306,11 @@ def it_prints_every_pair_without_running_any_when_dry_run(): assert (calls, code) == ([], 0) assert len([line for line in lines if line.startswith("==>")]) == 4 - def it_takes_conventions_and_base_by_keyword(): + def it_takes_workflows_and_base_by_keyword(): # `*` (not `/`) before the injected seams: the two leading parameters must - # stay nameable, since every caller passes the workflow text by name. + # stay nameable, since every caller passes the workflow texts by name. assert run( - conventions=CONVENTIONS, + workflows=WORKFLOWS, base="origin/main", runner=lambda _argv, _cwd: 0, exists=has_manifest, diff --git a/internals/checks/src/checks/preflight/matrix.py b/internals/checks/src/checks/preflight/matrix.py index 258151b07..433bca297 100644 --- a/internals/checks/src/checks/preflight/matrix.py +++ b/internals/checks/src/checks/preflight/matrix.py @@ -1,9 +1,12 @@ -"""Derive the testing-conventions gate matrix from `.github/workflows/conventions.yml` (#781). +"""Derive the testing-conventions gate matrix from the CI workflows (#781). The justfile's `test-conventions` recipe restated the (source, gates) pairs by hand and covered 3 of the 8 scan roots CI declares, so a locally-green run said -nothing about 34 of the ~40 pairs. Reading the workflow makes that drift -impossible: a caller added to `conventions.yml` is a pair `preflight` runs. +nothing about most of the pairs. Reading the workflows makes that drift +impossible: a caller added to any of them is a pair `preflight` runs. + +The callers were one file until #861 split `conventions.yml` into one workflow +per domain, so the matrix now spans every document in `.github/workflows/`. """ from __future__ import annotations @@ -73,22 +76,28 @@ class Root: config: str = "" -def parse_gate_matrix(text: str) -> list[Root]: - """Every `conventions.yml` job that calls the reusable workflow, as a Root.""" +def parse_gate_matrix(texts: list[str]) -> list[Root]: + """Every job across `texts` that calls the reusable workflow, as a Root. + + Most of `.github/workflows/` does not call it at all (the release, docs and + gate workflows), so a document with no `jobs:` mapping -- or none at all -- + contributes nothing rather than raising. + """ roots = [] - for job, spec in yaml.safe_load(text)["jobs"].items(): - if REUSABLE not in spec.get("uses", ""): - continue - inputs = spec.get("with", {}) - roots.append( - Root( - job=job, - source=inputs["source"], - languages=json.loads(inputs.get("languages") or "[]"), - gates=json.loads(inputs["gates"]) if inputs.get("gates") else list(ROOT_GATES), - config=inputs.get("config", ""), + for text in texts: + for job, spec in ((yaml.safe_load(text) or {}).get("jobs") or {}).items(): + if REUSABLE not in spec.get("uses", ""): + continue + inputs = spec.get("with", {}) + roots.append( + Root( + job=job, + source=inputs["source"], + languages=json.loads(inputs.get("languages") or "[]"), + gates=json.loads(inputs["gates"]) if inputs.get("gates") else list(ROOT_GATES), + config=inputs.get("config", ""), + ) ) - ) return roots diff --git a/internals/checks/src/checks/preflight/matrix_test.py b/internals/checks/src/checks/preflight/matrix_test.py index 6f31473b1..6dad311ed 100644 --- a/internals/checks/src/checks/preflight/matrix_test.py +++ b/internals/checks/src/checks/preflight/matrix_test.py @@ -71,10 +71,20 @@ def it_defaults_languages_to_empty_when_a_caller_omits_them(): assert rust.languages == [] def it_ignores_jobs_that_do_not_call_the_reusable_workflow(): - # `unrelated` sits AFTER a caller in its file, so skipping it must not - # stop the walk at the first non-caller. assert [e.job for e in parse_gate_matrix([PYTHON_CI])] == ["python-sdk"] + def it_keeps_walking_a_document_past_a_job_that_is_not_a_caller(): + # `unrelated` sits BETWEEN two callers here, so skipping it must not end + # the walk at the first non-caller. + text = PYTHON_CI + """ rust-python-binding: + uses: x/y/.github/workflows/testing-conventions.yml@v0 + with: + languages: '["rust"]' + source: packages/python +""" + jobs = [e.job for e in parse_gate_matrix([text])] + assert jobs == ["python-sdk", "rust-python-binding"] + def describe_GATES(): def it_names_a_cli_subcommand_for_every_default_gate(): From b7d9bd9a970bc4cb61a0c796f77bfaf4732933b9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 19:09:36 +0000 Subject: [PATCH 3/3] e2e attestation for claude/issue-981-preflight-workflows --- .../claude-issue-981-preflight-workflows.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 internals/checks/e2e-attestations/claude-issue-981-preflight-workflows.json diff --git a/internals/checks/e2e-attestations/claude-issue-981-preflight-workflows.json b/internals/checks/e2e-attestations/claude-issue-981-preflight-workflows.json new file mode 100644 index 000000000..f98aabbd4 --- /dev/null +++ b/internals/checks/e2e-attestations/claude-issue-981-preflight-workflows.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e -x -q", + "ran_at": 1787252976, + "exit_code": 0, + "commit": "f388f328670f9d1634926c00c2319de470f9b1d1", + "branch": "claude/issue-981-preflight-workflows" +}