Repository navigation
feat(cli): add release-check for the pre-tag checks - #257
Conversation
There was a problem hiding this comment.
Approved. I broke each gate on purpose and checked that it fires, then broke each check in the source and checked that a test notices.
Every gate fires, in isolation
On a clean checkout of this head, one mutation at a time:
mutation result
appended a line to a tracked module working-tree fail "1 tracked file(s) uncommitted: tabulate/tabulate.py"
(+ module-versions, manifest - correct, the body moved)
same, with --allow-dirty working-tree skip "--allow-dirty"
edited a version inside manifest.json manifest fail "manifest.json is stale; run make manifest"
pyproject 2026.10.23.7 vs zerodep.py .1 project-version fail "pyproject.toml has 2026.10.23.7, zerodep.py has 2026.10.23.1"
both sides set to 1.2.3 project-version fail "1.2.3 is not CalVer (YYYY.M.D or YYYY.M.D.N)"
appended to tabulate.py body module-versions fail "1 module(s) need a bump: tabulate"
deleted manifest.json manifest fail "manifest.json is missing; run make manifest"
stale docs index (worktrees mounted) docs-index fail
clean tree all five ok, exit 0
python -m zerodep release-check - the exact command release.yml now runs - works from the repo root and takes 1.5s, so the YAML drop-in is sound. release.yml keeps fetch-depth: 0, which the tag walk and last_updated both need.
The tests pin the gates
manifest comparison forced to always pass 1 failed (test_release_check_fails_on_a_stale_manifest)
version-mismatch branch disabled 1 failed (test_release_check_fails_on_a_version_mismatch)
44 CLI tests pass; the five new ones cover the pass path and four failure modes. Docs verified on the orphan branches: docs/guide/cli.md has the zerodep release-check section and both changelogs have the entry.
Ignoring generated when comparing the manifest is the right call - it is exactly the churn that made #238 hand-trim its diff, and it means a release does not need the timestamp rewrite any more.
Non-blocking
module-versionspasses vacuously when no tags are around. In agit clone --no-tagsof this head:
tags=0 module-versions ok 49 modules match their declared versions
Zero tags means every module whose version is not 0.0.0 lands in the "already bumped manually" branch, so the strongest gate silently says ok. _list_release_tags() returning empty is worth its own skip/fail row ("no release tags; run git fetch --tags") rather than an ok it cannot justify.
manifestfalse-fails in a shallow clone. With--depth 1viafile://:
manifest fail manifest.json is stale; run make manifest
last_updated as git sees it: 2026-10-23T12:38:51-05:00
recorded in manifest.json: 2026-10-22T21:28:28-05:00
Every file's last_updated collapses to the graft commit's date, so the comparison fails on a manifest that is actually correct. CI is immune (fetch-depth: 0), but the promise of "the same command locally and in CI" breaks in a shallow checkout. Comparing the manifest with last_updated dropped as well as generated would make the content hash the only signal, and cost nothing - the timestamp is derived metadata either way.
- The
docs-indexfailure detail is a diff line, not a diagnosis. On a stale index the row reads:
docs-index fail expected: '<!-- AUTO-GENERATED by _scripts/generate_module_index.py - DO NOT EDIT -->'
That is detail[-1] of the generator's output, which happens to be the first line of its diff. The stable message ("docs module index is stale; run make docs-index") is already written as the fallback for empty output; using it when the check fails, with the generator's output after it, would be easier to act on.
-
The docs gate is the one CI cannot run. As documented,
docs-indexskips wheneverdocs_en/docs_zhare not mounted, which is always true in Actions. So a release can be green here and still ship a stale module index. That is a fair trade for having one command, but it makesmake release-checkbefore tagging the only enforcement point, and worth saying so in the AGENTS note rather than only in the PR description. -
Behaviour change worth stating: the old workflow ran
python -m zerodep manifestduring the release and thengit add -A, so a stale manifest was silently repaired into the release commit. Now it aborts. That is the tightening #238 was missing, but it is a new way for the release job to fail - a maintainer whose last PR touched a module body has to remembermake manifestbefore dispatching. -
Minor:
_read_declared_versionmatches the first^version = "..."anywhere inpyproject.toml. Correct today (one occurrence, under[project]), but a[tool.*]table that grows aversionkey above[project]would be read instead. Scoping the regex to the[project]table removes the trap. -
Minor:
_CALVER_REaccepts2026.13.45- no month/day range check. Harmless for a self-chosen version, worth a note if the intent is real CalVer validation.
There was a problem hiding this comment.
The refactor is right: one command, one implementation of each rule, and release.yml no longer re-implements what the CLI already knows. Three of the five checks verify cleanly. But the manifest check fails on this PR's own base, which as written blocks the next release instead of gating it, so I can't approve yet.
The blocker
On 727aebf (this head, on 9f93c1d), a clean worktree:
Check Result Detail
module-versions ok 49 modules match their declared versions
manifest fail manifest.json is stale; run make manifest
project-version ok 2026.10.23.1 is CalVer and matches zerodep.py
docs-index skip docs worktrees are not mounted
working-tree ok no uncommitted tracked changes
exit=1
The same comparison on plain origin/master: 0 of 49 modules have a matching last_updated.
Cause: the check compares the whole parsed manifest, and last_updated comes from _git_last_updated() -> git log -1 --format=%aI -- <file>. Master's history is now five commits whose root is 8873cd3 release: v2026.10.23.1 - no parents, author date 2026-10-23T16:55:43+00:00, containing the entire tree. So every module's "last commit touching this file" is that root commit, and every computed last_updated is that one timestamp, while the committed manifest still carries the historical per-module dates.
Because this PR also removes the Regenerate manifest step, the release pipeline goes from self-healing to hard-blocked on exactly this drift: the first release after merge stops at release-check. Running make manifest does clear it, and the resulting diff is 50 insertions / 50 deletions - 49 dates all collapsing to 2026-10-23T16:55:43+00:00 plus generated.
Three ways out, in my order of preference: compare only the fields that describe the source tree (files, version, content_hash, deps, tier, category) and ignore last_updated the way generated already is; or regenerate the manifest in this PR and accept that provenance collapses to the root-commit date; or put a regeneration step back before the check, which makes the check unable to fail and gives up the point of the refactor.
What does check out
docs-indexwith the real docs branches mounted as worktrees:ok docs module index matches the manifest. Madedocs_en/docs/modules/index.mdstale and it reportedFAIL: ... has 199 lines, expected 197. Both paths work.working-tree: aftermake manifest, it correctly reported1 tracked file(s) uncommitted: manifest.json.module-versions: reuses_find_changed_modules, so it inherits the tag-based logic including the post-bump blind spot we discussed on #238 - deliberate, just noting the inheritance.test_cli.py: 44 passed.
Also
- The version that actually gets tagged is never checked.
Determine version(release.yml:41) computes${VER}from the workflow input or the date, the checks run at :65 against the previous release's version inpyproject.toml, and the sed bump to${VER}happens at :73, after the checks. A manual dispatch withversion: 1.0passes every check and gets taggedv1.0. Inline below. - The five new tests build a throwaway repo and generate the manifest inside it, so the manifest's
last_updatedalways agrees with the synthetic history - the exact case that breaks in the real repo is unrepresentable there. Inline below. docs-indexskipping when the worktrees are not mounted is honest and documented, but note the practical consequence: inrelease.ymlthat check never runs, so the docs gate only exists for maintainers running it locally. Worth one clause in the PR body so nobody reads a green CI release as "docs index verified".
| expected = _generate_manifest(repo_root) | ||
| on_disk.pop("generated", None) | ||
| expected.pop("generated", None) | ||
| if on_disk == expected: |
There was a problem hiding this comment.
This comparison is what fails on the current tree, and the reason is one field: last_updated.
_generate_manifest fills it from _git_last_updated() -> git log -1 --format=%aI -- <file>. Master's history is now five commits with a parentless root (8873cd3 release: v2026.10.23.1, author date 2026-10-23T16:55:43+00:00) that contains the whole tree, so "the last commit touching this file" is that root commit for all 49 modules. The committed manifest still has the historical dates, so the comparison differs on every module:
on origin/master: modules with matching last_updated: 0 / 49
expected 2026-10-23T16:55:43+00:00 vs disk 2026-08-25T10:54:32-05:00
on the PR head: manifest fail manifest.json is stale; run make manifest (exit 1)
So release-check reports a stale manifest on the very tree the PR says passes, and since the Regenerate manifest step is being removed from release.yml, the release fails instead of fixing itself.
Suggestion: compare the manifest with last_updated excluded, the same way generated already is:
for d in (on_disk, expected):
d.pop("generated", None)
for mod in d.get("modules", {}).values():
mod.pop("last_updated", None)That leaves the fields that actually describe the source tree (files, version, content_hash, deps, tier, category) and makes the check a statement about content rather than about git dates. If instead the intent is to keep dates honest, then this PR needs to regenerate and commit the manifest - which is a 50/50 diff where all 49 dates collapse to the root-commit date, so either way the field stops carrying per-module history.
| # Same code path maintainers run locally; a failing check here | ||
| # means the tree is not taggable (unbumped module, stale manifest, | ||
| # version mismatch, stale docs index, uncommitted changes). | ||
| python -m zerodep release-check |
There was a problem hiding this comment.
release-check validates the version that is already in pyproject.toml, but that is not the version this run will tag.
The order in this workflow is: Determine version (:41) computes ${VER} from github.event.inputs.version or date +%Y.%-m.%-d; Release checks (:65) runs here; Bump project version (:73) seds ${VER} into pyproject.toml and zerodep.py afterwards. So the CalVer rule is only ever applied to the previous release's version - dispatch this with version: 1.0 and every check passes, then the tree is bumped to 1.0 and tagged v1.0.
Two cheap fixes:
- pass the intended version in:
python -m zerodep release-check --expect-version "${VER}", with the check failing when--expect-versionis set and does not match_CALVER_RE(that also catches a manual input drifting from the repo's convention); or - run
Release checksafterBump project version, soproject-versionsees the version about to be tagged. This one has a wrinkle: the bump should stay in the same commit as the checks' result, andworking-treewould then flag the sed edits unless--allow-dirtyis passed, so the first option is cleaner.
Related, since this step is also where Regenerate manifest used to be: dropping it is the right call once the manifest check is trustworthy, but it changes a self-healing release into a blocking one for any module change merged without a regenerated manifest. That is the policy the check implies - just flagging it so it lands as a decision rather than a surprise on the next release.
| assert "docs worktrees are not mounted" in r.stdout | ||
|
|
||
|
|
||
| def test_release_check_fails_on_a_stale_manifest(tmp_path: Path): |
There was a problem hiding this comment.
This test edits the working tree, which is why it passes while the real repo fails.
_make_release_repo builds the repo and generates the manifest inside the same fixture, so the manifest's last_updated is derived from that fixture's own git state and always agrees with it. The failure mode that actually bites is different: a manifest generated before a commit that later touches a module, so git log -1 -- <file> returns a newer date than the manifest records. That case cannot be expressed here.
A test that would cover it, in the same fixture:
repo = _make_release_repo(tmp_path)
subprocess.run(["git", "-C", str(repo), "add", "-A"], check=True)
subprocess.run(["git", "-C", str(repo), "commit", "-qm", "touch demo"], check=True)
# manifest was generated before the commit; nothing in the working tree changed
r = _run_release_check(repo, "--allow-dirty")
assert "manifest" in r.stdoutWhether the assertion expects ok or fail depends on how the last_updated question above is resolved - but it is the case where the check and the git history disagree, and right now nothing pins it.
There was a problem hiding this comment.
Ran the command on this head (727aebf) in every state I could induce. Clean tree first:
Check Result Detail
module-versions ok 49 modules match their declared versions
manifest ok manifest.json matches the source tree
project-version ok 2026.10.23.1 is CalVer and matches zerodep.py
docs-index skip docs worktrees are not mounted
working-tree ok no uncommitted tracked changes
release-check passed
Then one induced failure per check:
touched sse/sse.py -> module-versions fail 1 module(s) need a bump: sse
tabulate version edited in JSON -> manifest fail manifest.json is stale; run make manifest
pyproject version = 1.2.3 -> project-version fail pyproject.toml has 1.2.3, zerodep.py has 2026.10.23.1
stale docs_en/docs_zh index -> docs-index fail (generator's own message)
tracked file modified -> working-tree fail 1 tracked file(s) uncommitted: …
Regenerating the docs index flips that row to ok, and an untracked scratch file leaves the run green (--untracked-files=no), which is the right default. --allow-dirty skips only the working-tree row. It also runs correctly from a subdirectory and as python -m zerodep release-check, which is the exact form release.yml uses. Exit codes are 0/1 as advertised.
The workflow change is a strict improvement beyond moving code: the old flow ran zerodep manifest, i.e. it rewrote a stale manifest on the way to tagging; the new one compares and fails. A release can no longer paper over a stale manifest.
Two gaps, neither blocking:
-
docs-indexalways skips in the release job.docs_en/is in.gitignoreandrelease.ymlonly doesactions/checkout@v4, so in CI that check reports "docs worktrees are not mounted" and the release proceeds. It works locally with the worktrees mounted (I verified both directions), but the fifth check never actually gates a tag. A--require-docsflag passed fromrelease.yml, or checking the docs branches out in that job, would make it real. -
The
module-versionsfailure path is untested. I mutatedcmd_release_checksostaleis always empty — the check then reports "49 modules match their declared versions" on a tree with an unbumped module — and all 5 new tests still passed. The tests cover a clean tree, a stale manifest, a version mismatch, a non-CalVer version, and uncommitted changes, but not the check that the whole release gate exists for.--allow-dirtyis also untested. Mutating the docs row does fail one test, so that path is at least pinned in its skip form.
Tests on the head: 5 release_check tests pass, the whole CLI suite is 44 passed. Against master's CLI the same 5 tests fail, so they do pin the feature. CI green on 3.10-3.13 plus lint and GitGuardian.
Approving — the implementation is careful (manifest comparison ignores generated, docs check fails instead of skipping when the worktrees are mounted) and the local/CI code path is now genuinely shared.
|
Both points fixed in 2e931fb, taking your first option for each. Manifest drift is no longer a release blocker. The check now compares only the fields that describe the source tree — The tagged version is now checked. Tests: four new cases cover timestamp drift, an accepted CalVer version, a rejected non-CalVer version and an existing tag (the last one in a git-initialised fixture repo). 48 CLI tests pass. On the |
|
Reproduced Clementine's table, and it is a shallow-checkout artifact rather than drift on master. Same head, same command, two checkouts:
This matters for which fix to take. Regenerating the manifest from that tree writes CI is unaffected either way: Where I think the review is right and I missed it: the version that actually gets tagged is never validated. |
There was a problem hiding this comment.
The blocker is gone and the checks still bite.
The failure I reported
Clean worktree at 2e931fb, no arguments:
Check Result Detail
module-versions ok 49 modules match their declared versions
manifest ok manifest.json matches the source tree
project-version ok 2026.10.23.1 is CalVer and matches zerodep.py
docs-index skip docs worktrees are not mounted
working-tree ok no uncommitted tracked changes
release-check passed
exit=0
Same tree, origin/master, that used to be 0 of 49 modules with matching last_updated. Now green.
That it still has teeth
I did not want to trade one blind spot for another, so I mutated a scratch worktree and re-ran the command each time (--allow-dirty so working-tree stays out of the way):
case exit manifest module-versions
clean tree 0 ok ok
module content edited (working tree) 1 fail fail
manifest content_hash stale 1 fail ok
manifest version stale 1 fail ok
manifest deps stale 1 fail ok
manifest tier stale 1 fail ok
manifest last_updated only 0 ok ok <- the fix
manifest generated only 0 ok ok
module dropped from manifest 1 fail ok
Every source-tree field still fails when it drifts; only the two commit-derived timestamps are ignored. Worth noting the module-versions cell in row two: editing a module now fails that check too, because v2026.10.23.1 is tagged with tabulate 0.1.2, so the tag-based comparison is live again for it - the post-bump blind spot we discussed on #238 closes as soon as a release lands.
--version
2026.10.24 exit=0 ok v2026.10.24 is free to tag
2026.10.23.1 exit=1 fail tag v2026.10.23.1 already exists; ...
1.0 exit=1 fail 1.0 is not CalVer (YYYY.M.D or YYYY.M.D.N)
2026.10.24.2 exit=0 ok v2026.10.24.2 is free to tag
release.yml:71 passes it and the checkout uses fetch-depth: 0, so git tag -l actually sees the tags in CI - that mattered, a shallow checkout would have answered "free to tag" for every version.
Rest
test_cli.py: 48 passed (was 44).- Docs on both orphan branches: changelog and
guide/cli.mdcover the command, the ignore rule and "when the docs worktrees are mounted", so the CI-skip caveat is written down where a reader will find it. docs-indexverified earlier with the real docs worktrees mounted (ok) and with a deliberately stale index (fail, accurate line count).
Two nits inline, neither blocking: the allowlist direction and the regex not validating an actual calendar date.
| # Fields that describe the source tree. ``last_updated`` per module and the | ||
| # top-level ``generated`` come from commit timestamps, so a squashed or | ||
| # rewritten history changes them without the manifest being stale. | ||
| _MANIFEST_SOURCE_FIELDS = ( |
There was a problem hiding this comment.
Allowlist here means a field added to the manifest later is silently ignored by this check, which is the same failure direction as the original bug, just inverted.
I tested it: adding "new_future_field": "x" to modules.tabulate in manifest.json leaves the check green.
unknown extra field added exit=0 ok manifest.json matches the source tree
A denylist of the two timestamp fields is exactly as safe today and stays correct as the manifest grows:
_IGNORED_MODULE_FIELDS = ("last_updated",)
def _manifest_source_view(manifest: dict) -> dict[str, dict]:
view = {}
for name, entry in manifest.get("modules", {}).items():
if not isinstance(entry, dict):
view[name] = entry
continue
view[name] = {k: v for k, v in entry.items() if k not in _IGNORED_MODULE_FIELDS}
return viewThat also flags a hand-edited manifest that gained a field the generator does not emit, which reads more like "matches the source tree" than skipping it. Not blocking - the allowlist covers every field the generator writes today.
|
|
||
| requested = getattr(args, "version", None) | ||
| if requested is not None: | ||
| if not _CALVER_RE.match(requested): |
There was a problem hiding this comment.
The regex checks the shape, not the date, so a typo in a manual dispatch walks through - which is the case --version exists to catch.
Measured at this head:
2026.13.01 exit=0 ok v2026.13.01 is free to tag (month 13)
2026.2.30 exit=0 ok v2026.2.30 is free to tag (Feb 30)
2026.99.99 exit=0 ok v2026.99.99 is free to tag (month 99, day 99)
2026.999.1 exit=1 fail 2026.999.1 is not CalVer (only because \d{1,2} caps the digits)
One extra line catches it, and it reuses the same failure row:
try:
year, month, day = (int(p) for p in requested.split(".")[:3])
datetime.date(year, month, day)
except ValueError:
... # same "is not CalVer (YYYY.M.D or YYYY.M.D.N)" row
except (TypeError, AttributeError):
...Non-blocking: the workflow default is date +%Y.%-m.%-d, which is always a real date, so this only bites a manual input - and it currently bites silently, which is the worst place for it.
Adds
zerodep release-check, so the checks that gate a release run locally through the CLI, andrelease.ymlcalls the same command instead of repeating them in YAML.What it checks
module-versionsversion-check --strictlogic)manifestmanifest.jsondoes not match the source tree (compared ignoring thegeneratedtimestamp)project-versionpyproject.tomlandzerodep.pydisagree, or the version is not CalVer (YYYY.M.D/YYYY.M.D.N)docs-indexdocs_en/docs_zhare mounted and the generated module index is stale; skipped otherwiseworking-tree--allow-dirtyskips this while iterating)Any failure exits 1. The docs-index check is skipped when the worktrees are not mounted, which is the case in CI, so the same command works in both places.
Changes
zerodep.py: therelease-checksubcommand. It reuses_scan_modules,_find_changed_modulesand_generate_manifest, so there is one implementation of each rule..github/workflows/release.yml: the separate "Check module versions" and "Regenerate manifest" steps become a singlepython -m zerodep release-checkstep. The manifest no longer needs a rewrite for the timestamp.Makefile:make release-check.AGENTS.md,docs_en/docs_zhCLI guide and both changelogs.Verification
test_cli.py: 5 new tests build a throwaway repo (copied CLI, one module, generated manifest) and cover the pass path, stale manifest, version mismatch, non-CalVer version and uncommitted tracked changes. 44 CLI tests pass.python zerodep.py release-checkon this tree: all checks pass.pre-commit(ruff, ruff-format, ty, complexipy) passes.