fix(ci): stop the live vNEXT gate turning main and every PR red - #678
Merged
Conversation
`TestLiveRepositoryVnext::test_no_unresolved_placeholder_survives_a_release` asserted that the live tree carries no `(since vNEXT)` placeholder. That is not an invariant of this repo -- it is the opposite of the documented process. Since #648, a feature PR that documents version-gated behaviour writes the literal `vNEXT` placeholder and only the release PR rewrites it. CLAUDE.md says so directly ("Writing `vNEXT` in a feature PR is correct and stays green"), and TestReleaseModeSelection, added by the same PR as this assertion, explains that arming the gate outside a release PR "would demand a contributor delete a placeholder the process requires them to write". So main legitimately carries placeholders for most of a release cycle. The assertion was green only because #670 landed days after 0.90.0 had just rewritten every one of them. #675 added the first placeholder of the next cycle and the assertion went red -- on main, and on every open PR, since a PR run tests the merge commit. The release-time requirement itself is real and stays enforced where it can see the base branch's version: the "Unresolved vNEXT placeholder check" step in ci.yml (`--release-if-newer-than`) and its local twin `make vnext-check`. A unit test cannot see that version, so it cannot make this call. What a test CAN assert is that the live scan works, and that replaces it: every gate the scanner reports must point at a real line that really contains the placeholder. Zero hits and ten hits are both valid answers depending on where in the cycle the tree sits. The neighbouring "guards the guard" test already covers the globs resolving at all.
`VnextResidue.path` is repo-root-relative -- find_vnext_residue stores `path.relative_to(REPO_ROOT).as_posix()` -- so reading it back as a bare `Path(gate.path)` resolves against the current working directory. The loop body only executes when the tree actually carries a placeholder, which is precisely the state main is in now, so running pytest from anywhere but the repo root raised FileNotFoundError. Reproduced from tests/ before the change and confirmed green after, from both the repo root and a subdirectory. `REPO_ROOT / gate.path` also covers the one fallback branch, where a path outside the repo is stored absolute: joining an absolute right-hand side discards the left, yielding that path unchanged. Raised by Devin review on #678.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
mainis red, and so is every open PR (a PR run tests the merge commit):Verified directly against
origin/main:Why it fires
The test asserts the live tree carries no
(since vNEXT)placeholder. That is the opposite of the documented process.Since #648, a feature PR that documents version-gated behaviour writes the literal
vNEXTplaceholder, and only the release PR rewrites it. CLAUDE.md states it outright:And
TestReleaseModeSelection, added by the very same PR as this assertion, spells out the reasoning:So
mainlegitimately carries placeholders for most of a release cycle. The assertion was a time bomb: #670 landed a few days after 0.90.0 had rewritten every placeholder, so it passed on an empty tree. #675 added the first placeholder of the next cycle and detonated it.The CI step is correctly gated — it passes
--release-if-newer-than "$base"and arms only on a version-raising PR. Only the pytest mirror is ungated, because a unit test cannot see the base branch's version.The change
Drop the false invariant; keep the enforcement where it can actually see the base version (the ci.yml step and
make vnext-check).Replaced with the half that does have a stable answer: every gate the live scanner reports must point at a real line that really contains the placeholder. Zero hits and ten hits are both valid depending on where in the release cycle the tree sits. The neighbouring
test_prose_mentions_are_still_present_and_ignoredalready guards against the globs silently matching nothing.Verification
make checkgreen on this branch —6077 passed, 12 skippedScope
Found while working on #676 (CI cost). Split out deliberately: this is unrelated to that change, blocks every open PR, and should merge on its own.