diff --git a/README.md b/README.md index 20bb4a3e..23b0c6a5 100644 --- a/README.md +++ b/README.md @@ -973,20 +973,20 @@ python3 scripts/bump-leg.py --root --leg spec \ It fetches the commit into the leg, checks the leg out AT it, recomputes the `sorted-ls-tree-r-v1` digest from the leg's own objects, rewrites `commit:` and `digests.tree_sha256` in `contracts/-pin.yaml`, rewrites every -`.github/workflows/*.yml` `@` naming THAT leg's repository — and none -naming anything else, `actions/checkout@` and the sibling leg included — -then runs the project's own `validate-pins.py` and `validate-manifest.py` and -commits ONCE with explicit pathspecs. - -It REFUSES a `--to` that is not exactly 40 hex, a commit no branch of the -leg's remote contains (including one committed inside the mount and never -pushed — a pin the rest of the world cannot fetch is a root it cannot -bootstrap), a dirty root, a root standing on its own tracking branch (the next -step is a pull request, never a push to the default branch), and a validator -that goes red — rolling the pin, the workflow files, the leg's checkout and -the index back first, so a refusal leaves the tree exactly as it was found. -The leg is left DETACHED at the new commit; `make bootstrap` re-places it on -its tracking branch AT the new pin. +TRACKED `.github/workflows/*.yml` `@` naming THAT leg's repository — and +none naming anything else, `actions/checkout@` and the sibling leg +included — then runs the project's own `validate-pins.py` and +`validate-manifest.py` and commits ONCE with explicit pathspecs. + +It REFUSES a `--to` that is not exactly 40 hex, a commit no branch of the leg's +remote contains (including one committed inside the mount and never pushed — a +pin the rest of the world cannot fetch is a root it cannot bootstrap), a dirty +root, an untracked or ignored workflow file it would rewrite, a root standing on +its own tracking branch (the next step is a pull request, never a push to the +default branch), and a validator that goes red — rolling the pin, the workflow +files, the leg's checkout and the index back first, so a refusal leaves the tree +exactly as it was found. The leg is left DETACHED at the new commit; `make +bootstrap` re-places it on its tracking branch AT the new pin. ## Bootstrap is COPIED into the project, not fetched diff --git a/docs/handbook.html b/docs/handbook.html index e5a3782a..66854b56 100644 --- a/docs/handbook.html +++ b/docs/handbook.html @@ -1304,17 +1304,18 @@

Advancing a leg

--to <40 hex commit> # --dry-run prints the move and writes nothing

It fetches the commit into the leg, checks the leg out AT it, recomputes the sorted-ls-tree-r-v1 digest from the leg’s own objects, rewrites - commit: and digests.tree_sha256, rewrites every workflow + commit: and digests.tree_sha256, rewrites every TRACKED workflow @<sha> naming THAT leg’s repository — and none naming anything else, actions/checkout@<sha> and the sibling leg included — then runs the project’s own validate-pins.py and validate-manifest.py and commits ONCE with explicit pathspecs.

Refuses a --to that is not exactly 40 hex; a commit no branch of the leg’s remote contains (a pin the rest of the world cannot fetch - is a root it cannot bootstrap); a dirty root; a root standing on its own tracking branch (the - next step is a pull request, never a push to the default branch); and a validator that goes - red — rolling the pin, the workflow files, the leg’s checkout and the index back - first, so a refusal leaves the tree exactly as it was found.

+ is a root it cannot bootstrap); a dirty root; an untracked or ignored workflow file it would + rewrite; a root standing on its own tracking branch (the next step is a pull request, never a + push to the default branch); and a validator that goes red — rolling the pin, the + workflow files, the leg’s checkout and the index back first, so a refusal leaves the + tree exactly as it was found.

The leg is left DETACHED at the new commit; make bootstrap re-places it on its tracking branch AT the new pin. Editing the pin file by hand to make the validator agree is what the validator exists to refuse.

@@ -1780,10 +1781,11 @@

For AI assistants

single-repository.yaml, the doctor’s advisory row and the assistant’s “what you do tell them” added (#163); a family.yaml beside project.yaml as a manifest kinds - finding added (#211) — a page cannot name the commit that contains it, and the blob - below is the fact a test checks. README.md blob 20bb4a3e884c3232380407bda345af6e29d428c6. - Every command, flag, exit code and file name on this page is quoted from that README and - from AGENTS.md.

+ finding added (#211); bump-leg.py rewriting tracked workflow files only, and + refusing an untracked or ignored one it would rewrite, added (#207) — a page cannot + name the commit that contains it, and the blob below is the fact a test checks. + README.md blob 23b0c6a55802788ce0113c00ea476714c4f38252. Every command, flag, exit code + and file name on this page is quoted from that README and from AGENTS.md.

diff --git a/scripts/bump-leg.py b/scripts/bump-leg.py index 2be25961..d0b6f518 100755 --- a/scripts/bump-leg.py +++ b/scripts/bump-leg.py @@ -14,7 +14,7 @@ 1. the GITLINK the assembly root records at the leg's path, 2. `commit:` and `digests.tree_sha256` in `contracts/-pin.yaml`, - 3. every `.github/workflows/*.yml` reference of the form + 3. every TRACKED `.github/workflows/*.yml` reference of the form `//...@<40 hex>` naming THAT leg's repository. `family.py bump` already did exactly this for a family's MEMBER pins. Nothing @@ -33,6 +33,15 @@ * a dirty root. The commit is made with explicit pathspecs, so unrelated edits would not be swept in — but a lockstep commit that cannot be read as exactly one change is the thing the rule exists to keep readable. + * a workflow file git does not TRACK — untracked, or ignored — that names + the leg at any other commit. The lockstep commit's `git add` names every + file this rewrites: an untracked one would be added to the commit whole, + as a new file nobody committed, and an ignored one would stop that + `git add` after the rewrite, in git's own words. Leaving either alone + would turn the bump red, because `validate-pins.py` reads every file on + disk and would still see its old `@`. Refused by name, before + anything is written, under `--dry-run` as well. One that names no leg, + or this leg already at `--to`, is left alone. * the root standing on its own tracking branch. The next step this prints would then be a push to the default branch, and these organisations are pull-request only. @@ -228,9 +237,24 @@ def rewrite_pin(text: str, commit: str, digest: str) -> str: return "\n".join(out) + "\n" +def _tracked_workflows(root: Path, workflows: Path) -> set[str]: + """The paths git TRACKS under `workflows`, as POSIX paths relative to + `root` — the spelling `plan_workflows` and the commit's pathspecs use. + + The index, by `git ls-files`: `_ensure_root_clean` has already refused a + root whose tracked files differ from it, so it is also what HEAD holds. + NUL-separated, so a name git would otherwise quote comes back as the + bytes it is. + """ + raw = git_out(["ls-files", "-z", "--", + workflows.relative_to(root).as_posix()], + cwd=root, binary=True) + return {os.fsdecode(name) for name in raw.split(b"\0") if name} + + def plan_workflows(root: Path, repository: str, commit: str) -> list[tuple[Path, str, int]]: - """Every workflow file whose `@` refs to THIS leg have to move. + """Every TRACKED workflow file whose `@` refs to THIS leg have to move. Returns `(path, rewritten text, references moved)`, one row per file that actually changes. A reference to any OTHER repository is left exactly as @@ -240,11 +264,29 @@ def plan_workflows(root: Path, repository: str, `.yaml` is read as well as `.yml` because `validate-pins.py` reads both, and a file this rewrote nothing in is a file the gate still calls red. + + TRACKED FILES ONLY (#207). The directory is walked on disk, because that + is the set `validate-pins.py` reads, but a row is made for a file only + when git tracks it: the lockstep commit's `git add` names every file it + rewrote, so a draft that was never committed would be added to "Bump spec + leg" whole, as a new file, while `_ensure_root_clean` — which skips + untracked files on exactly the ground that they cannot reach this commit + — said nothing. (An IGNORED one is no better, only later: `git add` + refuses it after the rewrite, with its own hint and no word about this + tool.) An untracked or ignored file that has a reference to move is + REFUSED by name rather than skipped: skipped, `validate-pins.py` would + still read its old `@` off the disk and turn the bump red after the + rewrite, with a finding that never says the file is not in the + repository. One with nothing to move — no reference to this leg, or this + leg already at `commit` — is not this tool's business and is left alone, + unnamed. """ plans: list[tuple[Path, str, int]] = [] workflows = root / ".github" / "workflows" if not workflows.is_dir(): return plans + tracked = _tracked_workflows(root, workflows) + untracked: list[tuple[str, int]] = [] for path in sorted(workflows.iterdir()): if path.suffix not in (".yml", ".yaml") or not path.is_file(): continue @@ -262,8 +304,28 @@ def swap(match: re.Match) -> str: text = path.read_text(encoding="utf-8") rewritten = WORKFLOW_REF_RE.sub(swap, text) - if moved: + if not moved: + continue + relative = path.relative_to(root).as_posix() + if relative in tracked: plans.append((path, rewritten, moved)) + else: + untracked.append((relative, moved)) + if untracked: + raise Refusal( + "bump-leg-workflow-untracked", + f"{len(untracked)} workflow file(s) that git does not track " + f"(untracked or ignored) name {repository} at a commit other " + f"than {commit[:12]}:\n" + + "\n".join(f" {relative} {count} reference(s)" + for relative, count in untracked), + "Remediation: the lockstep commit adds no file the repository " + "does not already hold, and `scripts/validate-pins.py` reads " + "every workflow file on disk, so one left where it is would turn " + "the bump red over its old `@`. Either commit it first — it " + "is then tracked and moves with the bump, and the dry run lists " + "it — or move it out of .github/workflows/ until the bump has " + "landed, then re-run.") return plans @@ -484,10 +546,12 @@ def _ensure_root_clean(root: Path) -> None: # THE ROOT MUST BE CLEAN. Tracked changes only: an untracked scratch file # cannot reach a commit made with explicit pathspecs, and refusing one # would be this tool having an opinion about somebody's working - # directory. Untracked content INSIDE the leg is ignored for the same - # reason; the leg sitting at a different commit than the gitlink is NOT, - # because that is a half-finished bump and this one would land on top of - # it. + # directory. The one kind that could reach it, a workflow file the rewrite + # would pick up, is never picked up: `plan_workflows` rewrites tracked + # files only and refuses an untracked one by name (#207). Untracked + # content INSIDE the leg is ignored for the same reason; the leg sitting + # at a different commit than the gitlink is NOT, because that is a + # half-finished bump and this one would land on top of it. dirt = git_out(["status", "--porcelain", "--untracked-files=no", "--ignore-submodules=untracked"], cwd=root) if dirt: diff --git a/tests/test_bump_leg.py b/tests/test_bump_leg.py index 608da2c0..05d0a34d 100644 --- a/tests/test_bump_leg.py +++ b/tests/test_bump_leg.py @@ -122,6 +122,54 @@ def reference_both_legs(project: Path) -> tuple[str, str]: return spec, code +#: A workflow nobody has committed yet (#207): `{uses}` is its one job. +DRAFT = """\ +name: draft +on: workflow_dispatch +jobs: + d: + uses: {uses} +""" + + +def spec_ref(sha: str) -> str: + return f"{ORG}/{PROJECT}-spec/.github/workflows/x.yml@{sha}" + + +def leave_untracked(project: Path, name: str, uses: str) -> Path: + """A draft workflow written into the working tree and committed nowhere.""" + path = project / ".github" / "workflows" / name + path.write_text(DRAFT.format(uses=uses), encoding="utf-8") + return path + + +def ignore_locally(project: Path, pattern: str) -> None: + """`.gitignore` is tracked, and editing it would dirty the root; the + clone's own `info/exclude` ignores a file without touching the tree.""" + exclude = Path(git("rev-parse", "--git-path", "info/exclude", + cwd=project).stdout.strip()) + if not exclude.is_absolute(): + exclude = project / exclude + with exclude.open("a", encoding="utf-8", newline="\n") as handle: + handle.write(pattern + "\n") + + +def found(project: Path) -> dict: + """Everything a refusal promises not to have moved: the branch tip, the + gitlink in the index, the leg's own checkout, the pin, the working-tree + status and the bytes of every workflow file.""" + workflows = project / ".github" / "workflows" + return { + "head": git("rev-parse", "HEAD", cwd=project).stdout.strip(), + "gitlink": git("ls-files", "-s", "--", "spec", cwd=project).stdout, + "leg": git("rev-parse", "HEAD", cwd=project / "spec").stdout.strip(), + "pin": (project / "contracts" / "spec-pin.yaml").read_bytes(), + "status": git("status", "--porcelain", cwd=project).stdout, + "workflows": {path.name: path.read_bytes() + for path in sorted(workflows.iterdir())}, + } + + # --- the bump itself -------------------------------------------------------- def test_bump_moves_the_gitlink_and_the_pin_in_one_commit(project, scaffolded, @@ -242,6 +290,227 @@ def expression(path: Path) -> str: REPO / "templates" / "assembly-root" / "scripts" / "validate-pins.py") +# --- only TRACKED workflows are rewritten and committed (#207) -------------- + +@pytest.mark.parametrize("name", ["draft.yml", "draft.yaml"]) +def test_an_untracked_draft_naming_the_leg_is_refused_by_name_not_committed( + project, scaffolded, advanced, name): + """THE BUG: `plan_workflows` walked the directory on disk, so an untracked + draft that referenced the leg was rewritten and then staged by the commit's + explicit pathspec — added WHOLE to "Bump spec leg", as a new file. The + root-clean check says untracked files are skipped because they cannot + reach that commit, and this one did. + + It is refused rather than skipped: `validate-pins.py` reads every file on + disk, so a skipped draft would leave its old `@` to turn the bump red + after the rewrite. Refused BEFORE anything is written, and under + `--dry-run` as well, so the dry run is never a promise the real run + breaks. `.yaml` is read by the validator too, so it counts too. + """ + was = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + branched(project) + leave_untracked(project, name, spec_ref(was)) + before = found(project) + assert before["status"] == f"?? .github/workflows/{name}\n" + + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert "bump-leg-workflow-untracked" in result.stderr + assert f".github/workflows/{name} 1 reference(s)" in result.stderr + assert f"{ORG}/{PROJECT}-spec" in result.stderr + assert "committed" not in result.stdout + assert found(project) == before, "a refusal leaves the tree as it was" + + dry = bump(project, scaffolded, advanced, "--dry-run") + assert dry.returncode == 2, dry.stderr + dry.stdout + assert "bump-leg-workflow-untracked" in dry.stderr + assert found(project) == before + + +def test_an_untracked_draft_refuses_the_whole_bump_and_rewrites_nothing( + project, scaffolded, advanced): + """A TRACKED workflow that has to move does not move on its own once an + untracked one is in the way: half a bump is the failure the lockstep + commit exists to prevent.""" + was, _ = reference_both_legs(project) + branched(project) + leave_untracked(project, "draft.yml", spec_ref(was)) + before = found(project) + + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert "bump-leg-workflow-untracked" in result.stderr + assert ".github/workflows/legs.yml" not in result.stderr, ( + "only the file git does not hold is named") + assert found(project) == before + assert was in (project / ".github" / "workflows" / "legs.yml").read_text() + + +def test_every_untracked_draft_is_named_once_with_its_count( + project, scaffolded, advanced): + was = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + branched(project) + leave_untracked(project, "a-draft.yml", spec_ref(was)) + twice = project / ".github" / "workflows" / "b-draft.yml" + twice.write_text( + DRAFT.format(uses=spec_ref(was)) + f" e:\n uses: {spec_ref(was)}\n", + encoding="utf-8") + + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert "2 workflow file(s) that git does not track" in result.stderr + assert ".github/workflows/a-draft.yml 1 reference(s)" in result.stderr + assert ".github/workflows/b-draft.yml 2 reference(s)" in result.stderr + + +def test_moving_the_draft_out_of_the_directory_lets_the_bump_through( + project, scaffolded, advanced, tmp_path): + """The first of the two exits the refusal names.""" + was = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + branched(project) + draft = leave_untracked(project, "draft.yml", spec_ref(was)) + assert bump(project, scaffolded, advanced).returncode == 2 + + shutil.move(str(draft), str(tmp_path / "draft.yml")) + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert committed(project) == {"spec", "contracts/spec-pin.yaml"} + assert validate(project).returncode == 0 + + +def test_committing_the_draft_first_makes_it_move_with_the_bump( + project, scaffolded, advanced): + """The other exit: a COMMITTED workflow is a tracked file, so it is the + ordinary case — rewritten and staged with the pin, the dry run having + listed it.""" + was = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + branched(project) + draft = leave_untracked(project, "draft.yml", spec_ref(was)) + assert bump(project, scaffolded, advanced).returncode == 2 + + git("add", "--", ".github/workflows/draft.yml", cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Add the draft workflow", cwd=project) + dry = bump(project, scaffolded, advanced, "--dry-run") + assert dry.returncode == 0, dry.stderr + dry.stdout + assert ".github/workflows/draft.yml 1 reference(s)" in dry.stdout + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert committed(project) == {"spec", "contracts/spec-pin.yaml", + ".github/workflows/draft.yml"} + assert f"@{advanced}" in draft.read_text() + assert was not in draft.read_text() + assert validate(project).returncode == 0 + + +def test_an_untracked_workflow_with_nothing_to_move_is_left_alone( + project, scaffolded, advanced): + """Refusing is for a file the bump WOULD have rewritten. One that names no + leg of this project, or this leg already at `--to`, is not the bump's + business: not rewritten, not staged, not named, not refused — and the + project's own gate agrees with it afterwards.""" + _, code = reference_both_legs(project) + branched(project) + other = leave_untracked( + project, "other.yml", + f"{ORG}/{PROJECT}-code/.github/workflows/y.yml@{code}") + already = leave_untracked(project, "already.yml", spec_ref(advanced)) + kept = {path: path.read_bytes() for path in (other, already)} + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert committed(project) == {"spec", "contracts/spec-pin.yaml", + ".github/workflows/legs.yml"} + assert "1 reference(s) in 1 file(s)" in result.stdout + assert "other.yml" not in result.stdout + result.stderr + assert "already.yml" not in result.stdout + result.stderr + assert {path: path.read_bytes() for path in kept} == kept + status = git("status", "--porcelain", cwd=project).stdout.splitlines() + assert sorted(status) == ["?? .github/workflows/already.yml", + "?? .github/workflows/other.yml"] + assert validate(project).returncode == 0 + + +def test_an_ignored_workflow_naming_the_leg_is_refused_untouched( + project, scaffolded, advanced): + """Ignored is untracked: `git status` does not show it, but the validator + reads it, and the commit's `git add` would refuse it after the rewrite, + in git's words and not this tool's. It is named before anything is + written, and its bytes are exactly what they were.""" + was = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + branched(project) + ignore_locally(project, ".github/workflows/local-*.yml") + leave_untracked(project, "local-draft.yml", spec_ref(was)) + before = found(project) + assert before["status"] == "", "an ignored file is not in the status" + + result = bump(project, scaffolded, advanced) + assert result.returncode == 2, result.stderr + result.stdout + assert "bump-leg-workflow-untracked" in result.stderr + assert ".github/workflows/local-draft.yml 1 reference(s)" in result.stderr + assert found(project) == before + + +def test_an_ignored_workflow_with_nothing_to_move_is_untouched( + project, scaffolded, advanced): + _, code = reference_both_legs(project) + branched(project) + ignore_locally(project, ".github/workflows/local-*.yml") + local = leave_untracked( + project, "local-other.yml", + f"{ORG}/{PROJECT}-code/.github/workflows/y.yml@{code}") + kept = local.read_bytes() + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert committed(project) == {"spec", "contracts/spec-pin.yaml", + ".github/workflows/legs.yml"} + assert local.read_bytes() == kept + assert "local-other.yml" not in result.stdout + result.stderr + assert git("status", "--porcelain", cwd=project).stdout == "" + assert validate(project).returncode == 0 + + +def test_a_tracked_workflow_with_a_non_ascii_name_is_not_taken_for_untracked( + project, scaffolded, advanced): + """The tracked set is read NUL-separated (`git ls-files -z`), and this is + what that buys. Asked in the plain form, git QUOTES a name holding a byte + outside ASCII (`core.quotePath`, on by default) as + `".github/workflows/l\\303\\251gs.yml"`, which is not the path the + directory walk yields; a tracked `l\u00e9gs.yml` that names the leg would + then be refused as untracked, and "commit it first" could never clear it, + because it is already committed. Non-ASCII is the one quoted character + every platform this suite runs on can spell in a file name. + """ + was = git("rev-parse", "HEAD:spec", cwd=project).stdout.strip() + name = "l\u00e9gs.yml" + relative = f".github/workflows/{name}" + path = project / ".github" / "workflows" / name + path.write_text(DRAFT.format(uses=spec_ref(was)), encoding="utf-8") + git("add", "--", relative, cwd=project) + git("-c", "user.name=t", "-c", "user.email=t@t", "commit", "-qm", + "Add a workflow with an accented name", cwd=project) + branched(project) + plain = git("-c", "core.quotePath=true", "ls-files", "--", + ".github/workflows", cwd=project).stdout + assert "\\303\\251" in plain, ( + "this test only means something while git quotes the name in the " + f"plain listing: {plain!r}") + + result = bump(project, scaffolded, advanced) + assert result.returncode == 0, result.stderr + result.stdout + assert "bump-leg-workflow-untracked" not in result.stderr + assert f"@{advanced}" in path.read_text(encoding="utf-8") + assert was not in path.read_text(encoding="utf-8") + # Bytes, not text: the names come back from `-z` as git stores them. + names = subprocess.run( + ["git", "show", "--name-only", "-z", "--format=", "HEAD"], + cwd=str(project), capture_output=True, check=True).stdout.split(b"\0") + assert relative.encode("utf-8") in names + assert validate(project).returncode == 0 + + # --- --dry-run -------------------------------------------------------------- def test_dry_run_prints_the_move_and_changes_nothing(project, scaffolded,