ci(contract): catch a stale client-runtime pin, which nothing did

[LukasWodka]

Why

The envelope-contract drift job compares this repo's vendored contracts against the pinned client-runtime ref. That's deliberate — an unrelated upstream commit must not redden every open PR here — but it means the job cannot see the pin itself going stale. It compares v2 against v2 and agrees.

This is measured, not hypothetical:

• client-runtime#376 merged 2026-08-24, and scripts/.client-runtime-ref carried a note to re-point once it did.
• That note went unactioned for 2.5 weeks, during which client-runtime#544 landed envelope contract v3.
• This repo went on verifying both installers against v2 the whole time — with the drift job green.

The pin comment claimed "the weekly run is what catches a pin gone stale enough to matter." No job compared the pin against the producer's default branch, so nothing caught it. A comment asserting a property the code did not have.

What this adds

pin-staleness — fetches envelope_contract.json at the pinned ref and on client-runtime's default branch, and compares them.

• Fails closed on both reads. A fetch that didn't happen is not evidence the pin is fresh, and two unknowns must never compare equal.
• Warns on a pull_request, fails on the weekly/dispatch run. Failing a PR would punish whoever happens to touch an installer line for a pin somebody else left behind — which is how a whole tier gets skipped. The scheduled run is where the alarm is actionable and blocks nobody.
• The failure message names every surface adoption touches, because that isn't obvious: re-vendoring the JSON is not enough — gen-envelope-embed.sh also rewrites the bash vectors and the embedded literals in scripts/lib/install-client-helm.sh and scripts/install-k8s.ps1. Four surfaces, one bump.

scripts/contract-summary.py — prints <version>|<cpu>m / <mem> MiB for a contract file. It exists because my first version of this job inlined the reader as a heredoc inside a YAML block scalar, which broke the block's indentation and is unreadable even when it parses. stdlib only, and it raises rather than degrading to unknown on a file it cannot read.

Verified against the real refs, both directions:

pin	reads	verdict
e4c4813 (current)	v2 — 1000m / 3072 MiB	vs develop's v3 — 900m / 3136 MiB → STALE, guard fires
develop	v3	identical → silent

What this deliberately does NOT do: adopt v3

v3's overhead is the control-plane footprint of chart 1.9.109. The interim trim moved that at 1.9.112 — the chart now renders 2272 MiB / 650 m. Adopting v3 alone would shrink the smallest envelope and re-break the 2-core / 8 GiB node that install_client_helm: the ticket's 8 GiB node now FITS unreduced pins:

5120 + 3136 = 8256 MiB  >  8192 MiB     (v3 — no longer fits)
5120 + 3072 = 8192 MiB  =  8192 MiB     (v2 — fits exactly)
5120 + 2272 = 7392 MiB  <  8192 MiB     (post-trim — fits comfortably)

I tried the adoption first and backed it out: it reddens 11 expectations here and 16 upstream, all hand-written independent pins, and the interim trim's own PR says the requests were never sized from usage (1050m / 3276 Mi requested against 36m / ~1022 Mi used). So the number is still moving, and baking it in now means redoing 27 pins twice.

Adoption waits for a contract regenerated against the chart as it actually renders. This change makes the gap visible in the meantime, which is the part that was missing.

Test plan

• envelope-contract-drift.yml parses; both jobs enumerate (pin-staleness, envelope-contract)
• the guard's logic run locally against the real refs, in both the stale and current directions (table above)
• contract-summary.py: black + ruff clean; raises on a non-integer contract_version and on a missing file rather than printing unknown
• scripts/gen-manifest.sh --check — up to date, and unchanged: the manifest's surface is the set the bootstrap fetches, and CI tooling is not in it

---

Note

Low Risk
CI and documentation only; no installer or contract adoption in this PR. Touches cross-repo GitHub API reads and secret-scoped App tokens in the workflow.

Overview
Adds a pin-staleness CI job so a stale scripts/.client-runtime-ref is no longer invisible while the existing drift job stays green (it only compares vendored files to the pinned upstream ref).

The job fetches envelope_contract.json and log_redactions.json at the pin and on client-runtime’s default branch (via a read-only App token), fails closed on unreadable API responses, and warns on pull_request vs errors on schedule/dispatch when upstream has moved. Comments in the workflow and scripts/.client-runtime-ref drop the incorrect claim that the weekly drift run alone catches a stale pin.

scripts/contract-summary.py is a stdlib helper that prints envelope version and overhead for log output; it errors on bad input instead of silently reporting “unknown.”

^{Reviewed by Cursor Bugbot for commit 8dd5042. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

bugbot run

[cursor]

Stale Bugbot comment from a previous run.

[LukasWodka]

bugbot run

[cursor]

Stale Bugbot comment from a previous run.

[LukasWodka]

bugbot run

[cursor]

Stale Bugbot comment from a previous run.

[LukasWodka]

bugbot run

[saqlainsyed007]

Re-checked at b9983ca4 — none of my five comments below have been addressed yet (no code change, no reply). Re-flagging so they don't get lost under the resolved Bugbot threads:

1. scripts/.client-runtime-ref line 7 — the false claim "the weekly run is what catches a pin gone stale enough to matter" is removed here, but an identical copy of that sentence is still sitting in .github/workflows/envelope-contract-drift.yml's header comment (# weekly run is what catches a pin gone stale enough to matter.). The PR's own thesis makes that line false too — please fix both instances in this PR, not just one.

2. envelope-contract-drift.yml line 145 — the comment claims CONTRACTS is a "left half is the path in client-runtime, right half is where this repo vendors it" mapping, but the value ("envelope_contract.json log_redactions.json") is a flat list — both entries are fetched from client-runtime, neither is a vendored-in-this-repo path. This is exactly the "comment asserting a property the code doesn't have" pattern the PR exists to eliminate. Please drop the left/right framing or implement it.

3. envelope-contract-drift.yml line 163 — every fail-closed exit 1 (empty ref, invalid ref shape, unreadable pinned/develop contract) fires unconditionally, before the EVENT = pull_request branch further down. That means a fork PR (no secrets → token mint fails) or a transient client-runtime API hiccup reddens the PR outright — the exact outcome the "warns on a PR, fails on the schedule" design was written to avoid. Please route read/validation failures through the same warn-on-PR gate as the staleness verdict, or explicitly narrow the stated guarantee.

4. envelope-contract-drift.yml line 160 — /tmp/pinned-$path / /tmp/head-$path only works because both current contract paths are flat filenames. Add a third contract that lives under a subdirectory and the redirect target's parent won't exist, aborting the job under set -e — contradicting the "adding a third contract is one line" comment. Sanitize the temp filename (e.g. ${path//\//_}).

5. envelope-contract-drift.yml line 162 — the develop-branch comparison target is hardcoded to the literal develop, but the job/PR describe it as comparing against client-runtime's default branch. They coincide today, so not a live bug, but if client-runtime's default branch is ever renamed this silently 404s. Consider deriving it or naming the coupling explicitly in the comment.

[saqlainsyed007]

Strong guard, and mutation-proof where it counts: this PR's own pin-staleness run was fed the genuinely-stale current pin and fired the STALE path (envelope v2 vs develop v3, DIFFERS, ::warning::PIN STALE), while correctly reporting log_redactions.json matches — so it discriminates per-contract and compares against upstream develop, not itself. CI is fully green (mutation-check, both Bugbots, drift all pass), the 5 Bugbot threads are all resolved with fixes visible in the diff, and the manifest claim checks out (CI-only files, drift green).

Two non-blocking self-consistency fixes, worth doing precisely because rule 7 is this PR's whole thesis:

1. The false claim you delete from scripts/.client-runtime-ref — "the weekly run is what catches a pin gone stale enough to matter" — still sits verbatim in the header of the workflow you're editing (.github/workflows/envelope-contract-drift.yml, ~line 24). The fix is half-applied; the same rule-7 assertion survives one comment over.

2. The CONTRACTS comment says "Left half is the path in client-runtime, right half is where this repo vendors it," but the value is a flat two-path list of producer paths — there's no left/right pairing. That's a comment asserting a structure the code doesn't have.

Both are one-line comment edits. Findings 3–5 (cannot-read reddens a PR; /tmp/pinned-$path slash-safety; hardcoded develop vs "default branch") are inline as design/latent notes, not blockers. No approval yet — happy to approve once 1–2 are tidied.

[LukasWodka]

bugbot run

[LukasWodka]

All five addressed in 8dd50421, replied per-thread and resolved. Summary so you don't have to open five threads:

#	finding	what changed
1	false claim survives in the workflow header	deleted there too, with the same correction scripts/.client-runtime-ref got
2	CONTRACTS left/right framing over a flat list	dropped the framing; described as the flat list of producer paths it is
3	fail-closed exits redden a PR	read failures route through cannot_tell() — narrowed, see below
4	/tmp/pinned-$path breaks on a nested path	safe="${path//\//_}"; summary derives its name through the same sanitiser
5	hardcoded develop vs "default branch"	derived from the producer

One deliberate deviation, on finding 3. I did not route the ref checks through the warn gate — only the reads. An empty or malformed ref is this PR's own diff (scripts/.client-runtime-ref is in this repo and a PR can change it), so warning there would let a PR land a broken pin behind a message nobody reads. Your finding listed the ref checks alongside the reads, but both harms you named were reads, so I read the split as what you were after. Happy to warn on those too if you disagree.

Two things proved rather than asserted:

Finding 4 — old vs new step, same stub, with a third contract added under a subdirectory:

OLD  exit=1  /tmp/pinned-schemas/nested_contract.json: No such file or directory
NEW  exit=0  schemas/nested_contract.json: pinned copy matches develop

Your predicted failure reproduces exactly.

Finding 3 — 8 stubbed cases pin the split: read-fail on a PR → exit 0 + ::warning::; read-fail on schedule → exit 1 + ::error::; malformed ref (../../etc/passwd) and empty ref on a PR → exit 1; clean run → exit 0.

Also ran: yaml parse, bash -n, shellcheck -S warning, actionlint — all clean.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 8dd5042. Configure here.}

[cursor]

Token step still fails pull requests

Medium Severity

The create-github-app-token step has no continue-on-error or outcome check, so a fork or Dependabot pull_request (empty RELEASE_TRAIN_APP_* secrets) or an App outage fails the job before cannot_tell runs. That still reddens the PR instead of warning, which is the outcome the job's comments say they avoided.

Additional Locations (1)

• .github/workflows/envelope-contract-drift.yml#L166-L185

 

^{Reviewed by Cursor Bugbot for commit 8dd5042. Configure here.}

[saqlainsyed007]

Four of the five points are cleanly fixed and well-verified — thanks for the thorough replies and the mutation-style proofs on each (temp-filename sanitization, both-contracts check, derived default branch, the duplicate false comment).

One is still open, and it's the sharpest of the five. The cannot_tell() gate correctly routes the read failures (contract fetch, default-branch fetch) through warn-on-PR/fail-on-schedule — good, and the choice to keep ref-shape validation hard-failing (since that's this PR's own diff surface) is reasonable and well-argued. But the token-minting step (create-github-app-token, minting the client-runtime reader) has no continue-on-error or outcome check ahead of it. On a fork PR — this repo is public — or during an App outage, that step fails outright and the job dies before cannot_tell() ever runs. That's exactly the fork-PR case point 3 named, just one step earlier than where the fix landed.

Cursor's own new finding on this head (.github/workflows/envelope-contract-drift.yml#L121, "Token step still fails pull requests") says the same thing — please address that before this is mergeable, either with continue-on-error: true + an explicit outcome check feeding cannot_tell(), or an equivalent guard around the token step.

Everything else looks good to me.

[saqlainsyed007]

Re-reviewed at 8dd5042 — both of my rule-7 self-consistency nits are addressed, thanks:

• The false "the weekly run is what catches a pin gone stale enough to matter" claim in envelope-contract-drift.yml's header is gone — the comment now explicitly names it as the half-applied fix and points at pin-staleness as what actually catches a stale pin.
• The CONTRACTS comment now correctly describes a flat list of producer paths ("neither is a path in this repo"), matching the value.

I'm satisfied on my findings. The one thing still holding an approve is the open Cursor/Bugbot thread at envelope-contract-drift.yml:121, and I verified it's on-point: the workflow triggers on pull_request to develop/main, and the Mint a read-only installation token step has no continue-on-error. On a fork PR (which, as your own comment at ~line 167 notes, mints no token) the create-github-app-token step fails, and with no continue-on-error the job reds there — before the cannot_tell() warn-on-PR logic you built ever runs. That's the exact "reddening an unrelated PR" outcome the split was written to avoid, defeated one step up.

Fix would be continue-on-error: true on the token step plus treating an empty steps.token.outputs.token downstream as a cannot_tell (warn on PR, fail on schedule) — or, if fork PRs can't reach this workflow in practice, resolving the Bugbot thread with why. Happy to approve once that thread is closed.