feat(ci): enforce vNEXT placeholder resolution in release PRs - #670
Merged
Conversation
Release checklist step 4 told you to resolve every `(since vNEXT)` and verify with a grep that "MUST come back empty". It never could: the process docs have to mention the placeholder they describe, so every release forced a fresh eyeball-classification of each hit -- with a silent-ship failure mode when one was missed. An unresolved gate is worse than no gate: the agent refuses a command the user actually has. check_version_gates.py gains a vNEXT residue check that separates the two mechanically -- a placeholder inside an inline-code span is prose quoting the token, outside one it is a live gate. Validated against the pre-0.90.0 tree: 16 real gates found, 4 prose mentions ignored, no allowlist needed. Two deliberate asymmetries, both measured rather than assumed: - The backtick rule is NOT applied to numeric gates. docs/sdk.md writes 14 genuine gates as `0.66.0+`, where backticks are typography rather than quotation; stripping code spans there would hide every one. - Fenced blocks are NOT exempt. CLAUDE.md's command list is one giant fence carrying real gates -- exempting fences dropped 2 of the 16. A false positive is a loud CI failure; a false negative is the bug the check exists to prevent. Residue is INFO on every run and fatal only under --release, because a feature PR is supposed to carry the placeholder. CI arms it via --release-if-newer-than <base version>, comparing with PEP 440 ordering rather than trusting the diff: a two-dot diff (all a shallow checkout can do) also fires for a stale branch whose base has since been released, and arming there would tell a contributor to delete a placeholder the process requires. - new `make vnext-check`; deliberately not in `make check`, which would then fail every feature PR run locally - CONTRIBUTING.md step 4, plugin sync map, CI workflows and local-CI sections updated; CLAUDE.md release flow notes the new enforcement - 19 new tests, including a positive control pinning both asymmetries
…base Review follow-ups from PR #670: - find_vnext_residue returned a bare (path, line, text) tuple, which CONTRIBUTING's Code Quality Patterns forbid beyond two values. Now a frozen VnextResidue dataclass; call sites and tests read named fields. - The CI step's fail-open path (unreadable base version -> gate never arms) now emits a ::warning:: annotation naming the consequence and the local fallback, instead of disarming silently. Verified live on PR #670's own run that the depth-1 fetch populates the base ref under the default shallow checkout (log: "base branch version: 0.90.0"), so the warning firing at all means the environment changed.
Friction log from running the 0.89.0 -> 0.90.0 release end-to-end, folded back into the checklist per its own closing rule: - NEW step 7: web/frontend/src/whatsnew.ts was in no checklist at all -- only docs/web-server.md and the file's own header mention it. Skipping it errors nowhere and the release's UI work ships dark (the popup falls back to the previous, already-dismissed reel). Also added to the plugin sync map as a silent-drift row. - Step 3: document the 160-char headline cap and the test that enforces it; 2 of 0.90.0's 13 bullets hit it blind. - Step 4: keep version tags out of markdown headings -- resolving vNEXT inside a heading changes its anchor slug and breaks inbound links (bit 0.90.0's What's-new section). - Steps 15-16 (was 14): "tag and publish the release" read as if manual publishing followed the tag. It doesn't: the tag push IS the release, everything else is release-kbagent.yml. Spelled out the exact command, the pyproject-must-match-tag gate, the do-not-pre-create rule (a hand-made release silently discards the changelog-rendered notes), and a post-tag verification step with the two historical half-release failure modes (v0.66.1 empty body, v0.64.0 missing wheel) as the reason to look. - CLAUDE.md release flow mirrors the same three additions in brief.
Member
Author
|
Scope note: the last commit ( |
padak
added a commit
that referenced
this pull request
Aug 23, 2026
* fix(ci): stop the live vNEXT gate turning main and every PR red `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. * fix(test): anchor the live vNEXT scan on REPO_ROOT, not the cwd `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.
Closes the last hand-run step of the release checklist.
The defect
CONTRIBUTING.mdstep 4 said: resolve every(since vNEXT), then verify withgrep -rn "vNEXT" CLAUDE.md docs/ plugins/ src/keboola_agent_cli/commands/context.pyIt never could. The process docs have to mention the placeholder they describe —
CLAUDE.mdlines 170, 177, 271 anddocs/web-server.md:351all contain it permanently. So every release forced a fresh eyeball-classification of each hit, with a silent-ship failure mode when one was missed. That is not a hypothetical: 0.90.0 (#669) had 16 to resolve across 6 files, and getting one wrong ships agents a gate no installed version can satisfy — strictly worse than no gate, because they then refuse a command the user has.The rule
A
vNEXTinside an inline-code span is prose quoting the token; outside one it is a live gate.Validated against the pre-0.90.0 tree (
1860a86): 16 real gates found, 4 prose mentions ignored, no allowlist needed. 16 is exactly what #669 resolved by hand.Two asymmetries — both measured, not assumed
Backtick-stripping is NOT applied to numeric gates. I checked before generalising:
docs/sdk.mdwrites 14 genuine gates as`0.66.0+`, where backticks are ordinary typography rather than quotation. Applying the rule there would hide every one.vNEXTis a placeholder token prose quotes; a version number is a value prose formats.Fenced blocks are NOT exempt, even though the same "code means quotation" argument seems to apply. I implemented fence-stripping first, and the positive control immediately dropped from 16 to 14:
CLAUDE.md's## All CLI Commandssection is one giant fence carrying real agent-facing gates. Reverted. The trade is asymmetric — a false positive is a loud CI failure someone fixes in a minute; a false negative is precisely the bug the check exists to prevent. A doc wanting to show the placeholder uses inline backticks. Both asymmetries are pinned by tests so a future "cleanup" cannot quietly undo them.Arming
Residue is INFO on every run and fatal only under
--release, because a feature PR is supposed to carry the placeholder.CI arms it with
--release-if-newer-than <base version>on any PR that raisespyproject.toml's version. The comparison is load-bearing: a two-dot diff (all a shallow checkout can do — there is no merge base) also fires for a stale feature branch whose base has since been released, where the version moved down. Arming there would tell a contributor to delete a placeholder the process requires them to write. PEP 440 ordering viapackaging(already a runtime dep) also keeps0.10.0above0.9.0and reads0.90.0b1as a release.0.89.00.90.00.91.0make vnext-checkis the local form. It is deliberately not inmake check— that would fail every feature PR run locally.Changes
scripts/check_version_gates.py—find_vnext_residue(),is_release_mode(),--release/--release-if-newer-than, residue count in--list.github/workflows/ci.yml— conditional step in thecheckjobMakefile—make vnext-checkCONTRIBUTING.md— step 4 rewritten (with the historical note on why the grep was unsatisfiable), plugin sync map row forgotchas.md, CI-workflows section, local-CI target list, commit-conventions bulletCLAUDE.md— release flow: writingvNEXTin a feature PR stays green; the release PR's replacement is now CI-enforcedtests/test_check_version_gates.py— 19 new tests (21 → 40 in the file)What this does NOT fix
Whether a behavior deserves a gate at all is still judgement, so a missing
gotchas.mdentry still ships silently — the sync-map row now says exactly that instead of implying full coverage. That half stays with/kbagent:review.Verification
make check— 6048 passed, 12 skipped, clean lint/format/typecheck/all gates1860a86; the fence variant caught only 14, which is why it was revertedmake vnext-checkgreen on this branch