Skip to content

fix(caller-drift): derive version-bump-gate inputs from repos.yml, not the caller (backend#2953) - #447

Open
saqlainsyed007 wants to merge 4 commits into
developfrom
fix/2953-derive-version-bump-inputs-from-repos-yml
Open

fix(caller-drift): derive version-bump-gate inputs from repos.yml, not the caller (backend#2953)#447
saqlainsyed007 wants to merge 4 commits into
developfrom
fix/2953-derive-version-bump-inputs-from-repos-yml

Conversation

@saqlainsyed007

@saqlainsyed007 saqlainsyed007 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

The class

version-bump-gate.yml's header says version_file/publish_paths live in release-train/repos.yml — then the caller in each version_file repo 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). The caller-drift audit already runs under an org-wide-read PAT and already reads repos.yml for train membership, so the cross-check belongs there — the env-vocabulary-agreement.sh shape the issue names, holding no copy of its own:

  • 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 the derived values. NARROWER (unsafe — under-watches) and WIDER (nag) are distinct findings; version-file drift is caught on its own; exclude-paths (backend#2758) and soft-fail are deliberately not compared (no repos.yml counterpart).
  • the audit loop runs it for every repo repos.yml gives a version_file, keyed off the source rather than the inventory's required/exempt flag, counted in the callers family.

Statements this makes false, updated in the same PR

  • version-bump-gate.yml header: dropped the stale hand-copied per-repo table (itself drifted) and pointed at the guard.
  • repo-inventory.yml shared 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 required gate: the fleet audit is red on eight pre-existing drifts unrelated to this PR (stale dsv2 exemptions, .github staging not requiring selftests, client-runtime strict, rfcs conversation resolution), and because this PR changes the audit contract, gate fails 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-drift selftest 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), and load_release_train's carry-through / fail-closed paths.
  • version-bump-gate selftest 58/0 (script body untouched; header-only change).
  • ruff --select E4,E7,E9,F (CI's selection), actionlint, and reason-citations all clean.
  • Live proof against the fleet: flags design-system-v2 (src/*, omits tokens/*), 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-gate caller to release-train/repos.yml, so hand-copied version-file / publish-paths can’t drift narrower than the train (the failure mode where published paths stop being watched).

load_release_train now returns per-repo version_file and publish_paths from repos.yml; version_bump_input_findings flags mismatches (narrower vs wider publish-paths, wrong version-file, missing paths, and exclude-paths that hollow out a published glob). Zero-pair guards fail the audit if version_file disappears from repos.yml while the inventory still requires the gate, or per-repo if required but no version_file entry exists.

Inventory/docs: repo-inventory.yml pins soft-fail: false for version-bump-gate callers (not covered by the repos.yml cross-check); shared exempt text no longer hard-codes repo counts. version-bump-gate.yml header drops the stale per-repo table and points reviewers at this audit.

Selftests add broad coverage for the new comparison logic and load_release_train behavior.

Reviewed by Cursor Bugbot for commit 908a147. Bugbot is set up for automated code reviews on this repo. Configure here.

…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>
@saqlainsyed007

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@saqlainsyed007

Copy link
Copy Markdown
Contributor Author

CI triage (head cb7210e)

Green: all selftests (caller-drift 210/0, and the mutation suite), quality/ruff, actionlint, reason-citations, closing-ref, and both Bugbots (no findings).

Red: audit (caller-drift on this head) and, because of it, the required gate (Conformance gate — a contract change must merge on a clean audit). The audit reports 9 findings; here is every one triaged:

# 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 LukasWodka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Zero pairs must not pass (load_release_train / the wiring at ~L2441). If repos.yml yields no entry with version_file (key renamed, dropped fleet-wide), the check runs on nobody and the audit is green — reproduced with a stub repos.yml using versionFile:. die() when no train entry carries version_file while the inventory marks any version-bump-gate.yml caller required, and flag an inventory-required row whose repos.yml entry has no version_file (the version_gate_not_applicable reason asserts that correspondence in prose; make it a check).

  2. Assert the specific refusal (selftest L2054, L2068). die() always exits 2, and the stub's misroute path (GhError(500)die) also exits 2 — renaming TRAIN_FILE shows both fail-closed tests pass for the wrong reason. Capture stderr and assert the declares version_filebut no usablepublish_paths`` message.

  3. A malformed train entry should be a per-repo finding, not a whole-audit abort. repos.yml itself says omitting publish_paths is the legal strict mode ("Omit the key to keep the strict behaviour"), so version_file without publish_paths is a reachable, sanctioned state — and the new die() 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 the unreadable bucket shape) and emit one finding for that repo; the run stays red, the rest of the report survives.

  4. exclude-paths can reproduce the exact drift this guard catches. In the gate, is_excluded runs per file inside the for pat in $PUBLISH_PATHS loop (L662), so publish-paths: "src/* tokens/*" + exclude-paths: "tokens/*" watches exactly what publish-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.

  5. soft-fail is owned by nobody. The selftest (L1998) says "the inventory's caller_inputs floor owns it", but caller_inputs at head has only a code-quality.yml row. A caller that drops soft-fail: false reverts the gate to advisory with no finding anywhere. Add version-bump-gate.yml: {soft-fail: false} to caller_inputs (all six callers already pass it, so it lands green), or drop the claim.

  6. Two implementations of one rule. release-train/scripts/publish-inventory-check.sh family 3 (release-train#147, backend#2953) already compares each caller's publish-paths and version-file against repos.yml on 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>
@saqlainsyed007

Copy link
Copy Markdown
Contributor Author

bugbot run

@saqlainsyed007

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review, and for separating the CI-red (fleet drift, waits on #443) from the code. Pushed eef2edc addressing three of the six:

  • 5 — soft-fail ownership. Added version-bump-gate.yml: {soft-fail: false} to caller_inputs, so the "the inventory's caller_inputs floor owns it" claim is now true. Matched your measurement (all six pass it).
  • 2 — the refusal message. Both load_release_train fail-closed tests now capture stderr and assert no usable publish_paths, so a misroute-to-die or a renamed TRAIN_FILE no longer passes them for the wrong reason.
  • 6 — the other implementation. Cross-referenced release-train/scripts/publish-inventory-check.sh (family 3) in load_release_train's docstring: this is the PR-time gate, that is the weekly train-side sweep of the same invariant; neither is a second authority (repos.yml is the single source), and a change to "agreement" belongs in both.

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 die. The code here (load_release_train, the comment at the die) takes the stance that version_file without publish_paths is a malformed source — "both are required: true on the reusable, so an absent one is a source error." Your note says repos.yml itself sanctions omitting publish_paths ("Omit the key to keep the strict behaviour"), i.e. it's a legal strict mode, and so the die-before-any-repo-is-read is wrong. Those two readings are in direct conflict and I can't settle it without release-train's repos.yml in front of me. Can you confirm the repos.yml semantics (quote the surrounding lines)? If omitted publish_paths is legal-strict, I'll convert the die to a per-repo finding recorded on the entry via the existing RepoRead/unreadable bucket — the run stays red on that repo, the other 19 survive — and re-point the ask-2 assertion at that finding's message. If it's genuinely malformed, the die is correct and the ask dissolves. I don't want to guess and turn a legal config into a whole-fleet abort, or vice-versa.

1 — zero pairs must not pass. Agreed this is a real hole. I'll die when no train entry carries a version_file while the inventory marks any version-bump-gate.yml caller required, and flag an inventory-required row whose repos.yml entry has no version_file. Holding it in the same push as (3) since both live at the load_release_train seam and I don't want two conflicting failure-mode changes landing separately.

4 — exclude-paths drift. Also real: exclude-paths: "tokens/*" on publish-paths: "src/* tokens/*" reproduces the NARROWER effect the guard exists to catch, and it's currently not compared at all (deliberately, since repos.yml has no counterpart). I'll flag the specific case where an exclude glob equals a repos.yml publish glob (a whole published tree excluded), which is the unambiguous reproduction — rather than the general "does this exclude name files that ship" claim, which needs the ecosystem manifests you note are out of scope.

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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@LukasWodka LukasWodka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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: with TRAIN_FILE renamed, the misroute path's stderr no longer satisfies it, and the selftest reddens.
  • 5caller_inputs now carries version-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 pass soft-fail: false, so it lands green.
  • 6 — the load_release_train cross-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:

  1. Zero pairs. Reproduced again on this head: a repos.yml with the key renamed (versionFile:) returns without dying and the wiring predicate runs on nobody. die() when no train entry carries version_file while the inventory marks any version-bump-gate.yml caller required; flag an inventory-required row whose train entry has no version_file.
  2. exclude-paths hollowing. 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>
@saqlainsyed007

Copy link
Copy Markdown
Contributor Author

Addressed 1, 3, 4 and the body in 6a1847b — thank you for reading release-train's repos.yml to settle 3.

3 — strict mode is now a per-repo finding, not a whole-audit die. load_release_train carries a version_file-with-no-publish_paths entry through (publish_paths None) instead of dying; version_bump_input_findings reports it as a per-repo finding ("…no usable publish_paths … its sanctioned strict mode. Add publish_paths … or exempt the caller"), so that one repo reddens and the other 19 survive. The finding fires with or without a caller — the source is the problem. The empty-string case takes the same path. A non-string/empty version_file (genuinely malformed) still fails closed.

Ask-2 assertions re-pointed. The two tests now assert the finding text ("strict mode", "no usable publish_paths") from version_bump_input_findings, and a separate test still pins the surviving version_file die with redirect_stderr + the specific message, so neither passes on a bare exit 2.

1 — zero pairs, both halves. Extracted to pure helpers so they're unit-tested, not asserted structurally:

  • zero_pair_die_message(train, inventory_repos)die when no entry carries version_file and the inventory marks version-bump-gate.yml required somewhere (your versionFile:-rename repro); None for a train-free fleet or one that still carries a version_file.
  • version_bump_missing_pair_finding(name, train_entry, caller_state) → per-repo finding when the caller is required but the entry has no version_file; None otherwise.

4 — exclude hollowing. An exclude-paths glob equal to a repos.yml publish glob is now a finding ("…hollows the gate…"), reproduced on your src/* tokens/* + exclude-paths: tokens/* case. exclude-paths is otherwise still not compared.

Body + optional pin. Merge-order section updated: dsv2#313 has merged; #443 is the real gate dependency. Added the optional assertion that the real inventory carries caller_inputs["version-bump-gate.yml"]["soft-fail"] is False.

Selftests 221/0 and 58/0, ruff clean. Re-requesting — merge still correctly gated on gate after #443.

@saqlainsyed007

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Comment thread scripts/caller-drift.py
… (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>
@saqlainsyed007

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 LukasWodka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants