fix(caller-drift): derive version-bump-gate inputs from repos.yml, not the caller (backend#2953) - #447
Conversation
…t the caller (backend#2953)
The version-bump-gate caller in each version_file repo restates
release-train/repos.yml's `version_file`/`publish_paths`. Measured
2026-09-01, five of six had drifted NARROWER than the source, each
dropping a published path the gate then stopped watching -- so a PR
touching only those files shipped changed bytes under an already-released
version and still got a green check. Correcting the copies buys one
afternoon and re-arms the trap; nothing compared the two.
Close the loop where both sides are readable at once. The reusable cannot:
repos.yml is PRIVATE and a reusable runs with the caller repo's token. The
caller-drift audit already runs under an org-wide-read PAT and already
reads repos.yml for train membership, so:
- load_release_train now returns {name: {version_file, publish_paths}}
(keys are still the membership set), reading the two fields from repos.yml
and failing closed on a version_file with no publish_paths.
- version_bump_input_findings compares each repo's caller inputs against
those derived values -- holding no copy of its own (the
env-vocabulary-agreement shape). NARROWER (unsafe) and WIDER (nag) are
distinct findings; version-file drift is caught on its own; exclude-paths
and soft-fail are deliberately not compared.
- the audit loop runs it for every repo repos.yml gives a version_file,
counted in the `callers` family.
Also refreshed two statements this makes false: version-bump-gate.yml's
header (dropped the stale hand-copied per-repo table, pointed at the guard)
and the inventory's shared exempt reason (dropped the brittle "five" count).
Verified: caller-drift selftest 210/0 (13 new cases pin direction wording,
set semantics, the two ignored inputs, and load_release_train's fail-closed
paths); version-bump-gate selftest 58/0; ruff E4,E7,E9,F and actionlint
clean; reason-citations clean. Live proof against the fleet: flags
design-system-v2 (`src/*`, omits `tokens/*`) and clears the other five.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
bugbot run |
CI triage (head
|
| # | finding | mine? | disposition |
|---|---|---|---|
| 1 | design-system-v2 version-bump-gate-caller publish-paths NARROWER (omits tokens/*) |
yes | this is the guard added here doing its job; fixed by tracebloc/design-system-v2#313 (green, ready) |
| 2–6 | design-system-v2 protection.develop/staging/prod + rulesets.promotion_merge_commit_only/tag_trust_root exempt-but-present |
no | pre-existing; reasons on file dated 2026-08-27, being re-recorded in #443 (staging/main stay flagged by design until the branches are armed — backend#2547) |
| 7 | client-runtime develop strict=True vs policy |
no | pre-existing; addressed in #443 |
| 8 | rfcs main conversation-resolution (marked divergent) |
no | pre-existing; not on the train |
So exactly one finding belongs to this PR, and it is the intended catch — #313 clears it. The other eight are pre-existing fleet drift that already turned develop's scheduled audit red on 2026-09-07, before this branch existed (git diff origin/develop -- repo-inventory.yml here touches only the shared exempt-reason text — no protection/ruleset entry).
Merge order: the required gate can only go green once the whole-fleet audit is clean, so this PR is gated behind (a) #313 and (b) the fleet returning to green (#443 + the design-system-v2 arming in backend#2547). That coupling is the conformance-gate's existing design — a contract change waits for a clean fleet — not a defect here. The code itself is verified: 210/0 selftests plus a live run against the fleet that flags only design-system-v2 and clears the other five version_file repos.
LukasWodka
left a comment
There was a problem hiding this comment.
Read at cb7210e. The derivation is right and I reproduced the live proof independently: load_release_train reads version_file/publish_paths from release-train/repos.yml and version_bump_input_findings holds no copy of either; comparing the real repos.yml against the six live callers on each repo's develop gives 6/6 agreement today (design-system-v2#313 merged this morning), so the new check arms green. Selftest 210/0 locally; mutations swapping NARROWER/WIDER, comparing as an ordered string, dropping the version-file compare and removing the fail-closed die are all caught.
Why CI is red — not this PR's code. audit reports 9 findings: 1 is the new check flagging design-system-v2 src/* vs src/* tokens/* (fixed by dsv2#313, clears on re-run); the other 8 are develop's own pre-existing drift, red on the scheduled audit since 2026-09-07 (five stale design-system-v2 exemptions, .github staging not requiring selftests, client-runtime strict, rfcs conversation resolution). gate fails transitively on that, so every contract-file PR is blocked until #443 lands. Please update the "Merge order" section accordingly (dsv2#313 is done; #443 is the real dependency).
Before approval — the wait on #443 gives room for these:
-
Zero pairs must not pass (
load_release_train/ the wiring at ~L2441). Ifrepos.ymlyields no entry withversion_file(key renamed, dropped fleet-wide), the check runs on nobody and the audit is green — reproduced with a stub repos.yml usingversionFile:.die()when no train entry carriesversion_filewhile the inventory marks anyversion-bump-gate.ymlcallerrequired, and flag an inventory-requiredrow whose repos.yml entry has noversion_file(theversion_gate_not_applicablereason asserts that correspondence in prose; make it a check). -
Assert the specific refusal (selftest L2054, L2068).
die()always exits 2, and the stub's misroute path (GhError(500)→die) also exits 2 — renamingTRAIN_FILEshows both fail-closed tests pass for the wrong reason. Capture stderr and assert thedeclaresversion_filebut no usablepublish_paths`` message. -
A malformed train entry should be a per-repo finding, not a whole-audit abort.
repos.ymlitself says omittingpublish_pathsis the legal strict mode ("Omit the key to keep the strict behaviour"), soversion_filewithoutpublish_pathsis a reachable, sanctioned state — and the newdie()on it exits 2 before any repo is read, discarding protection/ruleset/copies/callers findings for all 20 repos and blocking every contract PR here until a file in a different repo is edited. Record it on the entry (the file already has theunreadablebucket shape) and emit one finding for that repo; the run stays red, the rest of the report survives. -
exclude-pathscan reproduce the exact drift this guard catches. In the gate,is_excludedruns per file inside thefor pat in $PUBLISH_PATHSloop (L662), sopublish-paths: "src/* tokens/*"+exclude-paths: "tokens/*"watches exactly whatpublish-paths: "src/*"does — NARROWER in effect, clean in this audit. At minimum flag an exclude glob that equals a repos.yml publish glob; and the docstring's "names files INSIDE publish-paths that do not ship" is a claim, not a check. -
soft-failis owned by nobody. The selftest (L1998) says "the inventory'scaller_inputsfloor owns it", butcaller_inputsat head has only acode-quality.ymlrow. A caller that dropssoft-fail: falsereverts the gate to advisory with no finding anywhere. Addversion-bump-gate.yml: {soft-fail: false}tocaller_inputs(all six callers already pass it, so it lands green), or drop the claim. -
Two implementations of one rule.
release-train/scripts/publish-inventory-check.shfamily 3 (release-train#147, backend#2953) already compares each caller'spublish-pathsandversion-fileagainstrepos.ymlon its own Monday cron. This PR adds a second, differently-worded implementation and the rewritten gate header names only this one as "the cross-check". Either say which is authoritative and cross-reference the other, or retire one — not both silently.
Optional: nothing exercises the main() wiring (pointing it at "nope.yml" leaves the selftest 210/0); a small test on the "is this repo checked" predicate would close that, but it is a pre-existing pattern and not a blocker.
Bugbot clean, no conflicts. Happy to approve once these are in and gate is green after #443.
…l message, cross-ref the train check (backend#2953) Address part of @LukasWodka review on .github#447: - Ask 5: add version-bump-gate.yml {soft-fail: false} to caller_inputs. The gate soft-fail defaults TRUE and the derive-from-repos.yml check does not compare it, so a caller dropping it reverted the gate to advisory with no finding anywhere. Measured green across all six callers. - Ask 2: the two fail-closed load_release_train tests asserted only exit 2, which every die() gives (a misroute or a renamed TRAIN_FILE passed them for the wrong reason). Capture stderr and assert the specific publish_paths refusal so they pin THIS branch. - Ask 6: cross-reference release-train/scripts/publish-inventory-check.sh -- the weekly train-side sweep of the same invariant -- so the header no longer names only this check, and says keep the two in step. Asks 1/3/4 (zero-pair guard, malformed-entry die-vs-per-repo-finding, exclude -paths) need the repos.yml strict-mode semantics confirmed -- replied on the PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
bugbot run |
|
Thanks for the thorough review, and for separating the CI-red (fleet drift, waits on #443) from the code. Pushed
On the remaining three I'd like to converge with you before I touch the audit's failure semantics, because 3 turns on a fact I can't verify from this repo: 3 — malformed entry → per-repo finding vs whole-audit 1 — zero pairs must not pass. Agreed this is a real hole. I'll 4 — exclude-paths drift. Also real: Bundling 1+3+4 into the next push once you confirm the strict-mode reading, so the failure-mode changes land coherently. Leaving this change-requested until then. |
LukasWodka
left a comment
There was a problem hiding this comment.
Re-read at eef2edc. Thanks for splitting the push — 2, 5 and 6 are settled:
- 2 — both fail-closed tests now capture stderr and assert
no usable publish_paths. Verified the assertion discriminates: withTRAIN_FILErenamed, the misroute path's stderr no longer satisfies it, and the selftest reddens. - 5 —
caller_inputsnow carriesversion-bump-gate.yml: {soft-fail: false}; the floor consumer flags a missing key as well as a wrong value, and all six required callers on develop passsoft-fail: false, so it lands green. - 6 — the
load_release_traincross-reference to release-train's family-3 sweep (train-side weekly vs fleet-audit PR-time, repos.yml the single source both read) is the resolution I asked for. Accepted.
Selftests 210/0 and 58/0 locally, ruff clean, Bugbot clean, no conflicts.
The confirmation you asked for on 3. I read release-train's repos.yml on its default branch. The publish_paths comment under data-ingestors reads: "Omit the key to keep the strict behaviour: absent is never permission to skip the version guard." — and the cli block repeats it. So version_file without publish_paths is a legal, sanctioned strict mode, not a malformed source. Your reading that the gate needs both is also right — the reusable marks publish-paths required: true — and that is exactly why it should be a per-repo finding on that repo ("repos.yml is in strict mode for X; the version-bump-gate caller cannot be conformant — add publish_paths or exempt the caller"), not a fleet-wide die before any repo is read. Today all six entries carry both keys, so the die is unreachable until a one-line edit elsewhere makes it fire and discards every other repo's findings. Please convert it as you proposed (record on the entry, per-repo finding, run stays red on that repo, the other 19 survive) and re-point the two ask-2 assertions at the finding text. The empty-string case can be the same per-repo finding.
Still open, as you planned for the next push:
- Zero pairs. Reproduced again on this head: a
repos.ymlwith the key renamed (versionFile:) returns without dying and the wiring predicate runs on nobody.die()when no train entry carriesversion_filewhile the inventory marks anyversion-bump-gate.ymlcallerrequired; flag an inventory-requiredrow whose train entry has noversion_file. exclude-pathshollowing. Reproduced:publish-paths: "src/* tokens/*"+exclude-paths: "tokens/*"→[]. Flagging an exclude glob equal to a repos.yml publish glob is the right minimum.
Merge order in the body still says design-system-v2#313 must merge first — it merged this morning; #443 (still draft, itself red) is the real dependency for the required gate.
Optional, not blocking: the selftest comment "caller_inputs owns it" is now true but unpinned — one assertion that the real inventory carries caller_inputs["version-bump-gate.yml"]["soft-fail"] is False would stop a silent deletion of the row leaving 210/210 green.
Holding changes-requested until 1, 3, 4 and the body update land; approve then, gated on gate going green after #443.
…lude hollowing (backend#2953) Ask 3: a repos.yml version_file with no publish_paths is its sanctioned strict mode, not a malformed source -- load_release_train carries it and version_bump_input_findings reports a per-repo finding, so one entry omitting the key no longer dies the whole audit. Ask 1: fail closed when no entry carries version_file while the inventory still requires the caller (fleet-wide die + per-repo finding). Ask 4: an exclude-paths glob equal to a repos.yml publish glob hollows the gate -- a finding. Ask-2 assertions re-pointed at the finding text; selftest 221/0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed 1, 3, 4 and the body in 6a1847b — thank you for reading release-train's 3 — strict mode is now a per-repo finding, not a whole-audit die. Ask-2 assertions re-pointed. The two tests now assert the finding text ("strict mode", "no usable 1 — zero pairs, both halves. Extracted to pure helpers so they're unit-tested, not asserted structurally:
4 — exclude hollowing. An Body + optional pin. Merge-order section updated: dsv2#313 has merged; #443 is the real Selftests 221/0 and 58/0, ruff clean. Re-requesting — merge still correctly gated on |
|
bugbot run |
… (backend#2953) The strict-mode finding advises "add publish_paths or exempt the caller", but it fired purely on repos.yml state, so exempting never cleared it and an exempted repo stayed red -- the advice was untrue. Thread the caller state through version_bump_input_findings and return no finding when the caller is exempt. Selftest 223/0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 908a147. Configure here.
LukasWodka
left a comment
There was a problem hiding this comment.
Re-review of 908a147 (delta from 6a1847b only).
Asks 1/3/4 and the body fix were already verified at 6a1847b (per-repo strict-mode finding, zero-pair guard fleet-wide + per-repo, exclude-paths hollowing finding, all mutation-proven; selftest 221/0) — nothing in this delta touches those paths.
908a147 threads the inventory's version-bump-gate caller state into version_bump_input_findings(..., caller_state) and short-circuits the strict-mode finding when the caller is exempt, so the finding's own second remedy ("or exempt the caller") now actually clears it. Same _vbg_state expression feeds version_bump_missing_pair_finding, so the two per-repo checks read one state. The short-circuit sits inside the empty-publish_paths branch only, so it cannot silence the hollowing / NARROWER / WIDER comparisons, and the stale-exemption check above it still flags an exempt row whose caller file exists.
Verified locally: selftest 223/0; removing the short-circuit reddens exactly vbg: an exempt caller clears the strict-mode finding (222/1) while vbg: a required caller keeps the strict-mode finding stays green. Bugbot's Low thread is resolved.
CI: audit/gate red on this head = the same 8 pre-existing develop findings (design-system-v2 ×5, .github staging selftests, client-runtime strict, rfcs conversation), zero from this PR's new checks. Code LGTM — holding approval until #443 lands and the audit goes green on a fresh head. Nothing further needed from you on this PR.
The class
version-bump-gate.yml's header saysversion_file/publish_pathslive in release-train/repos.yml— then the caller in eachversion_filerepo restates them and the job only echoes them into a run summary. Measured 2026-09-01, five of six callers had drifted narrower than the source, each dropping a published path the gate then stopped watching: a PR touching only those files ships changed bytes under an already-released version and still goes green. Correcting the copies (four already were, by hand, between the issue filing and today) buys one afternoon — nothing compared the two.The fix: derive, don't restate
The reusable can't read
repos.yml(it's private; a reusable runs with the caller repo's token — see the gate header). Thecaller-driftaudit already runs under an org-wide-read PAT and already readsrepos.ymlfor train membership, so the cross-check belongs there — theenv-vocabulary-agreement.shshape the issue names, holding no copy of its own:load_release_trainnow returns{name: {version_file, publish_paths}}(keys are still the membership set), reading the two fields fromrepos.ymland failing closed on aversion_filewith nopublish_paths.version_bump_input_findingscompares each repo's caller inputs against the derived values.NARROWER(unsafe — under-watches) andWIDER(nag) are distinct findings;version-filedrift is caught on its own;exclude-paths(backend#2758) andsoft-failare deliberately not compared (norepos.ymlcounterpart).repos.ymlgives aversion_file, keyed off the source rather than the inventory's required/exempt flag, counted in thecallersfamily.Statements this makes false, updated in the same PR
version-bump-gate.ymlheader: dropped the stale hand-copied per-repo table (itself drifted) and pointed at the guard.repo-inventory.ymlshared exempt reason: dropped the brittle "five version-published repos" count (now six, drifts again on the next).Merge order
design-system-v2#313(the one live drift this guard's new check flagged) has merged to develop, so that instance is cleared. The remaining blocker is the requiredgate: the fleetauditis red on eight pre-existing drifts unrelated to this PR (stale dsv2 exemptions,.githubstaging not requiring selftests,client-runtimestrict,rfcsconversation resolution), and because this PR changes the audit contract,gatefails transitively until that remediation lands. The real dependency is #443 (still draft, itself red); this PR is otherwise ready and its own new check arms green.Verified
caller-driftselftest 221/0 — new cases pin direction wording, set/order semantics, the two ignored inputs, the strict-mode per-repo finding, exclude-paths hollowing, the zero-pair guard (fleet-wide + per-repo), andload_release_train's carry-through / fail-closed paths.version-bump-gateselftest 58/0 (script body untouched; header-only change).ruff --select E4,E7,E9,F(CI's selection),actionlint, andreason-citationsall clean.design-system-v2(src/*, omitstokens/*), clears the other five.Closes tracebloc/backend#2953.
🤖 Generated with Claude Code
Note
Medium Risk
Changes org-wide caller-drift contract and can redden fleet audits on publish-path drift; logic is test-heavy and read-only against repos.yml, but false positives would block contract PRs until callers are fixed.
Overview
Adds a fleet audit that compares each repo’s
version-bump-gatecaller torelease-train/repos.yml, so hand-copiedversion-file/publish-pathscan’t drift narrower than the train (the failure mode where published paths stop being watched).load_release_trainnow returns per-repoversion_fileandpublish_pathsfromrepos.yml;version_bump_input_findingsflags mismatches (narrower vs widerpublish-paths, wrongversion-file, missing paths, andexclude-pathsthat hollow out a published glob). Zero-pair guards fail the audit ifversion_filedisappears fromrepos.ymlwhile the inventory still requires the gate, or per-repo if required but noversion_fileentry exists.Inventory/docs:
repo-inventory.ymlpinssoft-fail: falseforversion-bump-gatecallers (not covered by the repos.yml cross-check); shared exempt text no longer hard-codes repo counts.version-bump-gate.ymlheader drops the stale per-repo table and points reviewers at this audit.Selftests add broad coverage for the new comparison logic and
load_release_trainbehavior.Reviewed by Cursor Bugbot for commit 908a147. Bugbot is set up for automated code reviews on this repo. Configure here.