From cc3add5170af19dddddea7bff8a73bd1d3d78a0e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 18:20:36 +0000 Subject: [PATCH 1/3] ci: assert publish globs and the release precheck agree on what ships RED half of dirsql#944. `dirsql-checks release-globs` holds `putitoutthere.toml` and `release-ci.yml` to one invariant: everything the publish globs match, the PR-time build precheck also builds, and neither reaches a path that never lands in an artifact. It fails on the current config, which is the bug: six packages/roots glob `/**` with no carve-out, so a changelog-fragment-only merge republishes all three packages, and release run 371 already shipped `dirsql-py` and `dirsql-npm` byte-identical off two new `testing-conventions.toml` files. The seventh finding is the same asymmetry in the other file -- `!plugins/dirsql-plugin-embeddings/tests/**` skips the precheck for a path publishing still cascades on. putitoutthere has no exclusion field and its leading-`!` negations do the opposite of what they look like; the check's module docstring records the verification at the pinned `@v0` sha (8f58767) and names the extglob form that does work. Both halves compare by name rather than re-matching paths, so the check carries no second copy of upstream's glob semantics to drift against. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D9pxEJux3YgFtKnWKXZAT6 --- .../workflows/putitoutthere-config-check.yml | 22 ++ internals/checks/src/checks/cli.py | 2 + internals/checks/src/checks/cli_test.py | 4 + .../src/checks/release_globs/__init__.py | 0 .../checks/src/checks/release_globs/cli.py | 28 +++ .../src/checks/release_globs/cli_test.py | 32 +++ .../checks/src/checks/release_globs/decide.py | 153 ++++++++++++++ .../src/checks/release_globs/decide_test.py | 190 ++++++++++++++++++ .../checks/src/checks/release_globs/gate.py | 68 +++++++ .../src/checks/release_globs/gate_test.py | 119 +++++++++++ .../tests/integration/release_globs_test.py | 83 ++++++++ 11 files changed, 701 insertions(+) create mode 100644 internals/checks/src/checks/release_globs/__init__.py create mode 100644 internals/checks/src/checks/release_globs/cli.py create mode 100644 internals/checks/src/checks/release_globs/cli_test.py create mode 100644 internals/checks/src/checks/release_globs/decide.py create mode 100644 internals/checks/src/checks/release_globs/decide_test.py create mode 100644 internals/checks/src/checks/release_globs/gate.py create mode 100644 internals/checks/src/checks/release_globs/gate_test.py create mode 100644 internals/checks/tests/integration/release_globs_test.py diff --git a/.github/workflows/putitoutthere-config-check.yml b/.github/workflows/putitoutthere-config-check.yml index 378a41b98..b6c6e69e9 100644 --- a/.github/workflows/putitoutthere-config-check.yml +++ b/.github/workflows/putitoutthere-config-check.yml @@ -17,6 +17,8 @@ on: branches: [main] paths: - 'putitoutthere.toml' + - '.github/workflows/release-ci.yml' + - 'internals/checks/src/checks/release_globs/**' - '.github/workflows/putitoutthere-config-check.yml' concurrency: @@ -29,3 +31,23 @@ permissions: jobs: check: uses: thekevinscott/putitoutthere/.github/workflows/check.yml@v0 + + # Upstream's `check` validates the config against its own schema; it has no + # opinion about which paths *should* cascade. This one asserts the repo-side + # policy the two files encode together: every path the publish globs match, + # release-ci.yml also builds, and neither reaches a path that never ships + # (fragments, attestation receipts, testing-conventions.toml). Without it a + # markdown-only merge releases all three packages byte-identical (#944). + release-globs: + name: Release globs + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Install uv + uses: astral-sh/setup-uv@v7 + with: + python-version: "3.13" + + - name: Assert publishing and the precheck agree on what ships + run: uv run --project internals/checks dirsql-checks release-globs diff --git a/internals/checks/src/checks/cli.py b/internals/checks/src/checks/cli.py index 842e872b7..6afb41581 100644 --- a/internals/checks/src/checks/cli.py +++ b/internals/checks/src/checks/cli.py @@ -14,6 +14,7 @@ from checks.npm_binary_extension_load.cli import cli as npm_binary_extension_load from checks.preflight.cli import cli as preflight from checks.pytest_gate.cli import cli as pytest_gate +from checks.release_globs.cli import cli as release_globs from checks.wheel_extension_load.cli import cli as wheel_extension_load @@ -28,4 +29,5 @@ def main() -> None: main.add_command(npm_binary_extension_load, name="npm-binary-extension-load") main.add_command(preflight, name="preflight") main.add_command(pytest_gate, name="pytest-gate") +main.add_command(release_globs, name="release-globs") main.add_command(wheel_extension_load, name="wheel-extension-load") diff --git a/internals/checks/src/checks/cli_test.py b/internals/checks/src/checks/cli_test.py index 1c29c5337..24fbb0007 100644 --- a/internals/checks/src/checks/cli_test.py +++ b/internals/checks/src/checks/cli_test.py @@ -32,3 +32,7 @@ def test_declared_deps_is_registered(): def test_preflight_is_registered(): assert "preflight" in main.commands + + +def test_release_globs_is_registered(): + assert "release-globs" in main.commands diff --git a/internals/checks/src/checks/release_globs/__init__.py b/internals/checks/src/checks/release_globs/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/internals/checks/src/checks/release_globs/cli.py b/internals/checks/src/checks/release_globs/cli.py new file mode 100644 index 000000000..baa7b4c21 --- /dev/null +++ b/internals/checks/src/checks/release_globs/cli.py @@ -0,0 +1,28 @@ +"""The release-globs check -- repo-only (#944). + +Backs `dirsql-checks release-globs`: the publish globs and the PR-time build +precheck must agree on which paths reach a released artifact. +""" + +from __future__ import annotations + +import click + +from checks.release_globs.gate import run + + +@click.command() +@click.option( + "--config", + "config_path", + default="putitoutthere.toml", + help="Release config declaring each package's publish globs.", +) +@click.option( + "--workflow", + "workflow_path", + default=".github/workflows/release-ci.yml", + help="Workflow whose PR path filter gates the release build precheck.", +) +def cli(config_path: str, workflow_path: str) -> None: + raise SystemExit(run(config_path, workflow_path)) diff --git a/internals/checks/src/checks/release_globs/cli_test.py b/internals/checks/src/checks/release_globs/cli_test.py new file mode 100644 index 000000000..657e26328 --- /dev/null +++ b/internals/checks/src/checks/release_globs/cli_test.py @@ -0,0 +1,32 @@ +"""Colocated unit tests for the release-globs command (isolation -- no +`CliRunner`). Driven through `.callback`; `run` is mocked at its import site. +""" + +from unittest import mock + +import pytest + +from checks.release_globs.cli import cli + + +def invoke(**kwargs): + with mock.patch("checks.release_globs.cli.run", **kwargs) as run: + with pytest.raises(SystemExit) as exc_info: + cli.callback(config_path="p.toml", workflow_path="w.yml") + return run, exc_info.value.code + + +def test_exits_with_runs_return_code(): + run, code = invoke(return_value=0) + run.assert_called_once_with("p.toml", "w.yml") + assert code == 0 + + +def test_propagates_a_disagreeing_config_as_exit_one(): + assert invoke(return_value=1)[1] == 1 + + +def test_defaults_to_the_repo_root_config_and_the_release_precheck_workflow(): + config_path, workflow_path = cli.params + assert config_path.default == "putitoutthere.toml" + assert workflow_path.default == ".github/workflows/release-ci.yml" diff --git a/internals/checks/src/checks/release_globs/decide.py b/internals/checks/src/checks/release_globs/decide.py new file mode 100644 index 000000000..fb63c0448 --- /dev/null +++ b/internals/checks/src/checks/release_globs/decide.py @@ -0,0 +1,153 @@ +"""Pure decision logic for the release-globs check (#944). + +Merging to `main` is the release trigger, and putitoutthere decides what to +release by intersecting each `[[package]]`'s `globs` with that package's diff +since its own last tag. A bare ``packages//**`` therefore republishes on a +changelog fragment, an e2e attestation receipt, or a `testing-conventions.toml` +edit -- none of which reach the built artifact. Run 371 did exactly that: two +new gate-config files republished `dirsql-py` and `dirsql-npm` byte-identical to +the release before them. + +**putitoutthere has no glob exclusion field and its leading-`!` negations do the +opposite of what they look like.** Verified against the pinned `@v0` sha +8f5876751f679ee3450617957c97495205002499: `globs` is `z.array(z.string())` on a +`.strict()` schema (no `exclude`-shaped key is representable), and both callers +-- `cascade.ts`'s seed pass and `check.ts` -- go through `glob.ts`'s +``matchesAny``, a plain OR that returns on the first hit. Under minimatch +10.2.6 a `!`-prefixed pattern matches every path *outside* its own subtree, so +adding ``!packages/rust/changelog.d/**`` neither subtracts the fragment (the +earlier ``packages/rust/**`` already returned true) nor stays inert -- it makes +`README.md` match, and the package cascades on every commit. + +The form that does work is minimatch's extglob, which excludes *inside* a single +positive pattern: ``packages/rust/!(changelog.d|migrations.d)/**``. It stays a +carve-out rather than an allowlist -- a subdirectory added later is covered +automatically, and only the named names are dropped. +""" + +from __future__ import annotations + +# Non-shipping paths inside a published package's tree: the changelog and +# migration fragment dirs, the e2e attestation receipts, and the package's own +# testing-conventions gate config. +NON_SHIPPING_DIRS = ("changelog.d", "migrations.d", "e2e-attestations") +NON_SHIPPING_FILES = ("testing-conventions.toml",) + +# Top-level directories holding independently published packages. +PUBLISHED_ROOTS = ("packages", "plugins") + +_GLOB_META = "*?[]{}!" + + +def carve_out(root: str) -> list[str]: + """The canonical publish globs for the package subtree at ``root``. + + Two patterns, because a single extglob cannot cover both depths: the first + matches files sitting directly in the package root, the second everything + under its subdirectories. + """ + return [ + f"{root}/!({'|'.join(NON_SHIPPING_FILES)})", + f"{root}/!({'|'.join(NON_SHIPPING_DIRS)})/**", + ] + + +def subtree_root(glob: str): + """``/`` when ``glob`` reaches into a published package tree, + else ``None``.""" + parts = glob.split("/") + if len(parts) < 3 or parts[0] not in PUBLISHED_ROOTS: + return None + return f"{parts[0]}/{parts[1]}" + + +def is_wildcard(glob: str) -> bool: + """True when the entry is a pattern rather than a literal path.""" + return any(character in glob for character in _GLOB_META) + + +def negations(globs) -> list[str]: + """Entries written as minimatch leading-``!`` negations.""" + return [glob for glob in globs if glob.startswith("!")] + + +def subtree_globs(globs, root: str) -> list[str]: + """Sorted wildcard entries that reach into ``root``, negations excluded.""" + return sorted( + glob + for glob in globs + if not glob.startswith("!") and is_wildcard(glob) and subtree_root(glob) == root + ) + + +def subtree_roots(globs) -> list[str]: + """Sorted package subtrees the non-negated wildcard entries reach into.""" + roots = { + root + for glob in globs + if not glob.startswith("!") + and is_wildcard(glob) + and (root := subtree_root(glob)) is not None + } + return sorted(roots) + + +def glob_problems(packages) -> list[str]: + """One message per `[[package]]` glob entry that would republish a no-op.""" + problems = [] + for package in packages: + name = package.get("name", "") + globs = package.get("globs", []) + for glob in negations(globs): + problems.append( + f'{name}: glob "{glob}" is a leading-`!` negation, which ' + f"putitoutthere does not support -- its matcher ORs the globs " + f"together, so the negation subtracts nothing and instead " + f"matches every path outside its own subtree. Write the " + f"exclusion as an extglob inside a positive pattern: " + f'"{carve_out("")[1]}".' + ) + for root in subtree_roots(globs): + found = subtree_globs(globs, root) + expected = carve_out(root) + if found != sorted(expected): + problems.append( + f"{name}: publish globs for {root} are " + f"{', '.join(found)} -- expected {', '.join(expected)}. " + f"A subtree glob that does not carve out " + f"{', '.join((*NON_SHIPPING_DIRS, *NON_SHIPPING_FILES))} " + f"republishes the package for changes that never reach its " + f"artifact. Replace the entries above in putitoutthere.toml." + ) + return problems + + +def unprechecked(exclusions) -> list[str]: + """One message per precheck exclusion the publish globs would still match. + + `release-ci.yml` is the PR-time build precheck; every path it skips must be + a path publishing also skips, or a merge cuts a release the matrix never + built. The comparison is by name rather than by matching, so it needs no + second copy of putitoutthere's glob semantics. + """ + problems = [] + for entry in exclusions: + path = entry.removeprefix("!") + root = subtree_root(path) + if root is None: + continue + rest = path[len(root) + 1 :] + if rest.endswith("/**") and rest.removesuffix("/**") in NON_SHIPPING_DIRS: + continue + if rest in NON_SHIPPING_FILES: + continue + problems.append( + f'release-ci.yml excludes "{entry}", but publishing does not: ' + f"{rest.removesuffix('/**')} is not one of " + f"{', '.join((*NON_SHIPPING_DIRS, *NON_SHIPPING_FILES))}, so " + f"putitoutthere still cascades {root} on a change there and a merge " + f"releases what the precheck never built. Either drop the exclusion " + f"from release-ci.yml or add the name to NON_SHIPPING_DIRS / " + f"NON_SHIPPING_FILES and re-run this check." + ) + return problems diff --git a/internals/checks/src/checks/release_globs/decide_test.py b/internals/checks/src/checks/release_globs/decide_test.py new file mode 100644 index 000000000..927712a6b --- /dev/null +++ b/internals/checks/src/checks/release_globs/decide_test.py @@ -0,0 +1,190 @@ +"""Colocated unit tests for the release-globs decision logic (#944).""" + +from checks.release_globs.decide import ( + NON_SHIPPING_DIRS, + NON_SHIPPING_FILES, + carve_out, + glob_problems, + is_wildcard, + negations, + subtree_globs, + subtree_root, + subtree_roots, + unprechecked, +) + +RUST = carve_out("packages/rust") + + +def describe_carve_out(): + def it_pairs_a_root_file_pattern_with_a_subdirectory_pattern(): + assert carve_out("packages/rust") == [ + "packages/rust/!(testing-conventions.toml)", + "packages/rust/!(changelog.d|migrations.d|e2e-attestations)/**", + ] + + def it_names_every_non_shipping_entry(): + first, second = carve_out("p/q") + assert all(name in second for name in NON_SHIPPING_DIRS) + assert all(name in first for name in NON_SHIPPING_FILES) + + +def describe_subtree_root(): + def it_reads_the_package_root_off_a_recursive_glob(): + assert subtree_root("packages/rust/**") == "packages/rust" + + def it_reads_the_same_root_off_an_extglob_carve_out(): + assert subtree_root(RUST[1]) == "packages/rust" + + def it_accepts_the_plugins_root(): + assert subtree_root("plugins/dirsql-plugin-embeddings/**") == ( + "plugins/dirsql-plugin-embeddings" + ) + + def it_ignores_a_repo_root_file(): + assert subtree_root("Cargo.toml") is None + + def it_ignores_a_two_segment_path_that_names_no_content(): + assert subtree_root("packages/rust") is None + + def it_ignores_a_directory_that_holds_no_published_package(): + assert subtree_root("internals/checks/src/**") is None + + +def describe_is_wildcard(): + def it_is_true_for_a_pattern(): + assert is_wildcard("packages/rust/**") is True + + def it_is_true_for_an_extglob(): + assert is_wildcard(RUST[0]) is True + + def it_is_false_for_a_literal_path(): + assert is_wildcard("Cargo.toml") is False + + +def describe_negations(): + def it_collects_leading_bang_entries(): + assert negations(["packages/rust/**", "!packages/rust/changelog.d/**"]) == [ + "!packages/rust/changelog.d/**" + ] + + def it_is_empty_when_every_entry_is_positive(): + assert negations(["packages/rust/**"]) == [] + + +def describe_subtree_globs(): + def it_returns_the_entries_reaching_into_one_root_sorted(): + assert subtree_globs([*reversed(RUST), "Cargo.toml"], "packages/rust") == sorted(RUST) + + def it_omits_entries_reaching_into_a_different_root(): + # Both a root that sorts before `packages/rust` and one that sorts + # after: the root comparison is an identity, not an ordering. + assert subtree_globs(["packages/python/**", "packages/ts/**"], "packages/rust") == [] + + def it_omits_a_literal_path_inside_the_root(): + assert subtree_globs(["packages/rust/Cargo.toml"], "packages/rust") == [] + + def it_omits_a_negation_so_it_is_reported_once_as_a_negation(): + assert subtree_globs(["!packages/rust/changelog.d/**"], "packages/rust") == [] + + +def describe_subtree_roots(): + def it_dedupes_and_sorts_the_roots_the_globs_reach_into(): + assert subtree_roots(["packages/ts/**", *RUST, "Cargo.lock"]) == [ + "packages/rust", + "packages/ts", + ] + + def it_is_empty_for_repo_root_globs_only(): + assert subtree_roots(["Cargo.toml", "Cargo.lock"]) == [] + + +def describe_glob_problems(): + def it_accepts_a_package_whose_globs_carve_out_the_non_shipping_paths(): + assert glob_problems([{"name": "dirsql-rust", "globs": [*RUST, "Cargo.toml"]}]) == [] + + def it_rejects_a_bare_recursive_subtree_glob(): + problems = glob_problems([{"name": "dirsql-rust", "globs": ["packages/rust/**"]}]) + assert len(problems) == 1 + assert "publish globs for packages/rust are packages/rust/**" in problems[0] + assert RUST[1] in problems[0] + + def it_rejects_a_carve_out_missing_the_root_file_pattern(): + problems = glob_problems([{"name": "dirsql-rust", "globs": [RUST[1]]}]) + assert len(problems) == 1 + assert RUST[0] in problems[0] + + def it_rejects_a_leading_bang_negation_and_explains_the_extglob_form(): + problems = glob_problems( + [{"name": "dirsql-rust", "globs": [*RUST, "!packages/rust/changelog.d/**"]}] + ) + assert len(problems) == 1 + assert "leading-`!` negation" in problems[0] + assert "matches every path outside its own subtree" in problems[0] + # The suggestion has to be the subdirectory pattern -- the fragment dirs + # the author was reaching for live there, not in the root-file pattern. + assert '"/!(changelog.d|migrations.d|e2e-attestations)/**"' in problems[0] + + def it_reports_every_root_a_package_globs(): + problems = glob_problems( + [{"name": "dirsql-py", "globs": ["packages/python/**", "packages/rust/**"]}] + ) + assert len(problems) == 2 + + def it_reports_every_package(): + problems = glob_problems( + [{"name": "a", "globs": ["packages/rust/**"]}, {"name": "b", "globs": ["packages/ts/**"]}] + ) + assert len(problems) == 2 + + def it_names_an_unnamed_package_rather_than_crashing(): + assert glob_problems([{"globs": ["packages/rust/**"]}])[0].startswith(":") + + def it_accepts_a_package_declaring_no_globs(): + assert glob_problems([{"name": "a"}]) == [] + + def it_accepts_an_empty_package_list(): + assert glob_problems([]) == [] + + +def describe_unprechecked(): + def it_accepts_an_exclusion_naming_a_non_shipping_directory(): + assert unprechecked(["!packages/rust/changelog.d/**"]) == [] + + def it_accepts_an_exclusion_naming_a_non_shipping_root_file(): + assert unprechecked(["!packages/python/testing-conventions.toml"]) == [] + + def it_ignores_an_exclusion_outside_every_published_package(): + assert unprechecked(["!docs/**"]) == [] + + def it_rejects_an_exclusion_for_a_directory_that_still_publishes(): + problems = unprechecked(["!plugins/dirsql-plugin-embeddings/tests/**"]) + assert len(problems) == 1 + assert "tests is not one of" in problems[0] + assert "plugins/dirsql-plugin-embeddings" in problems[0] + + def it_rejects_an_exclusion_for_a_root_file_that_still_publishes(): + assert len(unprechecked(["!packages/ts/biome.json"])) == 1 + + def it_rejects_a_non_shipping_name_used_at_the_wrong_depth(): + # `changelog.d` without the `/**` tail is a file exclusion, and no file + # by that name is carved out of the publish globs. + assert len(unprechecked(["!packages/rust/changelog.d"])) == 1 + + def it_reports_every_offending_exclusion(): + assert len(unprechecked(["!packages/ts/tools/**", "!packages/ts/biome.json"])) == 2 + + def it_keeps_scanning_past_an_exclusion_outside_every_package(): + assert len(unprechecked(["!docs/**", "!packages/ts/biome.json"])) == 1 + + def it_keeps_scanning_past_an_accepted_directory_exclusion(): + assert ( + len(unprechecked(["!packages/rust/changelog.d/**", "!packages/ts/biome.json"])) == 1 + ) + + def it_keeps_scanning_past_an_accepted_root_file_exclusion(): + exclusions = ["!packages/python/testing-conventions.toml", "!packages/ts/biome.json"] + assert len(unprechecked(exclusions)) == 1 + + def it_accepts_an_empty_exclusion_list(): + assert unprechecked([]) == [] diff --git a/internals/checks/src/checks/release_globs/gate.py b/internals/checks/src/checks/release_globs/gate.py new file mode 100644 index 000000000..5a9e20c2f --- /dev/null +++ b/internals/checks/src/checks/release_globs/gate.py @@ -0,0 +1,68 @@ +"""Orchestration for the release-globs check (#944). + +Reads the two files that between them decide what a merge to `main` publishes -- +`putitoutthere.toml`'s per-package `globs` and `release-ci.yml`'s PR path filter +-- and holds them to one invariant: **everything the publish globs match, the +precheck also builds, and neither reaches a path that never ships**. + +Both halves are compared by name, never by re-matching paths, so this check +carries no second copy of putitoutthere's glob semantics to drift against. +""" + +from __future__ import annotations + +import tomllib +from collections.abc import Callable + +import yaml + +from checks.release_globs.decide import glob_problems, unprechecked + + +def read_config(path: str) -> dict: + with open(path, "rb") as handle: + return tomllib.load(handle) + + +def read_workflow(path: str) -> dict: + with open(path, encoding="utf-8") as handle: + return yaml.safe_load(handle) + + +def pull_request_paths(workflow: dict) -> list[str]: + """The `on.pull_request.paths` filter, or empty when the workflow has none. + + YAML 1.1 resolves a bare ``on`` key to the boolean ``True``, which is how + PyYAML hands back every GitHub workflow; the string key is accepted too so a + quoted ``"on":`` reads the same. + """ + triggers = workflow.get(True, workflow.get("on")) or {} + return (triggers.get("pull_request") or {}).get("paths") or [] + + +def exclusions(paths) -> list[str]: + return [path for path in paths if path.startswith("!")] + + +def run( + config_path: str, + workflow_path: str, + *, + config: Callable[[str], dict] = read_config, + workflow: Callable[[str], dict] = read_workflow, + echo: Callable[[str], None] = print, +) -> int: + problems = glob_problems(config(config_path).get("package", [])) + problems += unprechecked(exclusions(pull_request_paths(workflow(workflow_path)))) + for problem in problems: + echo(f"::error::{problem}") + if problems: + echo( + f"release-globs: {len(problems)} problem(s). See " + f"internals/checks/src/checks/release_globs/decide.py for why " + f"leading-`!` negations do not work and what the extglob carve-out " + f"form is." + ) + return 1 + echo(f"ok release-globs: {config_path} and {workflow_path} agree on what ships.") + return 0 diff --git a/internals/checks/src/checks/release_globs/gate_test.py b/internals/checks/src/checks/release_globs/gate_test.py new file mode 100644 index 000000000..34ef24dd2 --- /dev/null +++ b/internals/checks/src/checks/release_globs/gate_test.py @@ -0,0 +1,119 @@ +"""Colocated unit tests for the release-globs gate (#944). + +Isolation: the two readers are injected, so nothing here touches the repo's real +`putitoutthere.toml` or `release-ci.yml`. The readers themselves are exercised +against scratch files. +""" + +from checks.release_globs.gate import ( + exclusions, + pull_request_paths, + read_config, + read_workflow, + run, +) + +CARVED = [ + "packages/rust/!(testing-conventions.toml)", + "packages/rust/!(changelog.d|migrations.d|e2e-attestations)/**", +] +CLEAN = {"package": [{"name": "dirsql-rust", "globs": CARVED}]} +DIRTY = {"package": [{"name": "dirsql-rust", "globs": ["packages/rust/**"]}]} +NO_PATHS: dict = {} + + +def describe_read_config(): + def it_parses_a_toml_release_config(tmp_path): + path = tmp_path / "putitoutthere.toml" + path.write_text('[[package]]\nname = "x"\nglobs = ["a/**"]\n') + assert read_config(str(path)) == {"package": [{"name": "x", "globs": ["a/**"]}]} + + +def describe_read_workflow(): + def it_parses_a_workflow_whose_on_key_yaml_resolves_to_a_boolean(tmp_path): + path = tmp_path / "w.yml" + path.write_text("on:\n pull_request:\n paths:\n - 'a/**'\n") + assert read_workflow(str(path))[True] == {"pull_request": {"paths": ["a/**"]}} + + +def describe_pull_request_paths(): + def it_reads_the_filter_off_the_boolean_on_key(): + assert pull_request_paths({True: {"pull_request": {"paths": ["a/**"]}}}) == ["a/**"] + + def it_reads_the_filter_off_a_quoted_string_on_key(): + assert pull_request_paths({"on": {"pull_request": {"paths": ["a/**"]}}}) == ["a/**"] + + def it_is_empty_when_the_workflow_declares_no_triggers(): + assert pull_request_paths({}) == [] + + def it_is_empty_when_the_on_block_is_null(): + assert pull_request_paths({True: None}) == [] + + def it_is_empty_when_the_workflow_has_no_pull_request_trigger(): + assert pull_request_paths({True: {"push": {"branches": ["main"]}}}) == [] + + def it_is_empty_when_the_pull_request_trigger_is_null(): + assert pull_request_paths({True: {"pull_request": None}}) == [] + + def it_is_empty_when_the_pull_request_trigger_filters_no_paths(): + assert pull_request_paths({True: {"pull_request": {"branches": ["main"]}}}) == [] + + +def describe_exclusions(): + def it_keeps_only_the_negated_entries(): + assert exclusions(["packages/rust/**", "!packages/rust/changelog.d/**"]) == [ + "!packages/rust/changelog.d/**" + ] + + +def drive(config, workflow, lines): + return run( + "putitoutthere.toml", + ".github/workflows/release-ci.yml", + config=lambda _p: config, + workflow=lambda _p: workflow, + echo=lines.append, + ) + + +def describe_run(): + def it_returns_zero_and_names_both_files_when_they_agree(): + lines: list[str] = [] + assert drive(CLEAN, NO_PATHS, lines) == 0 + assert lines == [ + "ok release-globs: putitoutthere.toml and " + ".github/workflows/release-ci.yml agree on what ships." + ] + + def it_returns_one_and_annotates_each_offending_glob(): + lines: list[str] = [] + assert drive(DIRTY, NO_PATHS, lines) == 1 + assert lines[0].startswith("::error::dirsql-rust: publish globs for packages/rust") + assert lines[1] == ( + "release-globs: 1 problem(s). See " + "internals/checks/src/checks/release_globs/decide.py for why " + "leading-`!` negations do not work and what the extglob carve-out form is." + ) + + def it_also_annotates_a_precheck_exclusion_publishing_does_not_share(): + lines: list[str] = [] + workflow = {True: {"pull_request": {"paths": ["packages/rust/**", "!packages/rust/tests/**"]}}} + assert drive(CLEAN, workflow, lines) == 1 + assert "release-ci.yml excludes" in lines[0] + + def it_accepts_a_config_declaring_no_packages(): + lines: list[str] = [] + assert drive({}, NO_PATHS, lines) == 0 + + def it_takes_both_paths_by_keyword(): + # `*` (not `/`) before the injected seams keeps both nameable. + assert ( + run( + config_path="c.toml", + workflow_path="w.yml", + config=lambda _p: {}, + workflow=lambda _p: {}, + echo=lambda _line: None, + ) + == 0 + ) diff --git a/internals/checks/tests/integration/release_globs_test.py b/internals/checks/tests/integration/release_globs_test.py new file mode 100644 index 000000000..4d9e5414c --- /dev/null +++ b/internals/checks/tests/integration/release_globs_test.py @@ -0,0 +1,83 @@ +"""Integration tests for the release-globs check against real files. + +Exercises `gate.run` with its default collaborators (a real `tomllib` parse of a +real `putitoutthere.toml`, a real PyYAML parse of a real workflow) rather than +the packaged `dirsql-checks` CLI. + +The reproduction is #944 exactly: `packages/rust/**` in the publish globs while +`release-ci.yml` excludes `packages/rust/changelog.d/**`, so a fragment-only PR +merges without ever running the build matrix and then republishes all three +packages. Release run 371 shipped that way off two `testing-conventions.toml` +files, which is the same shape one directory shallower. +""" + +from __future__ import annotations + +import pytest + +from checks.release_globs.decide import carve_out +from checks.release_globs.gate import run + +WORKFLOW = """\ +name: Release CI +on: + pull_request: + branches: [main] + paths: +{paths} +jobs: + precheck: + uses: thekevinscott/putitoutthere/.github/workflows/build.yml@v0 +""" + +EXCLUSIONS = [ + "packages/rust/**", + "!packages/rust/changelog.d/**", + "!packages/rust/migrations.d/**", + "!packages/rust/e2e-attestations/**", + "!packages/rust/testing-conventions.toml", + "Cargo.toml", +] + + +def write(tmp_path, globs, paths=EXCLUSIONS): + config = tmp_path / "putitoutthere.toml" + entries = "\n".join(f' "{glob}",' for glob in globs) + config.write_text( + f'[putitoutthere]\nversion = 1\n\n[[package]]\nname = "dirsql-rust"\n' + f'kind = "crates"\npath = "packages/rust"\nglobs = [\n{entries}\n]\n' + ) + workflow = tmp_path / "release-ci.yml" + workflow.write_text(WORKFLOW.format(paths="\n".join(f" - '{p}'" for p in paths))) + return str(config), str(workflow) + + +def describe_run_against_real_config_files(): + def it_fails_on_the_944_mismatch(tmp_path, capsys): + code = run(*write(tmp_path, ["packages/rust/**", "Cargo.toml", "Cargo.lock"])) + assert code == 1 + out = capsys.readouterr().out + assert "::error::dirsql-rust: publish globs for packages/rust" in out + assert "packages/rust/!(changelog.d|migrations.d|e2e-attestations)/**" in out + + def it_passes_once_the_publish_globs_carve_the_same_paths_out(tmp_path, capsys): + globs = [*carve_out("packages/rust"), "Cargo.toml", "Cargo.lock"] + assert run(*write(tmp_path, globs)) == 0 + assert "ok release-globs" in capsys.readouterr().out + + def it_still_fails_when_the_precheck_skips_a_path_publishing_keeps(tmp_path, capsys): + # The asymmetry #944 is about, in its general form: any path the build + # precheck declines to build but the release still ships. + globs = [*carve_out("packages/rust"), "Cargo.toml"] + paths = [*EXCLUSIONS, "!packages/rust/benches/**"] + assert run(*write(tmp_path, globs, paths)) == 1 + assert "release-ci.yml excludes" in capsys.readouterr().out + + @pytest.mark.parametrize("negation", ["!packages/rust/changelog.d/**", "!packages/rust/**"]) + def it_rejects_a_leading_bang_negation_in_the_publish_globs(tmp_path, capsys, negation): + # Verified against putitoutthere @v0 (8f58767): `matchesAny` ORs the + # globs, so this neither subtracts nor stays inert -- under minimatch it + # matches every path outside the named subtree. + globs = [*carve_out("packages/rust"), negation] + assert run(*write(tmp_path, globs)) == 1 + assert "leading-`!` negation" in capsys.readouterr().out From bfa0d84d566244942fb9f11578abe10d6024508e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 18:29:54 +0000 Subject: [PATCH 2/3] fix: carve non-shipping paths out of the publish globs GREEN half of dirsql#944. Each package subtree is now globbed as a pair of minimatch extglob patterns instead of a bare `/**`, so a merge that only touches a changelog fragment, a migration fragment, an e2e attestation receipt or a package's `testing-conventions.toml` cascades nothing. Two patterns per root because one extglob cannot span both depths -- the first covers files directly in the package root, the second everything under its subdirectories. It stays a carve-out rather than an allowlist: a subdirectory or root file added later is still matched, only the named names drop out. Replaying putitoutthere @v0's `matchesAny` over the 559 tracked files, the change removes 59/92/92/15 paths per package and every one of them is a fragment, a receipt or a gate config; nothing new matches, and synthetic future paths (`packages/rust/brand-new-dir/deep/x.rs`, `packages/ts/new-root-file.json`) still cascade. `release-ci.yml` gains the matching `testing-conventions.toml` exclusions so the precheck stops building for a gate-config edit. It also LOSES `!plugins/dirsql-plugin-embeddings/tests/**`: hatchling excludes only `**/*_test.py`, so `tests/conftest.py` and the package inits do land in the plugin's sdist, and skipping the precheck there was the same publish-without- building asymmetry the rest of this change removes. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01D9pxEJux3YgFtKnWKXZAT6 --- .github/workflows/release-ci.yml | 14 ++++++++++-- putitoutthere.toml | 38 +++++++++++++++++++++++++++----- 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/.github/workflows/release-ci.yml b/.github/workflows/release-ci.yml index 041fe1802..ef79648e4 100644 --- a/.github/workflows/release-ci.yml +++ b/.github/workflows/release-ci.yml @@ -20,6 +20,10 @@ name: Release CI # plans only dirsql-py). putitoutthere's build.yml exposes no package filter, # and the release cascade (rust -> py -> npm) is cross-package by design, so # this stays one workflow rather than splitting per package. +# +# The exclusions must stay a SUBSET of what putitoutthere.toml's globs carve +# out, or a merge releases what this never built (#944). `dirsql-checks +# release-globs` fails the PR when they diverge. on: pull_request: branches: [main] @@ -27,23 +31,29 @@ on: - 'packages/rust/**' - '!packages/rust/changelog.d/**' - '!packages/rust/migrations.d/**' + - '!packages/rust/e2e-attestations/**' + - '!packages/rust/testing-conventions.toml' - 'packages/python/**' - '!packages/python/changelog.d/**' - '!packages/python/migrations.d/**' - '!packages/python/e2e-attestations/**' + - '!packages/python/testing-conventions.toml' - 'packages/ts/**' - '!packages/ts/changelog.d/**' - '!packages/ts/migrations.d/**' - '!packages/ts/e2e-attestations/**' + - '!packages/ts/testing-conventions.toml' # putitoutthere's plugin globs are plugins/dirsql-plugin-embeddings/**; # README stays in scope here (hatchling embeds it as the PyPI # long-description, so it is a build input), fragments and receipts - # do not ship. + # do not ship. `tests/` is NOT excluded: hatchling drops only + # `**/*_test.py`, so `tests/conftest.py` and the package inits land in + # the sdist and a change there is a real build input. - 'plugins/dirsql-plugin-embeddings/**' - '!plugins/dirsql-plugin-embeddings/changelog.d/**' - '!plugins/dirsql-plugin-embeddings/migrations.d/**' - '!plugins/dirsql-plugin-embeddings/e2e-attestations/**' - - '!plugins/dirsql-plugin-embeddings/tests/**' + - '!plugins/dirsql-plugin-embeddings/testing-conventions.toml' - 'Cargo.toml' - 'Cargo.lock' - 'putitoutthere.toml' diff --git a/putitoutthere.toml b/putitoutthere.toml index d34f47c19..a10167213 100644 --- a/putitoutthere.toml +++ b/putitoutthere.toml @@ -12,6 +12,26 @@ # The fourth, `dirsql-plugin-embeddings`, is an independently-versioned # pure-Python plugin published as its own PyPI project. It is deliberately # outside the cascade -- it releases only when its own path changes. +# +# Every package subtree is globbed as a PAIR of extglob patterns rather than a +# bare `/**`, because merging to `main` publishes and a bare glob cascades +# on files that never reach the artifact -- fragment dirs, e2e attestation +# receipts, the package's testing-conventions gate config. Run 371 republished +# `dirsql-py` and `dirsql-npm` byte-identical off two new gate configs (#944). +# +# The extglob is load-bearing and a leading `!` is NOT a substitute. Verified at +# the pinned `@v0` sha 8f5876751f679ee3450617957c97495205002499: `globs` is a +# plain `string[]` on a `.strict()` schema (no exclusion key is representable), +# and both consumers go through `glob.ts`'s `matchesAny`, an OR that returns on +# the first hit. Under minimatch a `!`-prefixed pattern matches every path +# OUTSIDE its subtree, so `!packages/rust/changelog.d/**` would not subtract the +# fragment -- it would make `README.md` match and cascade on every commit. +# `dirsql-checks release-globs` fails the PR if these drift back. +# +# Two patterns per root because one extglob cannot span both depths: the first +# covers files sitting directly in the package root, the second everything under +# its subdirectories. It stays a carve-out, not an allowlist -- a subdirectory or +# root file added later is matched automatically. [putitoutthere] version = 1 @@ -25,7 +45,8 @@ crate = "dirsql" kind = "crates" path = "packages/rust" globs = [ - "packages/rust/**", + "packages/rust/!(testing-conventions.toml)", + "packages/rust/!(changelog.d|migrations.d|e2e-attestations)/**", "Cargo.toml", "Cargo.lock", ] @@ -48,8 +69,10 @@ build = "maturin" manylinux = "2_28" path = "packages/python" globs = [ - "packages/python/**", - "packages/rust/**", + "packages/python/!(testing-conventions.toml)", + "packages/python/!(changelog.d|migrations.d|e2e-attestations)/**", + "packages/rust/!(testing-conventions.toml)", + "packages/rust/!(changelog.d|migrations.d|e2e-attestations)/**", "Cargo.toml", "Cargo.lock", ] @@ -91,8 +114,10 @@ npm = "dirsql" kind = "npm" path = "packages/ts" globs = [ - "packages/ts/**", - "packages/rust/**", + "packages/ts/!(testing-conventions.toml)", + "packages/ts/!(changelog.d|migrations.d|e2e-attestations)/**", + "packages/rust/!(testing-conventions.toml)", + "packages/rust/!(changelog.d|migrations.d|e2e-attestations)/**", "Cargo.toml", "Cargo.lock", ] @@ -131,6 +156,7 @@ kind = "pypi" build = "hatch" path = "plugins/dirsql-plugin-embeddings" globs = [ - "plugins/dirsql-plugin-embeddings/**", + "plugins/dirsql-plugin-embeddings/!(testing-conventions.toml)", + "plugins/dirsql-plugin-embeddings/!(changelog.d|migrations.d|e2e-attestations)/**", ] first_version = "0.1.0" From f273449a3e64d2bfcf994d032497ceb4c0925f31 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 20 Aug 2026 18:30:23 +0000 Subject: [PATCH 3/3] e2e attestation for claude/tackle-944-iegeyj --- .../checks/e2e-attestations/claude-tackle-944-iegeyj.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 internals/checks/e2e-attestations/claude-tackle-944-iegeyj.json diff --git a/internals/checks/e2e-attestations/claude-tackle-944-iegeyj.json b/internals/checks/e2e-attestations/claude-tackle-944-iegeyj.json new file mode 100644 index 000000000..eeaf474a4 --- /dev/null +++ b/internals/checks/e2e-attestations/claude-tackle-944-iegeyj.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e -q", + "ran_at": 1787250623, + "exit_code": 0, + "commit": "bfa0d84d566244942fb9f11578abe10d6024508e", + "branch": "claude/tackle-944-iegeyj" +}