From ca9084dbc1906e296055b978fee28544db6657ae Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 11 Sep 2026 08:14:01 +0200 Subject: [PATCH 1/5] ci(contract): catch a stale client-runtime pin, which nothing did (backend#2460) The envelope-contract drift job compares the vendored contracts against the PINNED client-runtime ref. That is 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. Measured, not hypothetical. client-runtime#376 merged 2026-08-24 and the pin comment 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 -- and this repo went on verifying both installers against v2, with the drift job GREEN throughout. 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. So this adds the check the comment described: * `pin-staleness` fetches envelope_contract.json at the pinned ref AND on client-runtime's default branch and compares them. Both reads FAIL CLOSED -- a fetch that did not happen is not evidence the pin is fresh, and two unknowns must never compare equal; * it WARNS on a pull_request and 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 tier gets skipped. The scheduled run is where the alarm is actionable and blocks nobody; * the failure message names every surface adoption has to touch, because that was not 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. `scripts/contract-summary.py` prints `|m / MiB` for a contract file. It exists because the 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: pinned e4c4813 reads v2 (1000m / 3072 MiB) against develop's v3 (900m / 3136 MiB) -> STALE, guard fires; with the pin set to develop -> silent. WHAT THIS DELIBERATELY DOES NOT DO: adopt v3. v3's overhead is the control-plane footprint of chart 1.9.109, and 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 "the ticket's 8 GiB node now FITS unreduced" pins -- 5120 + 3136 > 8192 MiB. Adoption waits for a contract regenerated against the chart as it actually renders, and the trim's own PR says the requests were never sized from usage (1050m / 3276 Mi requested against 36m / ~1022 Mi used), so that number is still moving. This change makes the gap VISIBLE in the meantime, which is the part that was missing. scripts/manifest.sha256 needs no update: the manifest's surface is the set the bootstrap fetches, and CI tooling is not in it. `--check` confirms it is current. Co-Authored-By: Claude Opus 5 --- .github/workflows/envelope-contract-drift.yml | 64 +++++++++++++++++++ scripts/.client-runtime-ref | 12 +++- scripts/contract-summary.py | 55 ++++++++++++++++ 3 files changed, 129 insertions(+), 2 deletions(-) create mode 100755 scripts/contract-summary.py diff --git a/.github/workflows/envelope-contract-drift.yml b/.github/workflows/envelope-contract-drift.yml index 03867e7a..fdfc2a31 100644 --- a/.github/workflows/envelope-contract-drift.yml +++ b/.github/workflows/envelope-contract-drift.yml @@ -65,6 +65,70 @@ permissions: contents: read jobs: + # ── IS THE PIN ITSELF STALE? (backend#2460) ──────────────────────────────── + # The job below compares the vendored copies against the PINNED ref, which is + # the whole point of pinning: an unrelated client-runtime commit must not + # redden every open client PR. The cost is that it cannot see the pin going + # stale -- it compares v2 against v2 and agrees. + # + # MEASURED, not hypothetical. client-runtime#376 merged 2026-08-24 and the pin + # comment carried a note to re-point once it did. That went unactioned for 2.5 + # weeks, during which client-runtime#544 landed envelope contract v3 -- and + # this repo went on verifying its installers against v2, with the drift job + # GREEN the whole time. The pin comment claimed "the weekly run is what catches + # a pin gone stale enough to matter"; nothing compared the pin against the + # producer's default branch, so nothing caught it (CLAUDE.md rule 7: a comment + # asserting a property the code does not have). + # + # WARNS ON A PR, FAILS ON THE SCHEDULE. Failing a PR would punish whoever + # happens to touch an installer line for a pin someone else left behind, which + # is how a whole tier gets skipped (rule 4). The weekly and dispatch runs are + # where the alarm is actionable and blocks nobody. + pin-staleness: + name: pinned contract vs client-runtime's default branch + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + + - name: Compare the pinned contract against the producer's default branch + env: + EVENT: ${{ github.event_name }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + set -euo pipefail + ref="$(grep -vE '^[[:space:]]*(#|$)' scripts/.client-runtime-ref | head -1 | tr -d '[:space:]')" + [ -n "$ref" ] || { echo "::error::scripts/.client-runtime-ref names no ref"; exit 1; } + + # FAIL CLOSED on both reads: a fetch I could not do is not evidence the + # pin is fresh, and two unknowns must never compare equal (rule 3). + fetch() { + curl -fsSL -H "Authorization: Bearer $GH_TOKEN" \ + -H "Accept: application/vnd.github.raw" \ + "https://api.github.com/repos/tracebloc/client-runtime/contents/envelope_contract.json?ref=$1" + } + fetch "$ref" > /tmp/pinned.json \ + || { echo "::error::cannot read envelope_contract.json at the pinned ref $ref"; exit 1; } + fetch develop > /tmp/head.json \ + || { echo "::error::cannot read envelope_contract.json on client-runtime develop -- cannot tell whether the pin is stale"; exit 1; } + + pinned="$(python3 scripts/contract-summary.py /tmp/pinned.json)" + head="$(python3 scripts/contract-summary.py /tmp/head.json)" + echo "pinned ($ref): contract v${pinned%%|*} overhead ${pinned#*|}" + echo "develop : contract v${head%%|*} overhead ${head#*|}" + + if cmp -s /tmp/pinned.json /tmp/head.json; then + echo "the pin is current." + exit 0 + fi + + msg="this repo vendors envelope contract v${pinned%%|*} (${pinned#*|}) while client-runtime develop ships v${head%%|*} (${head#*|}). To adopt: bump scripts/.client-runtime-ref, re-vendor BOTH contracts, re-run scripts/gen-envelope-embed.sh (it rewrites the bash vectors AND the embedded literals in scripts/lib/install-client-helm.sh and scripts/install-k8s.ps1), then update the hand-written expectations the new numbers move." + if [ "$EVENT" = "pull_request" ]; then + echo "::warning::PIN STALE (not this PR's doing) -- $msg" + exit 0 + fi + echo "::error::PIN STALE -- $msg" + exit 1 + envelope-contract: timeout-minutes: 10 runs-on: ubuntu-latest diff --git a/scripts/.client-runtime-ref b/scripts/.client-runtime-ref index d686d09f..a96a713e 100644 --- a/scripts/.client-runtime-ref +++ b/scripts/.client-runtime-ref @@ -2,8 +2,16 @@ # envelope_contract.json is vendored from (backend#2220). # # Pin, don't float: an unrelated client-runtime commit must not redden every -# open client PR. Same rule as cli/scripts/.client-ref. The weekly run is what -# catches a pin gone stale enough to matter. +# open client PR. Same rule as cli/scripts/.client-ref. +# +# WHAT CATCHES A STALE PIN is the `pin-staleness` job in +# .github/workflows/envelope-contract-drift.yml -- it compares this ref's +# contract against client-runtime's DEFAULT BRANCH, warns on a PR and fails on +# the weekly/dispatch run. This comment used to say "the weekly run is what +# catches a pin gone stale enough to matter", and no job did that: the drift job +# checks out THIS ref, so it compared the vendored copy against itself and +# agreed. A pin sat 2.5 weeks behind contract v3 under a green gate +# (backend#2460). # # ONE REF, TWO CONTRACTS since backend#2378. This pin governs both vendored # copies -- scripts/tests/fixtures/envelope_contract.json and diff --git a/scripts/contract-summary.py b/scripts/contract-summary.py new file mode 100755 index 00000000..960726df --- /dev/null +++ b/scripts/contract-summary.py @@ -0,0 +1,55 @@ +#!/usr/bin/env python3 +"""Print `|m / MiB` for an envelope contract. + +Exists so the pin-staleness job in envelope-contract-drift.yml can compare two +copies of client-runtime/envelope_contract.json without a nested heredoc inside +a YAML block scalar -- which is how the first attempt at that job broke, and is +unreadable even when it works. + +FAILS LOUDLY on anything it cannot read. A summary that silently degrades to +"unknown" would let the comparison report two unknowns as equal, which is the +"cannot tell reads as agreement" trap (CLAUDE.md rule 3). + +stdlib only: it runs on a bare runner before any pip install. +""" + +from __future__ import annotations + +import json +import sys + +MIB = 1024 * 1024 + + +def summarise(path: str) -> str: + with open(path, encoding="utf-8") as handle: + document = json.load(handle) + version = document["contract_version"] + overhead = document["overhead"] + cpu = overhead["cpu_millicores"] + memory = overhead["memory_bytes"] + if ( + not isinstance(version, int) + or not isinstance(cpu, int) + or not isinstance(memory, int) + ): + raise TypeError( + f"{path}: contract_version, overhead.cpu_millicores and " + f"overhead.memory_bytes must all be integers" + ) + return f"{version}|{cpu}m / {memory // MIB} MiB" + + +def main(argv: list[str]) -> int: + if len(argv) != 2: + print( + f"usage: {argv[0] if argv else 'contract-summary.py'} ", + file=sys.stderr, + ) + return 2 + print(summarise(argv[1])) + return 0 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) From 8b1c6f57da39a75e202eb088bfd61469e9d6e40e Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 11 Sep 2026 08:47:32 +0200 Subject: [PATCH 2/5] ci(contract): mint an App token for the cross-repo read, and bound it (backend#2460) Both Bugbot findings, both right. HIGH: `pin-staleness` read the PRIVATE tracebloc/client-runtime with GITHUB_TOKEN, which is scoped to THIS repo and cannot see a sibling. So both Contents reads failed, the fail-closed branch fired, and the job went red with a cannot-read error on every run instead of comparing pins -- visible on this PR own first run. The warn-on-PR path could never even be reached. The fix was already in this file. The envelope-contract job twenty lines below mints an App installation token and its comment says exactly this: "GITHUB_TOKEN is scoped to THIS repo and cannot read another private one". I added a second cross-repo reader without reading the first. Same App, same least privilege -- named `repositories`, contents:read only. MEDIUM: the curl calls had no bounds and the job no timeout-minutes, so a stalled api.github.com would hang to the Actions six-hour cap -- a cannot-tell that never fails. Now --connect-timeout 10 --max-time 60 --retry 2 per read, and timeout-minutes: 10 on the job, so a read that cannot finish fails rather than hangs. The other reds on the previous run were not this PR: the opensuse prereq job failed on distro-mirror connectivity and says so itself ("not this PR. Re-run this job"), and the cancelled Helm runs were superseded by later pushes. Co-Authored-By: Claude Opus 5 --- .github/workflows/envelope-contract-drift.yml | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/.github/workflows/envelope-contract-drift.yml b/.github/workflows/envelope-contract-drift.yml index fdfc2a31..e238a789 100644 --- a/.github/workflows/envelope-contract-drift.yml +++ b/.github/workflows/envelope-contract-drift.yml @@ -87,13 +87,36 @@ jobs: pin-staleness: name: pinned contract vs client-runtime's default branch runs-on: ubuntu-latest + # A stalled api.github.com must not hold a runner to the six-hour cap; a + # read that cannot finish is a cannot-tell, and cannot-tell has to FAIL, not + # hang (Bugbot Medium). The curl calls carry their own bounds too, so a hang + # inside one read fails that read rather than the whole job. + timeout-minutes: 10 steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + - name: Mint a read-only installation token for client-runtime + id: token + # GITHUB_TOKEN is scoped to THIS repo and cannot read another private + # one -- the job below read tracebloc/client-runtime with it and so + # failed closed on EVERY run, turning a pin comparison into a + # cannot-read error (Bugbot High, and it was right: this PR's own run + # went red that way). The envelope-contract job twenty lines down + # already solved this and says so in its comment; I added a second + # cross-repo reader without reading the first. Same App, same least + # privilege: named `repositories`, contents:read only. + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + app-id: ${{ secrets.RELEASE_TRAIN_APP_ID }} + private-key: ${{ secrets.RELEASE_TRAIN_APP_PRIVATE_KEY }} + owner: ${{ github.repository_owner }} + repositories: client-runtime + permission-contents: read + - name: Compare the pinned contract against the producer's default branch env: EVENT: ${{ github.event_name }} - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_TOKEN: ${{ steps.token.outputs.token }} run: | set -euo pipefail ref="$(grep -vE '^[[:space:]]*(#|$)' scripts/.client-runtime-ref | head -1 | tr -d '[:space:]')" @@ -102,7 +125,8 @@ jobs: # FAIL CLOSED on both reads: a fetch I could not do is not evidence the # pin is fresh, and two unknowns must never compare equal (rule 3). fetch() { - curl -fsSL -H "Authorization: Bearer $GH_TOKEN" \ + curl -fsSL --connect-timeout 10 --max-time 60 --retry 2 \ + -H "Authorization: Bearer $GH_TOKEN" \ -H "Accept: application/vnd.github.raw" \ "https://api.github.com/repos/tracebloc/client-runtime/contents/envelope_contract.json?ref=$1" } From e7222b214c7e1307c5af9009226d45c551e39d5c Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 11 Sep 2026 08:58:10 +0200 Subject: [PATCH 3/5] ci(contract): the stale-pin check covers BOTH contracts the pin governs (backend#2460) Bugbot, and it is the sharpest finding on this PR: the guard reproduced the defect it exists to close, one file over. `scripts/.client-runtime-ref` governs TWO vendored contracts and says so in capitals -- "ONE REF, TWO CONTRACTS since backend#2378 ... Bump it, and re-vendor BOTH". The first version of `pin-staleness` compared only envelope_contract.json and then printed "the pin is current". An upstream-only change to log_redactions.json would have left the pin stale with this new guard reporting GREEN -- exactly the silent green it was written to end. Checking one of two and reporting on both is worse than not checking, because it looks like cover. I quoted that "ONE REF, TWO CONTRACTS" comment earlier in this same change while choosing which ref to pin to, and still wrote a one-file loop. Now both are fetched and compared, the contract list is one variable so a third is one line, each read still fails closed independently, and the message names which contract moved. The envelope contract keeps its version summary in the output because it has one; log_redactions.json is reported by name. Exercised against the real refs, three ways: pin e4c4813 (pre-#544) envelope DIFFERS, redactions match -> STALE pin = develop both match -> current envelope match + redactions DIFFER -> STALE redactions moved (Bugbot's case; the one-file check reported "current" on these inputs) A fourth, unplanned: `ba58303^` predates log_redactions.json existing, so the read 404s and the job refuses with "cannot read" rather than passing. That is the fail-closed path working on a real ref. Co-Authored-By: Claude Opus 5 --- .github/workflows/envelope-contract-drift.yml | 53 ++++++++++++++----- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/.github/workflows/envelope-contract-drift.yml b/.github/workflows/envelope-contract-drift.yml index e238a789..b0b83a73 100644 --- a/.github/workflows/envelope-contract-drift.yml +++ b/.github/workflows/envelope-contract-drift.yml @@ -122,30 +122,57 @@ jobs: ref="$(grep -vE '^[[:space:]]*(#|$)' scripts/.client-runtime-ref | head -1 | tr -d '[:space:]')" [ -n "$ref" ] || { echo "::error::scripts/.client-runtime-ref names no ref"; exit 1; } - # FAIL CLOSED on both reads: a fetch I could not do is not evidence the + # BOTH CONTRACTS, because the pin governs both. scripts/.client-runtime-ref + # says so in capitals -- "ONE REF, TWO CONTRACTS since backend#2378" -- + # and the first version of this job compared only the envelope contract + # and then printed "the pin is current". An upstream-only change to + # log_redactions.json would have left the pin stale with this guard + # reporting green: the exact silent-green gap the job exists to close, + # reproduced one file over (Bugbot Medium). Checking one of two and + # reporting on both is worse than not checking, because it looks like + # cover. + # + # Left half is the path in client-runtime, right half is where this repo + # vendors it -- named together so adding a third contract is one line. + CONTRACTS="envelope_contract.json log_redactions.json" + + # FAIL CLOSED on every read: a fetch I could not do is not evidence the # pin is fresh, and two unknowns must never compare equal (rule 3). fetch() { curl -fsSL --connect-timeout 10 --max-time 60 --retry 2 \ -H "Authorization: Bearer $GH_TOKEN" \ -H "Accept: application/vnd.github.raw" \ - "https://api.github.com/repos/tracebloc/client-runtime/contents/envelope_contract.json?ref=$1" + "https://api.github.com/repos/tracebloc/client-runtime/contents/$1?ref=$2" } - fetch "$ref" > /tmp/pinned.json \ - || { echo "::error::cannot read envelope_contract.json at the pinned ref $ref"; exit 1; } - fetch develop > /tmp/head.json \ - || { echo "::error::cannot read envelope_contract.json on client-runtime develop -- cannot tell whether the pin is stale"; exit 1; } - pinned="$(python3 scripts/contract-summary.py /tmp/pinned.json)" - head="$(python3 scripts/contract-summary.py /tmp/head.json)" - echo "pinned ($ref): contract v${pinned%%|*} overhead ${pinned#*|}" - echo "develop : contract v${head%%|*} overhead ${head#*|}" + stale="" + for path in $CONTRACTS; do + fetch "$path" "$ref" > "/tmp/pinned-$path" \ + || { echo "::error::cannot read $path at the pinned ref $ref"; exit 1; } + fetch "$path" develop > "/tmp/head-$path" \ + || { echo "::error::cannot read $path on client-runtime develop -- cannot tell whether the pin is stale"; exit 1; } + if cmp -s "/tmp/pinned-$path" "/tmp/head-$path"; then + echo "$path: pinned copy matches develop" + else + echo "$path: DIFFERS between the pinned ref and develop" + stale="$stale $path" + fi + done + + # The envelope contract carries a version worth naming in the message; + # log_redactions.json does not, so it is reported by name alone. + if [ -s /tmp/pinned-envelope_contract.json ]; then + pinned="$(python3 scripts/contract-summary.py /tmp/pinned-envelope_contract.json)" + head="$(python3 scripts/contract-summary.py /tmp/head-envelope_contract.json)" + echo "envelope contract: pinned v${pinned%%|*} (${pinned#*|}) vs develop v${head%%|*} (${head#*|})" + fi - if cmp -s /tmp/pinned.json /tmp/head.json; then - echo "the pin is current." + if [ -z "$stale" ]; then + echo "the pin is current for every contract it governs." exit 0 fi - msg="this repo vendors envelope contract v${pinned%%|*} (${pinned#*|}) while client-runtime develop ships v${head%%|*} (${head#*|}). To adopt: bump scripts/.client-runtime-ref, re-vendor BOTH contracts, re-run scripts/gen-envelope-embed.sh (it rewrites the bash vectors AND the embedded literals in scripts/lib/install-client-helm.sh and scripts/install-k8s.ps1), then update the hand-written expectations the new numbers move." + msg="the pin is behind client-runtime develop for:$stale. To adopt: bump scripts/.client-runtime-ref, re-vendor BOTH contracts (scripts/tests/fixtures/envelope_contract.json and client/log_redactions.json), re-run scripts/gen-envelope-embed.sh (it rewrites the bash vectors AND the embedded literals in scripts/lib/install-client-helm.sh and scripts/install-k8s.ps1), then update the hand-written expectations the new numbers move." if [ "$EVENT" = "pull_request" ]; then echo "::warning::PIN STALE (not this PR's doing) -- $msg" exit 0 From b9983ca46b9e59aea7a0c4efcf0d2b06358a8c4d Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 11 Sep 2026 09:17:02 +0200 Subject: [PATCH 4/5] ci(contract): gate the pin's shape, and stop the display line deciding the verdict (backend#2460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more from Bugbot, both right. MEDIUM — unvalidated ref. `pin-staleness` interpolated the pin into a contents API URL with no shape check, while the sibling `envelope-contract` job gates the same value before handing it to checkout. This job is a SECOND place the pin escapes into, so it needs its own gate rather than trusting that the other job ran first. Same expression: no path traversal, no option-looking refs. Exercised: a real SHA and `develop` accepted; `../../etc/passwd`, `--upload-pack=evil`, `a;rm -rf /` and empty refused. MEDIUM — and this one defeated the job's own design. `contract-summary.py` ran under `set -e` AFTER the comparison but BEFORE the warn-on-PR branch. So a develop-side schema change -- exactly the stale case this job exists to surface -- made the display helper raise, aborted the script, and FAILED a pull request that was supposed to warn. The warn path was unreachable in its most important case. The version line is cosmetic: `cmp` already decided, above. It is now `|| true` with an explicit fallback message, so a summary that cannot be produced says so and the verdict stands. The reads that MATTER still fail closed -- an unfetchable contract still exits 1, because that is a cannot-tell about the comparison itself rather than about how to print it. Verified: a contract file with an unrecognised schema yields an empty summary and the script CONTINUES to the verdict, where before `set -e` ended the run. Co-Authored-By: Claude Opus 5 --- .github/workflows/envelope-contract-drift.yml | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/.github/workflows/envelope-contract-drift.yml b/.github/workflows/envelope-contract-drift.yml index b0b83a73..156e55e6 100644 --- a/.github/workflows/envelope-contract-drift.yml +++ b/.github/workflows/envelope-contract-drift.yml @@ -121,6 +121,16 @@ jobs: set -euo pipefail ref="$(grep -vE '^[[:space:]]*(#|$)' scripts/.client-runtime-ref | head -1 | tr -d '[:space:]')" [ -n "$ref" ] || { echo "::error::scripts/.client-runtime-ref names no ref"; exit 1; } + # SAME SHAPE GATE the sibling job applies before it hands the pin to + # checkout -- no path traversal, no option-looking refs. This job + # interpolates the pin into a contents API URL, which is a second + # place the value escapes into, so it needs the same check rather + # than trusting that the other job ran first (Bugbot Medium). + if ! printf '%s' "$ref" | grep -qE '^[A-Za-z0-9][A-Za-z0-9._/-]*$' \ + || printf '%s' "$ref" | grep -q '\.\.'; then + echo "::error file=scripts/.client-runtime-ref::invalid ref shape: $ref" + exit 1 + fi # BOTH CONTRACTS, because the pin governs both. scripts/.client-runtime-ref # says so in capitals -- "ONE REF, TWO CONTRACTS since backend#2378" -- @@ -159,12 +169,21 @@ jobs: fi done - # The envelope contract carries a version worth naming in the message; - # log_redactions.json does not, so it is reported by name alone. + # COSMETIC, AND MUST NOT BE ABLE TO CHANGE THE VERDICT. The version + # line is nicer output; `cmp` above already decided. Under `set -e` an + # unreadable summary used to abort the script BEFORE the warn-on-PR + # branch, so a develop-side schema change -- exactly the stale case + # this job exists to surface -- would have FAILED a pull request that + # was supposed to warn (Bugbot Medium). `|| true` keeps the failure + # cosmetic; the reads that matter already failed closed above. if [ -s /tmp/pinned-envelope_contract.json ]; then - pinned="$(python3 scripts/contract-summary.py /tmp/pinned-envelope_contract.json)" - head="$(python3 scripts/contract-summary.py /tmp/head-envelope_contract.json)" - echo "envelope contract: pinned v${pinned%%|*} (${pinned#*|}) vs develop v${head%%|*} (${head#*|})" + pinned="$(python3 scripts/contract-summary.py /tmp/pinned-envelope_contract.json 2>/dev/null || true)" + head="$(python3 scripts/contract-summary.py /tmp/head-envelope_contract.json 2>/dev/null || true)" + if [ -n "$pinned" ] && [ -n "$head" ]; then + echo "envelope contract: pinned v${pinned%%|*} (${pinned#*|}) vs develop v${head%%|*} (${head#*|})" + else + echo "envelope contract: could not summarise one of the copies (schema change?) -- the comparison above stands regardless" + fi fi if [ -z "$stale" ]; then From 8dd50421f1b5ddd54ea9aaf95feec0a99623209b Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 11 Sep 2026 18:08:01 +0200 Subject: [PATCH 5/5] ci(contract): stop cannot-read reddening a PR, and finish the claim this PR deletes (backend#2460) Five review findings, all of them the same defect this job exists to catch. 1+2 (blocking): the false claim removed from scripts/.client-runtime-ref still sat verbatim in this workflow header, and the CONTRACTS comment described a producer/vendored left-right mapping over a flat list of producer paths. Both were comments asserting properties the code does not have. 3: every fail-closed exit fired before the EVENT branch, so a fork PR (no token) or a transient API hiccup reddened the PR outright -- the outcome warn-on-PR exists to avoid. Read failures now route through cannot_tell(). The split is deliberate: an empty or malformed ref IS the PR diff and still fails hard, since warning there would let a PR land a broken pin behind a message nobody reads. 4: /tmp/pinned-$path only worked because both paths are flat. Proven: with a third contract under a subdirectory the old script aborts under set -e with "/tmp/pinned-schemas/nested_contract.json: No such file or directory"; the sanitised name handles it. 5: the comparison target was the literal develop while the job name and PR say default branch. Now derived from the producer. Verified: yaml parses, bash -n, shellcheck -S warning clean, actionlint clean, and 8 stubbed cases pin the warn/fail split (read-fail PR exit 0 + warning, read-fail schedule exit 1, bad ref exit 1 on a PR, clean run exit 0). Co-Authored-By: Claude Opus 5 --- .github/workflows/envelope-contract-drift.yml | 96 +++++++++++++++---- 1 file changed, 80 insertions(+), 16 deletions(-) diff --git a/.github/workflows/envelope-contract-drift.yml b/.github/workflows/envelope-contract-drift.yml index 156e55e6..77447d17 100644 --- a/.github/workflows/envelope-contract-drift.yml +++ b/.github/workflows/envelope-contract-drift.yml @@ -20,8 +20,15 @@ name: Cross-repo contract drift # -> the bats + Pester replays, which run in the normal test jobs # # Pin, don't float (the rule cli/.github/workflows/chart-drift.yml states): an -# unrelated client-runtime commit must not redden every open client PR. The -# weekly run is what catches a pin gone stale enough to matter. +# unrelated client-runtime commit must not redden every open client PR. +# +# WHAT CATCHES A STALE PIN is the `pin-staleness` job below -- NOT this job, +# which checks out the pinned ref and so compares the vendored copy against +# itself and agrees. The sentence that used to sit here ("the weekly run is what +# catches a pin gone stale enough to matter") was false for exactly the reason +# this PR exists, and removing it from scripts/.client-runtime-ref while leaving +# it here left the fix half-applied -- the same rule-7 assertion surviving one +# comment over. # # tracebloc/client-runtime is PRIVATE, so unlike cli's chart-drift (which reads # the public tracebloc/client with no token) this needs an App installation @@ -142,12 +149,41 @@ jobs: # reporting on both is worse than not checking, because it looks like # cover. # - # Left half is the path in client-runtime, right half is where this repo - # vendors it -- named together so adding a third contract is one line. + # A FLAT LIST OF PRODUCER PATHS. Both entries are paths in + # client-runtime; neither is a path in this repo. This comment used to + # claim "left half is the path in client-runtime, right half is where + # this repo vendors it" -- a mapping the value never had, which is the + # precise defect this job exists to catch, sitting in the job itself. + # Where each one is vendored is named in the adopt instructions below. + # Adding a third contract is still one word here. CONTRACTS="envelope_contract.json log_redactions.json" + # Derived through the same sanitiser the loop uses, so that if this + # name ever gains a directory the summary block below follows it. + ENVELOPE="envelope_contract.json" # FAIL CLOSED on every read: a fetch I could not do is not evidence the # pin is fresh, and two unknowns must never compare equal (rule 3). + # + # FAILING CLOSED IS NOT THE SAME AS FAILING THE PR. A fork PR mints no + # token and a client-runtime API hiccup is transient; reddening an + # unrelated PR for either is the outcome the warn-on-PR design was + # written to avoid, and every read below used to do exactly that by + # exiting 1 before the EVENT branch was ever reached. + # + # The split is deliberate and is NOT "warn on everything": an empty or + # malformed ref is THIS PR's diff -- scripts/.client-runtime-ref lives in + # this repo and a PR can change it -- so those still fail hard above. + # Routing them through here would let a PR land a broken pin behind a + # warning nobody reads. Only "I could not find out" warns. + cannot_tell() { + if [ "$EVENT" = "pull_request" ]; then + echo "::warning::$1 -- cannot tell whether the pin is stale (not this PR's doing)" + exit 0 + fi + echo "::error::$1 -- cannot tell whether the pin is stale" + exit 1 + } + fetch() { curl -fsSL --connect-timeout 10 --max-time 60 --retry 2 \ -H "Authorization: Bearer $GH_TOKEN" \ @@ -155,16 +191,43 @@ jobs: "https://api.github.com/repos/tracebloc/client-runtime/contents/$1?ref=$2" } + # The repo object itself, for the default branch. Same token, same + # bounds, same fail-closed contract as fetch() -- a different Accept + # because this one is metadata, not a file. + fetch_json() { + curl -fsSL --connect-timeout 10 --max-time 60 --retry 2 \ + -H "Authorization: Bearer $GH_TOKEN" \ + -H "Accept: application/vnd.github+json" \ + "https://api.github.com/repos/tracebloc/client-runtime$1" + } + + # DERIVED, NOT HARDCODED. This job's name, and the PR describing it, + # both say it compares against client-runtime's DEFAULT BRANCH; the code + # said the literal `develop`. They agree today, so it was not a live bug + # -- but a default-branch rename would have turned every fetch into a + # silent 404, and "the comment and the code agree only by coincidence" + # is the thing this PR is about. Ask the producer instead (rule 1). + default_branch="$(fetch_json "" | python3 -c 'import json,sys; print(json.load(sys.stdin)["default_branch"])' 2>/dev/null)" \ + || cannot_tell "cannot read client-runtime's default branch" + [ -n "$default_branch" ] || cannot_tell "client-runtime reported no default branch" + echo "comparing against client-runtime's default branch: $default_branch" + stale="" for path in $CONTRACTS; do - fetch "$path" "$ref" > "/tmp/pinned-$path" \ - || { echo "::error::cannot read $path at the pinned ref $ref"; exit 1; } - fetch "$path" develop > "/tmp/head-$path" \ - || { echo "::error::cannot read $path on client-runtime develop -- cannot tell whether the pin is stale"; exit 1; } - if cmp -s "/tmp/pinned-$path" "/tmp/head-$path"; then - echo "$path: pinned copy matches develop" + # SLASH-SAFE TEMP NAMES. `/tmp/pinned-$path` only worked because both + # contract paths happen to be flat filenames; a contract under a + # subdirectory would redirect into a parent that does not exist and + # abort the job under `set -e` -- contradicting the "adding a third + # contract is one line" claim six lines up. + safe="${path//\//_}" + fetch "$path" "$ref" > "/tmp/pinned-$safe" \ + || cannot_tell "cannot read $path at the pinned ref $ref" + fetch "$path" "$default_branch" > "/tmp/head-$safe" \ + || cannot_tell "cannot read $path on client-runtime $default_branch" + if cmp -s "/tmp/pinned-$safe" "/tmp/head-$safe"; then + echo "$path: pinned copy matches $default_branch" else - echo "$path: DIFFERS between the pinned ref and develop" + echo "$path: DIFFERS between the pinned ref and $default_branch" stale="$stale $path" fi done @@ -176,11 +239,12 @@ jobs: # this job exists to surface -- would have FAILED a pull request that # was supposed to warn (Bugbot Medium). `|| true` keeps the failure # cosmetic; the reads that matter already failed closed above. - if [ -s /tmp/pinned-envelope_contract.json ]; then - pinned="$(python3 scripts/contract-summary.py /tmp/pinned-envelope_contract.json 2>/dev/null || true)" - head="$(python3 scripts/contract-summary.py /tmp/head-envelope_contract.json 2>/dev/null || true)" + envelope_safe="${ENVELOPE//\//_}" + if [ -s "/tmp/pinned-$envelope_safe" ]; then + pinned="$(python3 scripts/contract-summary.py "/tmp/pinned-$envelope_safe" 2>/dev/null || true)" + head="$(python3 scripts/contract-summary.py "/tmp/head-$envelope_safe" 2>/dev/null || true)" if [ -n "$pinned" ] && [ -n "$head" ]; then - echo "envelope contract: pinned v${pinned%%|*} (${pinned#*|}) vs develop v${head%%|*} (${head#*|})" + echo "envelope contract: pinned v${pinned%%|*} (${pinned#*|}) vs $default_branch v${head%%|*} (${head#*|})" else echo "envelope contract: could not summarise one of the copies (schema change?) -- the comparison above stands regardless" fi @@ -191,7 +255,7 @@ jobs: exit 0 fi - msg="the pin is behind client-runtime develop for:$stale. To adopt: bump scripts/.client-runtime-ref, re-vendor BOTH contracts (scripts/tests/fixtures/envelope_contract.json and client/log_redactions.json), re-run scripts/gen-envelope-embed.sh (it rewrites the bash vectors AND the embedded literals in scripts/lib/install-client-helm.sh and scripts/install-k8s.ps1), then update the hand-written expectations the new numbers move." + msg="the pin is behind client-runtime $default_branch for:$stale. To adopt: bump scripts/.client-runtime-ref, re-vendor BOTH contracts (scripts/tests/fixtures/envelope_contract.json and client/log_redactions.json), re-run scripts/gen-envelope-embed.sh (it rewrites the bash vectors AND the embedded literals in scripts/lib/install-client-helm.sh and scripts/install-k8s.ps1), then update the hand-written expectations the new numbers move." if [ "$EVENT" = "pull_request" ]; then echo "::warning::PIN STALE (not this PR's doing) -- $msg" exit 0