fix(release): publish the CHANGELOG section and lock the release-time dev closure - #368
fix(release): publish the CHANGELOG section and lock the release-time dev closure#368pengfei-threemoonslab wants to merge 2 commits into
Conversation
Two loose ends from the release-workflow review (#345). The GitHub Release body was the placeholder `Agents Shipgate <tag>` while CHANGELOG.md held the entry describing what actually shipped, so the one artifact users read on the release page said nothing. And `pip install -e ".[dev]"` resolved fresh at tag time, so the run that decided whether to publish could install different packages than the CI run that approved the commit — a ruff, pytest or plugin release landing in between was enough, and a release-only failure was not reproducible from the same tree. Release notes: - `scripts/release_notes.py` extracts the CHANGELOG section matching the tag and `stage` publishes it through `--notes-file`, verbatim, from the checkout pinned to the verified commit rather than retyped at tag time. - Verification requires the section, so the rehearsal fails on a missing one while the tag does not yet exist. `## Unreleased` never matches a tag, which makes promoting that heading a step the pipeline enforces. - A body over GitHub's 125,000-character limit is refused there too, rather than by a 422 after tagging; the 0.16 development section is already ~75,000 characters. - Draft repair rewrites the body as well as the assets; an already published release is still left untouched. Dependency locks: - CI and release verification install the identical hash-locked closure in `constraints/dev.txt`, add the project separately (`--no-deps`, backend pinned by `constraints/release-build.txt`) because an editable install cannot be hashed, and run `python -m pip check` to prove the closure satisfies what the project declares. - `scripts/update_locks.py` regenerates every lock in one command and restores the headers `uv` would overwrite — the reason regenerating had been a per-file ritual. - `scripts/verify_dependency_lock.py` runs in CI and before publication and fails on a declared requirement with no pin, a pin outside the declared range, a direct requirement the declarations no longer contain, or a pin without a hash. It never re-resolves against the index, so an unrelated upload cannot turn the build red. Verified by installing the closure into a clean 3.12 environment and running CI's own selection there: 88.09% coverage, suite green. Refs #345 Co-Authored-By: Claude Opus 5 <[email protected]>
pengfei-threemoonslab
left a comment
pengfei-threemoonslab
left a comment
There was a problem hiding this comment.
Engineering review — changes requested
The overall direction is good and CI is green, but two correctness gaps defeat the core lock guarantees: the editable install still resolves its PEP 517 backend outside the reviewed lock, and the stale-lock verifier ignores extras, markers, and direct URLs. The four additional inline comments cover valid universal-lock shapes and release-note integrity/rehearsal edge cases.
I recommend addressing all six comments before merge, then rerunning the release-pipeline tests, the committed-lock verifier, and Agents Shipgate.
Submitted as a comment review because GitHub does not permit a pull request author to formally request changes on their own pull request.
| # byte-reproducible. | ||
| run: | | ||
| python -m pip install --require-hashes --requirement constraints/dev.txt | ||
| PIP_CONSTRAINT=constraints/release-build.txt python -m pip install -e . --no-deps |
There was a problem hiding this comment.
[P1] Editable install escapes the lock
--no-deps does not disable PEP 517 build isolation. This PR’s lock installs pip 26.2.1, where PIP_CONSTRAINT no longer constrains isolated build environments; the PR CI log confirms fresh Installing build dependencies and Installing backend dependencies, and constraints/dev.txt contains no Hatchling closure. CI and release verification can therefore execute different backend bytes as the index changes, defeating #345’s core invariant.
Please hash-lock and preinstall the complete backend closure, then add --no-build-isolation here and in release verification, with a regression that asserts both.
| declared: set[str] = set() | ||
| for requirement in requirements: | ||
| name = canonicalize_name(requirement.name) | ||
| declared.add(name) | ||
| pin = pins.get(name) | ||
| if pin is None: | ||
| problems.append( | ||
| f"{target.source} declares {requirement} but {target.lock} pins no {name}; " | ||
| "the lock is stale. Regenerate it with scripts/update_locks.py." | ||
| ) | ||
| continue | ||
| if requirement.specifier and not requirement.specifier.contains( | ||
| pin.version, prereleases=True | ||
| ): |
There was a problem hiding this comment.
[P1] Full requirement semantics are ignored
This loop validates only canonical name plus version specifier. Requirement.extras, .marker, and .url are ignored, and the lock parser discards pin markers. A declaration can change from demo>=1 to demo[feature]>=1, move to a platform marker, or switch to a direct URL without regenerating; verify_lock_target() still returns no problems, and pip check does not activate the project’s dev extra to recover that coverage.
Please bind and compare complete normalized PEP 508 direct declarations (or a declaration digest), including sources and marker branches.
| name = canonicalize_name(match.group("name")) | ||
| if name in pins: | ||
| raise ReleaseError(f"{lock_path}:{number} pins {name} a second time.") | ||
| current = Pin(version=match.group("version"), line=number) | ||
| in_via = False | ||
| pins[name] = current |
There was a problem hiding this comment.
[P2] Universal locks may repeat names
uv pip compile --universal may validly emit the same distribution multiple times with different versions or URLs under disjoint markers, but this dictionary rejects every second canonical name. A future dependency fork across Python or platform versions will make update_locks.py generate a valid pip lock that this verifier refuses.
Please model marker-qualified pin lists instead of one pin per name.
| if [ "$(gh release view "${RELEASE_TAG}" --json isDraft --jq .isDraft)" = "true" ]; then | ||
| # Repair replaces the notes too. A draft left by an earlier attempt | ||
| # can carry a body from a superseded changelog, and finalisation | ||
| # re-derives every asset but never looks at the release text. |
There was a problem hiding this comment.
[P2] Validate notes before undrafting
The remote assets are content-bound before publication, but the release body is not. Between stage and finalize—including the protected-environment approval window—a release-write actor or integration can edit the draft notes; the final gh release edit --draft=false then publishes text that is not the reviewed changelog section.
Please carry a verified release-notes.md digest or copy through the handoff and compare or atomically reapply it when undrafting.
| marker = fenced.group("marker") | ||
| if fence is None: | ||
| fence = marker | ||
| elif marker[0] == fence[0] and len(marker) >= len(fence) and not fenced.group("info"): |
There was a problem hiding this comment.
[P2] Accept whitespace after closing fences
CommonMark permits spaces or tabs after a closing fence, but not fenced.group("info") only accepts an exactly empty suffix. A valid closer such as ````` `` leaves fence open and raises `ReleaseError: unterminated code fence`, blocking rehearsal and release.
Please treat a suffix containing only spaces or tabs as empty and add a regression fixture.
| # before the build, the downloads, and the qualification checks. | ||
| env: | ||
| RELEASE_TAG: ${{ steps.candidate.outputs.release_tag }} | ||
| run: python scripts/release_notes.py --tag "${RELEASE_TAG}" |
There was a problem hiding this comment.
[P2] Rehearsal misses the output-path guard
This shared verification step only validates extraction. Stage later refuses any pre-existing file, directory, or broken symlink named release-notes.md, so a candidate containing that path can pass mandatory pre-tag rehearsal and fail only after the tag is pushed.
Please exercise the same precondition here or write the notes to a trusted $RUNNER_TEMP path.
Addresses the six review comments on #368. P1 — the editable install escaped the lock. `--no-deps` does not disable PEP 517 build isolation, and current pip does not apply `PIP_CONSTRAINT` to an isolated build environment: constraining hatchling to a version that does not exist still built successfully, and the CI log showed `Installing build dependencies` on every run. The backend and its own dependencies were therefore resolved from the index each time, which is the drift #345 exists to remove. New `constraints/build-backend.txt` is the backend's hashed closure, compiled from the existing hand-maintained `constraints/release-build.txt` so the version still lives in one reviewed place, and both pipelines now install it and use `--no-build-isolation`. Verified: zero build-environment resolutions, backend 1.31.0, suite green in a clean environment built that way. The same inert `PIP_CONSTRAINT` claim appeared in the sealing job's comment and in the documented qualification-promotion command, so both are corrected to the install-then-`--no-isolation` form that works. P1 — the lock gate ignored most of PEP 508. It compared canonical name plus version specifier, so a declaration could grow an extra, move behind a marker, or become a direct URL with no problem reported. Each lock now records the normalized declarations it was compiled from, in a generated block `update_locks.py` writes and `verify_dependency_lock.py` compares; all four failure shapes are covered by tests, and two of them are reproduced against the real pyproject.toml. P2 — universal locks may pin one name several times under disjoint markers. Pins are modelled as marker-qualified lists, every branch must satisfy the declaration, and only a same-marker repeat is a duplicate. P2 — the release body was not bound. Between staging and finalisation, the environment approval window included, a release-write actor can edit a draft's text; asset digests are re-derived before undrafting but the body was not. Verification now publishes the notes digest, staging and finalisation pass it back with `--expected-sha256`, and finalisation re-derives the notes from `CHANGELOG.md` at the verified commit and reapplies the body in the same API call that undrafts. P2 — a closing code fence may carry trailing whitespace per CommonMark; requiring an exactly empty suffix raised "unterminated code fence" on a valid changelog and would have blocked rehearsal and release. P2 — the output-path guard existed only in staging, so a candidate committing `release-notes.md` could pass the mandatory rehearsal and fail only after the tag was pushed. All three jobs now write to `$RUNNER_TEMP`, which removes the class rather than duplicating the guard. Also adds a cross-lock invariant: locks installed into one environment must agree on every shared distribution, or the second install moves part of the first one's closure. Refs #345 Co-Authored-By: Claude Opus 5 <[email protected]>
|
All six addressed in P1 — Editable install escapes the lockConfirmed, including the mechanism. Constraining hatchling to a version that does not exist still built successfully under this PR's pip 26.2.1: So New Verified in a clean 3.12 environment built with the new sequence: zero The same inert claim appeared twice more, so I corrected both rather than fix one instance of the bug class: the sealing job's comment ("PIP_CONSTRAINT pins the build backend" — there P1 — Full requirement semantics are ignoredCorrect. Each lock now carries a generated block recording the normalized PEP 508 declarations it was compiled from:
Reproduced against the real A lock with no block at all is rejected rather than assumed fine. P2 — Universal locks may repeat namesFixed. Pins are marker-qualified lists per name; every branch must satisfy the declaration (checking only the first would fail open when one fork is out of range), and only a repeat under the same marker is a duplicate. Tests cover the valid fork, the out-of-range branch, and the genuine duplicate. P2 — Validate notes before undraftingFixed by atomic reapply. Verification publishes P2 — Accept whitespace after closing fencesFixed; parametrized over P2 — Rehearsal misses the output-path guardTook the AlsoLocks installed into one environment must agree on every shared distribution, or the second VerificationEvery fix was mutation-tested — dropping |
1 participant
Summary
Closes #345 — the two non-blocking leftovers from the release-workflow review.
the placeholder
Agents Shipgate <tag>whileCHANGELOG.mdheld the entrydescribing what actually shipped, so the one artifact users read said nothing.
scripts/release_notes.pyextracts the section matching the tag andstagepublishes it through
--notes-file, verbatim, from the checkout pinned to theverified commit rather than retyped at tag time. A missing section fails
verification, which the rehearsal also runs, so it is caught while the tag
does not yet exist — and
## Unreleasednever matches a tag, which is whatmakes promoting that heading a step the pipeline enforces rather than one an
operator remembers. A body over GitHub's 125,000-character limit is refused
there too, rather than by a 422 after tagging; the 0.16 development section is
already ~75,000 characters. Draft repair rewrites the body as well as the
assets; an already-published release is still left untouched.
pip install -e ".[dev]"resolved fresh at tag time, so the run that decided whether to publish could
install different packages than the run that approved the commit — a ruff,
pytest or plugin release landing in between was enough — and a release-only
failure was not reproducible from the same tree. Both now install the
hash-locked
constraints/dev.txtwith the byte-identical command, add theproject separately (
--no-deps, backend pinned byconstraints/release-build.txt) because an editable install cannot be hashed,and run
python -m pip checkto prove the closure satisfies what the projectdeclares.
scripts/update_locks.pyrecompiles every lock and restores the prose headeruvwould overwrite — that overwrite is why regenerating had been a per-fileritual.
scripts/verify_dependency_lock.pyruns in CI and beforepublication, failing on a declared requirement with no pin, a pin outside the
declared range, a direct requirement the declarations no longer contain (uv's
# viaprovenance makes that detectable), or a pin without a hash. It neverre-resolves against the index, so an unrelated upload cannot turn the build
red.
For the reviewer
v0.16.0b7fails the new changelog gate today, by design —CHANGELOG.mdstill says
## Unreleased. Cutting a release now starts by promoting thatheading to
## <version> - <date>; it is step 1 in the runbook and is calledout in
CONTRIBUTING.md.Every control #345 asked to preserve is untouched: SHA-pinned actions, the
protected
pypienvironment, thedist/andqualified-dist/pre-existenceand symlink checks, HTTPS-only downloads, the strict qualified-wheel filename,
and the tag ↔ package-version check. The one new job-written path
(
release-notes.md) follows the same pre-existence rule as the directories theverification job creates, so a committed symlink cannot decide where the write
lands.
Nothing here changes what the token-bearing jobs install.
constraints/dev.txtis the verification closure: it installs the project and runs its tests, so it
is outside the blast radius
release-seal.txtandrelease-publish.txtarelocked against. That distinction is written into the lock's header.
Type
(Release pipeline and repository tooling; no product surface, no new check IDs,
no schema or contract version change.)
Verification
CI is authoritative for
python -m ruff check .,python -m compileall -q src tests, andpython -m pytest.Additional local checks run:
lock —
pytest -n auto -m "not perf" --cov-fail-under=85: green, 88.09%coverage.
ruff(0.16.2, from the lock),compileall,generate_schemas.py --check,test_adapter_static_only.py,python -m build+twine check,and
cyclonedx-py environmentall pass in that environment too. This is thecheck that matters for the lock change: the packages CI will use are the ones
the suite actually ran against.
tests/test_release_pipeline.py, and every new gate wasmutation-tested rather than assumed: restoring the placeholder
--notes,deleting the changelog step from verification, and drifting CI's install from
the release's each fail the corresponding test; the three real lock failure
modes (range widened past the pin, dependency added, dependency removed) each
fail
verify_dependency_lock.pyagainst this repository with an actionablemessage.
agents-shipgate verify --config shipgate-self.yamlreturnsreview_required— "edits a release trust root … a human must review it" —which is the correct verdict for a
.github/workflows/**change and does notfail advisory CI.
pip_audit .cannot run on this Mac (its temporary venv'sensurepipdieswith SIGABRT); reproduced on the unmodified environment, so it is pre-existing
and not caused by this change. CI is the authority for that step.
Only CI can prove one thing I could not: the lock was installed and
exercised on macOS/arm64, and the universal lock's Linux wheels resolve for the
first time on this PR's ubuntu runner.
Release-readiness notes
docs/checks.md(none)STABILITY.md(none)🤖 Generated with Claude Code