diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1121e729..9f4a2ab1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,6 +93,12 @@ jobs: run: | set -euo pipefail git fetch --depth=1 origin "$BASE_SHA" + # The tags are the guard's oracle for the one edit it permits: a + # released section moving back to what that version actually shipped. + # Shallow, because only the CHANGELOG.md at each tag is ever read. + # Missing tags are not an error here -- the guard refuses on a tag it + # cannot reach, so a fetch that comes up short fails closed. + git fetch --depth=1 --tags origin || true # `git show` writes nothing and exits non-zero if the path is absent at # that commit, and `set -e` turns that into a failure rather than an # empty file that would compare equal to anything. diff --git a/CHANGELOG.md b/CHANGELOG.md index 223b579a..b04868e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,40 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.37.0] - 2026-09-09 + +### Fixed + +- **`dl -- ` no longer re-splits a quoted argument, and no longer + runs one as shell.** The words after `--` were rejoined with plain spaces and + handed to `bash -lc` as a command line, so every space the host's shell had + already consumed became a separator again. `dl -- claude 'fix the bug'` + arrived as four arguments where one was meant. Each word is quoted now + (`shell::join`), so the remote argv is the argv that was typed. + + Two consequences beyond the splitting, both of them silent. A word holding `#` + commented out the rest of the line: a supervisor sending + `claude 'Address the open review on PR #10848 (...)'` reached the agent as the + single word `Address`, and everything after the `#` -- the whole prompt, + including every rule it carried -- was discarded by the remote shell before + `claude` ran. And a word holding `$(...)` or a backtick was *executed*, in a + workspace that has the forwarded `GH_TOKEN`, which made any text flowing into + a `dl --` command line (a PR title, a review body) shell code. + + `aid` composed its own line and passed it as one word, which survived only + because the rejoin was an identity on a single argument. It hands dl argv now + and quotes nothing itself. The composed payload is byte for byte what it was: + a bare `NAME=value` needs no quoting, so the assignment-prefix spelling the + README documents still reaches the shell as one. + + A shell snippet is still asked for by naming a shell -- `dl -- bash -lc + 'a && b'` -- and that spelling was broken before this too, running `bash -lc a` + and then `b`. Passing a snippet as a single word no longer works, because a + single word is now a program name: `dl -- 'exit 7'` looks for a program + called `exit 7` and exits 127 where it used to exit 7. The e2e probes were the + only callers in the tree spelling it that way, and they name `bash -lc` now. + `dl -- ""` went from exit 0 to 127 for the same reason. + ## [0.36.0] - 2026-09-09 ### Fixed @@ -76,36 +110,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 context, so this commit moves the tag once; launches build locally until `devcontainer-prebuild.yml` republishes on `main`. -- **`dl -- ` no longer re-splits a quoted argument, and no longer - runs one as shell.** The words after `--` were rejoined with plain spaces and - handed to `bash -lc` as a command line, so every space the host's shell had - already consumed became a separator again. `dl -- claude 'fix the bug'` - arrived as four arguments where one was meant. Each word is quoted now - (`shell::join`), so the remote argv is the argv that was typed. - - Two consequences beyond the splitting, both of them silent. A word holding `#` - commented out the rest of the line: a supervisor sending - `claude 'Address the open review on PR #10848 (...)'` reached the agent as the - single word `Address`, and everything after the `#` -- the whole prompt, - including every rule it carried -- was discarded by the remote shell before - `claude` ran. And a word holding `$(...)` or a backtick was *executed*, in a - workspace that has the forwarded `GH_TOKEN`, which made any text flowing into - a `dl --` command line (a PR title, a review body) shell code. - - `aid` composed its own line and passed it as one word, which survived only - because the rejoin was an identity on a single argument. It hands dl argv now - and quotes nothing itself. The composed payload is byte for byte what it was: - a bare `NAME=value` needs no quoting, so the assignment-prefix spelling the - README documents still reaches the shell as one. - - A shell snippet is still asked for by naming a shell -- `dl -- bash -lc - 'a && b'` -- and that spelling was broken before this too, running `bash -lc a` - and then `b`. Passing a snippet as a single word no longer works, because a - single word is now a program name: `dl -- 'exit 7'` looks for a program - called `exit 7` and exits 127 where it used to exit 7. The e2e probes were the - only callers in the tree spelling it that way, and they name `bash -lc` now. - `dl -- ""` went from exit 0 to 127 for the same reason. - ## [0.35.0] - 2026-09-09 ### Added diff --git a/README.md b/README.md index fe5268ff..41f21fc7 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ one argument instead of a clone, a config file and a build command. [![GitHub pull-requests merged](https://badgen.net/github/merged-prs/blooop/devlaunch)](https://github.com/blooop/devlaunch/pulls?q=is%3Amerged) [![GitHub release](https://img.shields.io/github/release/blooop/devlaunch.svg)](https://GitHub.com/blooop/devlaunch/releases/) [![PyPI](https://img.shields.io/pypi/v/devlaunch)](https://pypi.org/project/devlaunch/) -[![Conda](https://img.shields.io/badge/conda-v0.36.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) +[![Conda](https://img.shields.io/badge/conda-v0.37.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) [![License](https://img.shields.io/github/license/blooop/devlaunch)](https://opensource.org/license/mit/) [![Platform](https://img.shields.io/badge/platform-linux--64-blue)](https://github.com/blooop/devlaunch/releases) [![Pixi Badge](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/prefix-dev/pixi/main/assets/badge/v0.json)](https://pixi.sh) @@ -276,7 +276,7 @@ clone, and [docs/cleanup.md](docs/cleanup.md) says what it carries one past and ```bash $ dl --version -dl 0.36.0 +dl 0.37.0 ``` `--devcontainer ` picks a non-default `devcontainer.json`. A bare name means diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 1067898d..7f7fd40d 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -13,7 +13,7 @@ dependencies = [ [[package]] name = "aid" -version = "0.36.0" +version = "0.37.0" dependencies = [ "devlaunch-test-support", "dl", @@ -437,7 +437,7 @@ dependencies = [ [[package]] name = "devlaunch-core" -version = "0.36.0" +version = "0.37.0" dependencies = [ "devlaunch-runner", "devlaunch-test-support", @@ -455,7 +455,7 @@ dependencies = [ [[package]] name = "devlaunch-runner" -version = "0.36.0" +version = "0.37.0" dependencies = [ "libc", "portable-pty", @@ -464,7 +464,7 @@ dependencies = [ [[package]] name = "devlaunch-test-support" -version = "0.36.0" +version = "0.37.0" dependencies = [ "devlaunch-runner", "serde", @@ -506,7 +506,7 @@ dependencies = [ [[package]] name = "dl" -version = "0.36.0" +version = "0.37.0" dependencies = [ "clap", "devlaunch-core", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 5659fcf4..c0e1a25c 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -11,7 +11,7 @@ members = [ # The single source of the version (docs/rust-rewrite-plan.md: cutover ships # 0.1.0, version read from Cargo.toml). [workspace.package] -version = "0.36.0" +version = "0.37.0" edition = "2024" license = "MIT" repository = "https://github.com/blooop/devlaunch" diff --git a/scripts/changelog_frozen.py b/scripts/changelog_frozen.py index 17fe43a5..5506892d 100755 --- a/scripts/changelog_frozen.py +++ b/scripts/changelog_frozen.py @@ -32,6 +32,22 @@ Editing an old entry stays possible; it just has to be a visible, deliberate override rather than a thing that happens to you during a rebase. +**The one edit that is allowed, and why it has to be.** A released section may +move back to what its tag says shipped -- `git show v:CHANGELOG.md`, +byte for byte. Without that arm the guard has a state it cannot get out of: once +a misfiled entry is merged, it is on the base branch, so *every* honest +correction changes a released section and the guard refuses all of them. The +false record becomes the only text CI will accept, which is the opposite of what +this is for. It happened -- #591 re-cut a version #593 had already published, and +its entry landed under the shipped heading from the one direction the base-versus- +head comparison cannot see. + +The tag is what makes this safe rather than a loophole: it is written by the +release, not by the branch asking to be let through, so a branch cannot forge +agreement with it. Anything short of a confident answer from git -- no tag, no +git, an unparsable file at that tag -- refuses, so the arm can only ever permit +a restoration it has positively proved. + Usage: changelog_frozen.py @@ -41,6 +57,7 @@ import difflib import re +import subprocess import sys from pathlib import Path @@ -99,7 +116,40 @@ def read(path: Path) -> str: raise Unparsable(f"{path}: cannot be read ({problem})") from problem -def frozen_sections_differ(base: dict[str, str], head: dict[str, str]) -> list[str]: +def shipped_at_tag(version: str, run=subprocess.run) -> str | None: + """The section for `version` as it stands in the tag that published it. + + The oracle that tells a *restoration* from a corruption, which the two files + alone cannot: by content they are the same edit, and only the tag knows which + text actually shipped. `git show v:CHANGELOG.md` is the whole of it. + + `None` for anything that is not a confident answer -- no tag, no git, a + CHANGELOG.md that will not parse at that tag, a version the tag's own file + does not contain. Every one of those returns to the caller as a refusal + rather than as permission, because a guard that cannot reach its oracle has + checked nothing, and reporting that as success is the defect it exists to + find. + """ + try: + done = run( + ["git", "show", f"v{version}:CHANGELOG.md"], + capture_output=True, + text=True, + check=False, + ) + except OSError: + return None + if done.returncode != 0: + return None + try: + return sections(done.stdout, f"v{version}").get(version) + except Unparsable: + return None + + +def frozen_sections_differ( + base: dict[str, str], head: dict[str, str], shipped=shipped_at_tag +) -> list[str]: """Report every version present in both whose text is not identical. Versions only on `head` are new -- that is a release cut, and the case the @@ -109,12 +159,22 @@ def frozen_sections_differ(base: dict[str, str], head: dict[str, str]) -> list[s compared here. """ complaints = [] + restored: list[str] = [] for version, base_text in base.items(): if version == MOVING or version not in head: continue head_text = head[version] if head_text == base_text: continue + # A released section may still move in exactly one direction: back to what + # the tag says shipped. That is not a loophole in the rule, it is the rule + # applied to the case the guard cannot otherwise reach -- when the bad + # entry is already on the base branch, every honest correction changes a + # released section, and refusing them all leaves the false record standing + # as the only state CI will accept. + if head_text == shipped(version): + restored.append(version) + continue diff = "".join( difflib.unified_diff( base_text.splitlines(keepends=True), @@ -132,6 +192,8 @@ def frozen_sections_differ(base: dict[str, str], head: dict[str, str]) -> list[s f"section, say so in the pull request; this guard is meant to make that a\n" f"decision rather than an accident.\n" ) + for version in restored: + print(f"'## [{version}]' changed, and now matches v{version} exactly: restored") return complaints @@ -152,7 +214,7 @@ def main(argv: list[str]) -> int: print(complaint, file=sys.stderr) if complaints: return 1 - print(f"every released section is untouched ({len(base) - 1} compared)") + print(f"every released section is untouched or restored ({len(base) - 1} compared)") return 0 diff --git a/test/test_changelog_frozen.py b/test/test_changelog_frozen.py index 026a06f7..0cc4c852 100644 --- a/test/test_changelog_frozen.py +++ b/test/test_changelog_frozen.py @@ -206,3 +206,108 @@ def test_ci_runs_the_guard_on_pull_requests(): # `test_bench_workflow.py` and `test_review_guard.py` both write about. workflow = CI.read_text(encoding="utf-8") assert "scripts/changelog_frozen.py" in workflow + + +# --------------------------------------------------------- restoring a section + + +def tagged_repo(tmp_path: Path, shipped: str, tag: str) -> Path: + """A repository whose `tag` holds `shipped` as its CHANGELOG.md. + + The tag is the whole point: it is the only witness to what a version actually + contained, and it is written by the release rather than by the branch asking + to be let through, which is what stops the permitted edit from being a way + around the rule. + """ + work = tmp_path / "tagged" + work.mkdir() + git("init", "-q", "-b", "main", cwd=work) + git("config", "user.email", "guard@example.invalid", cwd=work) + git("config", "user.name", "Guard Fixture", cwd=work) + (work / "CHANGELOG.md").write_text(shipped, encoding="utf-8") + git("add", "CHANGELOG.md", cwd=work) + git("commit", "-qm", "the release", cwd=work) + git("tag", tag, cwd=work) + return work + + +def check_in(base: str, head: str, tmp_path: Path, cwd: Path) -> subprocess.CompletedProcess: + (tmp_path / "base.md").write_text(base, encoding="utf-8") + (tmp_path / "head.md").write_text(head, encoding="utf-8") + return run( + sys.executable, + str(GUARD), + str(tmp_path / "base.md"), + str(tmp_path / "head.md"), + cwd=cwd, + ) + + +def test_a_released_section_restored_to_what_it_shipped_is_permitted(tmp_path): + """The state the guard would otherwise have no way out of. + + Once a misfiled entry is merged it is on the base branch, so every honest + correction changes a released section -- and a rule that refuses all of them + makes the false record the only text CI accepts. blooop/devlaunch#591 re-cut a + version #593 had already published and its entry landed under the shipped + heading, arriving from the one direction a base-versus-head comparison cannot + see. + """ + shipped = changelog("", "## [0.2.0] - 2026-02-02\n\n### Fixed\n\n- What 0.2.0 fixed.\n") + corrupted = changelog( + "", + "## [0.2.0] - 2026-02-02\n\n### Fixed\n\n- What 0.2.0 fixed.\n- An entry it never had.\n", + ) + repo = tagged_repo(tmp_path, shipped, "v0.2.0") + + done = check_in(corrupted, shipped, tmp_path, repo) + + assert done.returncode == 0, done.stderr + assert "restored" in done.stdout + + +def test_a_released_section_edited_to_anything_but_its_tag_is_still_refused(tmp_path): + """The permitted edit is one destination, not a direction.""" + shipped = changelog("", "## [0.2.0] - 2026-02-02\n\n### Fixed\n\n- What 0.2.0 fixed.\n") + base = changelog("", "## [0.2.0] - 2026-02-02\n\n### Fixed\n\n- What 0.2.0 fixed.\n") + invented = changelog( + "", + "## [0.2.0] - 2026-02-02\n\n### Fixed\n\n- Something nobody released.\n", + ) + repo = tagged_repo(tmp_path, shipped, "v0.2.0") + + done = check_in(base, invented, tmp_path, repo) + + assert done.returncode == 1 + assert "already released and this branch changes it" in done.stderr + + +def test_a_restoration_it_cannot_prove_from_a_tag_is_refused(tmp_path): + """No oracle is a refusal, never a pass. + + The head here is the text that *would* be a restoration, and the only thing + missing is the tag that says so. A guard that cannot reach its oracle has + checked nothing, and reporting that as success is the same class of defect as + the one it exists to find. + """ + shipped = changelog("", "## [0.2.0] - 2026-02-02\n\n### Fixed\n\n- What 0.2.0 fixed.\n") + corrupted = changelog( + "", + "## [0.2.0] - 2026-02-02\n\n### Fixed\n\n- What 0.2.0 fixed.\n- An entry it never had.\n", + ) + repo = tagged_repo(tmp_path, shipped, "v9.9.9-not-the-one") + + done = check_in(corrupted, shipped, tmp_path, repo) + + assert done.returncode == 1 + assert "already released and this branch changes it" in done.stderr + + +def test_ci_fetches_the_tags_the_guard_reads(): + """The oracle is unreachable in a shallow checkout unless the tags come too. + + `actions/checkout` fetches none by default, so without this the guard would + refuse every restoration -- failing closed, but for a reason nobody could act + on from the log. + """ + assert "git fetch --depth=1 --tags origin" in CI.read_text(encoding="utf-8")