fix(935): script-tests jq tripwire accepts exactly {1.6, 1.7} until server-management#1080 pins the fleet
jq-preflight.sh --expect takes a comma-separated set (validated whole, exact major.minor membership); pr-checks.yml passes 1.6,1.7. Both small runners float on runner-images:ubuntu-latest, so the fleet's jq is a function of pull history (1.7 measured on PR #933 job 12347 and PR #934 job 12360, 2026-10-29). New record ci.jq-accepted-version-set supersedes ci.jq-version-contract (archived); docs/ci-cd.md, the script header and runner-ships-1.6 comments now state the dated invariant. review-verdict.yml stays floor-only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: f9ce6bc6-5b36-491a-8d72-89c1f70dd3e1
This commit is contained in:
@@ -546,16 +546,18 @@ jobs:
|
||||
- name: Install test dependencies
|
||||
run: python3 -m pip install --disable-pip-version-check --quiet pytest pyyaml
|
||||
# jq gets its OWN step because its VERSION, not merely its presence, is load-bearing
|
||||
# (ersatztv#648). `--expect` makes this a TRIPWIRE: scripts/tests exercises the jq 1.6 code path
|
||||
# only because this runner ships 1.6, so an upgrade would silently delete that coverage — and
|
||||
# the three divergences found in ersatztv#643/#647 all lived exactly there. Going red forces an
|
||||
# explicit human decision instead of letting the coverage evaporate.
|
||||
# (ersatztv#648). `--expect` makes this a TRIPWIRE over an accepted SET of versions: as of
|
||||
# 2026-10-29 the `small` fleet is mixed, 1.6 or 1.7 depending on which host takes the job (both
|
||||
# map to the floating runner-images:ubuntu-latest image, no digest pin), so the set is exactly
|
||||
# {1.6, 1.7} (ersatztv#935). Any other version goes red. Which of the two the suite runs under is
|
||||
# decided by the runner, so the 1.6 coverage is incidental. server-management#1080 proposes a fleet
|
||||
# digest pin; when it settles one version, narrow this to that version.
|
||||
#
|
||||
# The pin lives HERE and deliberately NOT in review-verdict.yml: that workflow writes the
|
||||
# branch-protection-required `review-verdict/h10` status, so pinning a version there would turn
|
||||
# any jq bump on the runner into a repo-wide merge deadlock. It gets the floor-only mode.
|
||||
# See docs/ci-cd.md -> "The jq contract".
|
||||
- name: Preflight jq version
|
||||
run: ./scripts/jq-preflight.sh --expect 1.6
|
||||
run: ./scripts/jq-preflight.sh --expect 1.6,1.7
|
||||
- name: Run scripts/tests
|
||||
run: PYTHONPATH=. python3 -m pytest scripts/tests -q
|
||||
|
||||
@@ -572,7 +572,7 @@ jobs:
|
||||
# verdict and leaves a FORGED one — and this job's own red status is not a required check.
|
||||
#
|
||||
# The empty case is checked EXPLICITLY, not left to jq's exit status: `jq -e` over empty input
|
||||
# exits 4 on jq >= 1.7 but 0 on jq 1.6, and THE RUNNER SHIPS 1.6 (ersatztv#647) — so on a
|
||||
# exits 4 on jq >= 1.7 but 0 on jq 1.6, and a runner may report 1.6 (the floor; ersatztv#647) — so on a
|
||||
# transient error this guard passed, the row came back "", and the job posted over a
|
||||
# possibly-existing human verdict.
|
||||
#
|
||||
@@ -1232,7 +1232,7 @@ jobs:
|
||||
# COUNTED, NOT `jq -e`, and that is this function's own standing rule rather than a
|
||||
# style preference. The `kind` read above says it in full: `jq -e` reports the
|
||||
# truthiness of its LAST OUTPUT, which is the exit-status subtlety that already bit
|
||||
# this workflow at jq 1.6 (ersatztv#647), and the runner still ships 1.6. A count read
|
||||
# this workflow at jq 1.6 (ersatztv#647), and a runner may still report 1.6. A count read
|
||||
# as a VALUE and then range-checked in shell cannot be read two ways, and it matches
|
||||
# every other tally here. (Using `jq -e` here would be safe by accident — the
|
||||
# array is known non-empty by the check above, so the empty-input
|
||||
|
||||
+27
-16
@@ -1304,7 +1304,7 @@ minutes, and it would still be wrong to filter on `scripts/**`.
|
||||
stay unpinned on purpose: a pytest release does not add assertions to your suite, a ruff release
|
||||
adds rules to your lint.
|
||||
- Lint runs **before the jq preflight**, and after the `git` one. `Preflight jq version` is a hard
|
||||
`--expect` tripwire; a lint sitting behind it goes dark for as long as the jq contract is broken,
|
||||
`--expect` tripwire; a lint sitting behind it goes dark for as long as the jq version is outside the accepted set,
|
||||
under a red that says "jq". `Preflight external tools` stays ahead, because the lint steps consume
|
||||
`git` — without it a missing git reaches them as an empty population and they blame the glob.
|
||||
- Neither step is `ruff check .`. Both pass an **explicit population** from
|
||||
@@ -1331,7 +1331,7 @@ which is itself a small demonstration of why the suite needed to run in CI at al
|
||||
against a temp repo it creates itself, never this repository's history.
|
||||
|
||||
Two **preflight steps** run before the suite. The first asserts `git` is on PATH; the second runs
|
||||
`scripts/jq-preflight.sh --expect 1.6`, which checks jq's **version**, not merely its presence (see
|
||||
`scripts/jq-preflight.sh --expect 1.6,1.7`, which checks jq's **version**, not merely its presence (see
|
||||
"The jq contract" below). Those two tests exec the real shell scripts, which shell out to `jq` ~26
|
||||
times; the tests shim `curl` on PATH but not `jq`, so a runner image without it would surface as ~20
|
||||
opaque assertion failures instead of one diagnosis. Both deliberately **check** rather than install —
|
||||
@@ -1340,13 +1340,16 @@ the runner image.
|
||||
|
||||
### The jq contract (ersatztv#648)
|
||||
|
||||
> Full rationale: `docs/decisions/records/ci/jq-version-contract.md`.
|
||||
> Full rationale: `docs/decisions/records/ci/jq-accepted-version-set.md` (`ci.jq-accepted-version-set`).
|
||||
|
||||
Every shell gate in this repo — `decisions-guard`, `script-tests`'s own harness,
|
||||
`pretooluse-merge-consent.sh`, `review-verdict.yml`, `scripts/pr-changed-files.sh` — is authored and
|
||||
tested on a developer Mac shipping **jq 1.8.x**. The CI runner ships **jq 1.6**. Author to the
|
||||
1.6-compatible subset; three concrete constructs diverge between the two and each one produced a real
|
||||
bug when it hit CI for the first time:
|
||||
tested on a developer Mac shipping **jq 1.8.x**. The CI runners' jq is **not one version**: as of
|
||||
2026-10-29 both `small` runners map to the floating `runner-images:ubuntu-latest` image with no digest
|
||||
pin, so each host keeps whatever it last pulled (jq 1.7 reported on both hosts that took PR #933 and
|
||||
#934; a host holding an older image still reports 1.6; server-management#1080). Author to the
|
||||
1.6-compatible subset, which is the **floor**; three concrete constructs diverge between 1.6 and
|
||||
later versions and each one produced a real bug when it hit CI for the first time:
|
||||
|
||||
- **`jq -e` over EMPTY input.** Exits 4 on jq >= 1.7, but **0** on jq 1.6. A guard that infers
|
||||
"transport failure" from that exit status silently passes an empty/failed page on 1.6.
|
||||
@@ -1361,16 +1364,24 @@ bug when it hit CI for the first time:
|
||||
the parsed version and asserts a floor of 1.6) so a future divergence can be diagnosed from the log
|
||||
alone instead of guessing at the runner image.
|
||||
|
||||
**Pin vs floor is deliberately asymmetric.** `scripts/jq-preflight.sh --expect 1.6` additionally pins
|
||||
the version and fails loudly if it drifts, but that mode is used **only** by `script-tests`
|
||||
(`.gitea/workflows/pr-checks.yml`) — advisory, not a required check. `review-verdict.yml` runs the
|
||||
no-args floor-only mode and never pins, because that workflow writes `review-verdict/h10`, the
|
||||
branch-protection-**required** status check on `main`: a hard pin there would mean the day the
|
||||
runner's jq version changes (a base-image bump, a host reimage — nothing this repo controls), every
|
||||
PR on `main` stops merging until someone notices and re-pins. A required merge gate cannot fail
|
||||
because an upstream package manager did its job. The narrower pin on `script-tests` exists precisely
|
||||
because that job is the suite's only 1.6 coverage — if the runner's jq silently changed, that coverage
|
||||
would evaporate with no signal, so failing loudly there forces a human decision instead.
|
||||
**Accepted set, not a single pin (2026-10-29, ersatztv#935).** `scripts/jq-preflight.sh --expect
|
||||
1.6,1.7` additionally asserts the version is a member of the comma-separated set and fails loudly
|
||||
(exit 1) on any other version; a malformed set (`1.6,`, `1.x`) is a usage error (exit 2). It is used
|
||||
**only** by `script-tests` (`.gitea/workflows/pr-checks.yml`). The set is exactly {1.6, 1.7}: a
|
||||
1.8 runner stays red so a person decides. Which member the suite runs under is decided by whichever
|
||||
runner takes the job, so **real-jq 1.6 coverage of `scripts/tests/` is incidental**; the 1.6 quirks
|
||||
are still exercised on every run by in-repo jq shims (`test_pr_changed_files.py`,
|
||||
`test_merge_consent_exemption.py`). The floor stays at 1.6. **server-management#1080** (a fleet digest
|
||||
pin) is what narrows the set: when it settles one version, change `--expect` to that single version.
|
||||
The tripwire is advisory in name only: `pretooluse-merge-consent.sh` reads the combined status, so a
|
||||
red `script-tests` blocks the hook-mediated merge path until the `--expect` value is fixed.
|
||||
|
||||
**Pin vs floor is deliberately asymmetric.** `review-verdict.yml` runs the no-args floor-only mode and
|
||||
never `--expect`, because that workflow writes `review-verdict/h10`, the branch-protection-**required**
|
||||
status check on `main`: a version assertion there would mean the day a runner's jq version changes (a
|
||||
base-image pull, a host reimage — nothing this repo controls), every PR on `main` stops merging until
|
||||
someone notices and re-pins. A required merge gate cannot fail because an upstream package manager did
|
||||
its job.
|
||||
|
||||
Baking a pinned jq into `docker/ci/Dockerfile` was considered and rejected: `review-verdict.yml` is
|
||||
`runs-on: small` with no toolchain-image pin, and per `ci.small-lane-git-only` the small lane is
|
||||
|
||||
@@ -57,7 +57,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
|
||||
| `ci.grep-q-pipefail-inversion` | In any script running under `set -o pipefail`, a security or classification predicate of the form `producer \| grep -q…` is FORBIDDEN: `grep -q` exits at its first match, the producer then takes SIGPIPE and exits 141 once the data exceeds the pipe buffer (~64K), so `pipefail` reports the pipeline as FAILED even though grep MATCHED — inverting the predicate exactly when the input is large. A here-string (`grep -q… <<< "$data"`) is ALSO forbidden: bash materialises a large here-string via temporary storage, so it fails when temp space is full or unwritable, and inside an `if`/`!` that failure flips the predicate the same way. COUNT instead — `n=$(printf '%s\n' "$data" \| grep -cE "$re")` — because `grep -c` drains stdin (no early exit, no SIGPIPE) over an ordinary pipe (no temp file). Read grep's status honestly: exit 1 means a zero count and is a legitimate answer, anything >1 is a real error. Evaluate the counts ONCE at TOP LEVEL, never inline inside an `if`/`elif` condition: inside `$( )` an `exit` leaves only the subshell and `set -e` does not fire, so an error silently reads as "no match". Validate that each result is numeric and fail closed if not. This applies to both the enforced gate `.gitea/workflows/review-verdict.yml` and the advisory hook `.claude/hooks/pretooluse-merge-consent.sh`. | 2026-07-29 | [link](records/ci/grep-q-pipefail-inversion.md) |
|
||||
| `ci.image-build-delegates-the-spa-suite` | `docker/Dockerfile` runs no vitest suite. Its web-build stage lints, typechecks and BUILDS the SPA; the suite runs once, unfiltered, in `docker-build.yml`'s `test` job on a real checkout, and `build` carries `needs: [test, migrations, scan]` so no image is published past a red suite. THAT EDGE IS NOW THE ONLY LAYER, so the guard checks it is real and not merely present — and it does so by PINNING TEXT rather than parsing it. The commands each SPA-carrying Dockerfile stage runs, and the gating step's `run:` body and `if:`, are compared as strings against a declared pin; the step and its job must carry no `continue-on-error` in any spelling, the job no job-level `if:`, and the publish step must keep its own `docs_only` gate. A guard that asks what a command MEANS was wrong nine times in three review rounds. A pin is immune to a different SPELLING of the command, which is that whole class; it is not immune to the same text meaning something else, so the routes to that are pinned or refused too: `working-directory` and the step's own `shell:`, the workflow `defaults.run.shell` and the job-level `defaults` overriding it, a stage `SHELL`, `web/package.json`'s script map (pinned WHOLE — selecting on the literal `vitest` missed `npm run test`, `npm t` and the `prebuild`/`preinstall` lifecycle hooks), `web/vite.config.ts` PINNED WHOLE — it decides what the suite collects and what `vite build` loads, and pinning a BLOCK of it was defeated seven measured ways across three rounds, so the partial match was withdrawn rather than respelled an eighth time; and the ABSENCE of any config file that outranks it — `vitest.config.*` for vitest, and `vite.config.js`/`.mjs` for `vite build`, whose `DEFAULT_CONFIG_FILES` lists them BEFORE `.ts`. Pinning one file is worthless while a second can outrank it, and that family went short twice. The gating job's `container:` image is left to `ci.image-pin-population` rather than guarded twice. What is unmodelled is a LIST, not an "only": an `ENV` changing what a pinned `RUN` resolves, the plugin BODIES, and a publish through another action. The count of such routes lives in the `docs/guard-inventory.md` row and nowhere else — it briefly existed in three places with two different values, which is how a number stays wrong. It is a running total, wrong at every count so far. They share one shape, which is the transferable part: A PIN ASSUMES IT IS PINNING THE ARTIFACT THAT STILL DECIDES, and every route found so far is authority moving to another file, another occurrence, another workflow, or a hook the pinned command invokes. Do NOT restore a filtered in-image run by naming the specs that cannot run there: that list is a population nothing derives, and the red it produces when it goes stale is unreachable on a PR — `Build & push image (amd64)` is `if: github.event_name != 'pull_request'` — so it lands on `main` and on the `v*` tag path, where it fails the release cut. Held in both directions by `scripts/tests/test_image_build_delegates_the_spa_suite.py`. | 2026-08-30 | [link](records/ci/image-build-delegates-the-spa-suite.md) |
|
||||
| `ci.infra-shaped-red-under-load` | When a job dies inside a setup/cache step before your code compiles, check the runner host's load before diagnosing the diff, and never file a CI bug off one sample under pressure. | 2026-07-21 | [link](records/ci/infra-shaped-red-under-load.md) |
|
||||
| `ci.jq-version-contract` | Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset, because the CI runner ships jq 1.6 while every developer Mac ships 1.8.x. `scripts/jq-preflight.sh` (no args) prints the parsed version and asserts a floor of 1.6 in every gate job's log; `scripts/jq-preflight.sh --expect 1.6` additionally pins and fails loudly, but ONLY in the `script-tests` job. `review-verdict.yml` never pins — it writes the branch-protection-required `review-verdict/h10` status, so a hard pin there would turn any jq bump into a repo-wide merge deadlock. | 2026-07-26 | [link](records/ci/jq-version-contract.md) |
|
||||
| `ci.jq-accepted-version-set` | Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset (floor 1.6, unchanged). As of 2026-10-29 the `small` runner fleet is MIXED: both small runners map to the floating `runner-images:ubuntu-latest` image with no digest pin, so each host keeps whatever it last pulled (jq 1.7 reported on both hosts that took PR #933 and #934; a host holding an older image still reports 1.6). `scripts/jq-preflight.sh --expect 1.6,1.7` in the `script-tests` job accepts exactly that comma-separated SET and fails loudly on any other version; which member the suite runs under is decided by the runner, so the real-jq 1.6 coverage is INCIDENTAL, not guaranteed. server-management#1080 (fleet digest pin) is the event that narrows the set to one version. `review-verdict.yml` stays floor-only (no `--expect`): it writes the branch-protection-required `review-verdict/h10` status. | 2026-10-29 | [link](records/ci/jq-accepted-version-set.md) |
|
||||
| `ci.killed-job-triage` | Never trust a job's `conclusion` field alone — read the log tail and require an `❌ Failure - Main …` marker before treating a red as a real failure. | 2026-07-21 | [link](records/ci/killed-job-triage.md) |
|
||||
| `ci.monitor-armed-at-pr-open` | Arm a CI monitor on the PR head sha the moment the PR opens, polling the commit-status endpoint — not at the end of the work. | 2026-07-21 | [link](records/ci/monitor-armed-at-pr-open.md) |
|
||||
| `ci.no-host-health-gating` | Push when your work is validated — never SSH to bumblebee to sample load/RAM first, and never hand-schedule around other sessions' runs. | 2026-07-21 | [link](records/ci/no-host-health-gating.md) |
|
||||
|
||||
+3
-3
@@ -1,11 +1,11 @@
|
||||
---
|
||||
key: ci.jq-version-contract
|
||||
title: '2026-07-26 — jq 1.6 is the FLOOR every shell gate must run on; `scripts/jq-preflight.sh` makes the version observable, and only `script-tests` pins it (#648)'
|
||||
status: active
|
||||
status: superseded
|
||||
since: '2026-07-26'
|
||||
supersedes: none
|
||||
superseded-by: none
|
||||
rule: 'Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset, because the CI runner ships jq 1.6 while every developer Mac ships 1.8.x. `scripts/jq-preflight.sh` (no args) prints the parsed version and asserts a floor of 1.6 in every gate job''s log; `scripts/jq-preflight.sh --expect 1.6` additionally pins and fails loudly, but ONLY in the `script-tests` job. `review-verdict.yml` never pins — it writes the branch-protection-required `review-verdict/h10` status, so a hard pin there would turn any jq bump into a repo-wide merge deadlock.'
|
||||
superseded-by: ci.jq-accepted-version-set@2026-10-29
|
||||
rule: '(superseded) Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset, because the CI runner ships jq 1.6 while every developer Mac ships 1.8.x. `scripts/jq-preflight.sh` (no args) prints the parsed version and asserts a floor of 1.6 in every gate job''s log; `scripts/jq-preflight.sh --expect 1.6` additionally pins and fails loudly, but ONLY in the `script-tests` job. `review-verdict.yml` never pins — it writes the branch-protection-required `review-verdict/h10` status, so a hard pin there would turn any jq bump into a repo-wide merge deadlock.'
|
||||
signals: 'jq version divergence, jq 1.6 vs 1.8, jq -e exit code on empty input, contains NUL false positive, jq parse-error exit code collision, jq-preflight, script-tests --expect, review-verdict jq floor, merge deadlock from a pinned dependency · paths: `scripts/jq-preflight.sh`, `.gitea/workflows/review-verdict.yml`, `.gitea/workflows/pr-checks.yml`, `docs/ci-cd.md` · issues: #643, #647, #648, #649'
|
||||
mechanics: '`scripts/jq-preflight.sh` (no args) -> floor+observability in every gate job; `scripts/jq-preflight.sh --expect 1.6` -> tripwire, `script-tests` job only; `docs/ci-cd.md` -> "The jq contract"'
|
||||
---
|
||||
@@ -0,0 +1,79 @@
|
||||
---
|
||||
key: ci.jq-accepted-version-set
|
||||
title: '2026-10-29 — jq 1.6 stays the FLOOR; `script-tests` accepts exactly the SET {1.6, 1.7} until server-management#1080 pins the runner fleet (#935)'
|
||||
status: active
|
||||
since: '2026-10-29'
|
||||
supersedes: ci.jq-version-contract@2026-07-26
|
||||
superseded-by: none
|
||||
rule: 'Every shell gate that shells out to `jq` is authored to the jq 1.6-compatible subset (floor 1.6, unchanged). As of 2026-10-29 the `small` runner fleet is MIXED: both small runners map to the floating `runner-images:ubuntu-latest` image with no digest pin, so each host keeps whatever it last pulled (jq 1.7 reported on both hosts that took PR #933 and #934; a host holding an older image still reports 1.6). `scripts/jq-preflight.sh --expect 1.6,1.7` in the `script-tests` job accepts exactly that comma-separated SET and fails loudly on any other version; which member the suite runs under is decided by the runner, so the real-jq 1.6 coverage is INCIDENTAL, not guaranteed. server-management#1080 (fleet digest pin) is the event that narrows the set to one version. `review-verdict.yml` stays floor-only (no `--expect`): it writes the branch-protection-required `review-verdict/h10` status.'
|
||||
signals: 'jq version divergence, jq 1.6 vs 1.7, script-tests jq tripwire red, jq-1.7 (parsed 1.7; floor 1.6), --expect set, mixed runner fleet, floating runner-images ubuntu-latest, server-management#1080, jq-preflight, merge blocked by combined status · paths: `scripts/jq-preflight.sh`, `scripts/tests/test_jq_preflight.py`, `.gitea/workflows/pr-checks.yml`, `.gitea/workflows/review-verdict.yml`, `docs/ci-cd.md` · issues: #935, #648, #643, #647'
|
||||
mechanics: '`scripts/jq-preflight.sh` (no args) -> floor+observability in every gate job; `scripts/jq-preflight.sh --expect 1.6,1.7` -> set tripwire, `script-tests` job only (`test_jq_preflight.py` pins the exact value); `docs/ci-cd.md` -> "The jq contract"'
|
||||
---
|
||||
|
||||
Supersedes `ci.jq-version-contract` (2026-07-26, archived), whose premise was that the CI runner
|
||||
ships jq 1.6. That is no longer a fact the repo can state.
|
||||
|
||||
**Measured 2026-10-29.** `script-tests`' tripwire (`--expect 1.6`) went red on jazz-small-runner
|
||||
(PR #933, job 12347) and on bumblebee's small-runner (PR #934, job 12360), each reporting
|
||||
`jq-1.7 (parsed 1.7; floor 1.6)`. Cause (server-management#1080): both `small` runners map to the
|
||||
floating `runner-images:ubuntu-latest` image with no digest pin or forced pull, so each host keeps
|
||||
whatever it last pulled and the fleet's jq version is a function of pull history. A digest pin across
|
||||
the fleet is proposed in server-management#1080 and not yet decided.
|
||||
|
||||
**The invariant.** `script-tests` accepts exactly the set `{1.6, 1.7}`:
|
||||
|
||||
- `scripts/jq-preflight.sh --expect 1.6,1.7` parses the argument as a comma-separated set of
|
||||
`<major>.<minor>` members; an empty or free-form member (`1.6,`, `1.6,,1.7`, `1.x`) is a usage
|
||||
error (exit 2), never a member that matches nothing or everything. Membership is exact on
|
||||
`major.minor`, not a prefix or range test (`1.10` and `1.71` do not match `1.7` / `1.6`).
|
||||
- A version outside the set (`1.8`, which every developer Mac ships, `2.x`) exits 1 with the tripwire
|
||||
message. That red is witnessed by `scripts/tests/test_jq_preflight.py`
|
||||
(`test_a_version_outside_the_accepted_set_is_LOUD`), and the exact `--expect 1.6,1.7` value in
|
||||
`pr-checks.yml` is pinned by `test_script_tests_pins_the_jq_version`.
|
||||
- The set is exactly {1.6, 1.7}, not "1.6 or newer": widening it to every version would delete the
|
||||
tripwire. Admitting 1.7 is evidence-driven (both measured hosts run it); admitting 1.8 is not, and
|
||||
a 1.8 runner should go red so a person decides.
|
||||
|
||||
**The floor stays at 1.6.** Raising it would assert that no host in the fleet can report 1.6, which
|
||||
the unpinned image does not guarantee, and which only the #1080 decision can establish. A gate
|
||||
authored to the 1.6 subset is harmless on 1.7, and the three divergences that motivated the contract
|
||||
(`jq -e` over empty input, `contains("\u0000")`, the parse-error exit code) remain real on whichever
|
||||
host still runs 1.6.
|
||||
|
||||
**What coverage is lost.** The real-jq 1.6 run of `scripts/tests/` is now incidental: it happens only
|
||||
when a 1.6 host takes the job, and nothing guarantees that on any given PR. The 1.6 quirks are still
|
||||
exercised on every run by in-repo jq shims that reproduce them (`test_pr_changed_files.py`,
|
||||
`test_merge_consent_exemption.py`), so the shim-based proofs hold on a 1.7 runner; what is not
|
||||
covered on a 1.7 run is any NEW 1.6-only divergence in a construct no shim models.
|
||||
|
||||
**What narrows the set.** server-management#1080. When it pins the fleet to one jq version, change
|
||||
the `--expect` value in `pr-checks.yml` to that single version (the script accepts a one-member set)
|
||||
and record the pin here by superseding this record. If the pin lands on 1.7, decide then whether the
|
||||
floor rises, with the fleet evidence in hand.
|
||||
|
||||
**Unchanged from the superseded record.**
|
||||
|
||||
- *Pin vs floor is asymmetric.* `review-verdict.yml` runs the no-args floor-only mode and never
|
||||
`--expect`: it writes `review-verdict/h10`, the branch-protection-required status on `main`, so a
|
||||
version assertion there would turn any runner jq change into a repo-wide merge deadlock.
|
||||
`test_review_verdict_never_pins_a_jq_version` holds that.
|
||||
- *Firing the tripwire blocks merges.* `.claude/hooks/pretooluse-merge-consent.sh` reads the combined
|
||||
commit status and denies on anything not `success`/`skipped` (`ci.advisory-red-blocks-the-merge-gate`,
|
||||
#598), and `script-tests` is a Gitea Actions job folded into it. Only the
|
||||
`merge_when_checks_succeed` path, which defers to Gitea's required checks, is spared. This is the
|
||||
cost of the tripwire, accepted because the remedy is a one-line edit to `--expect`, and the
|
||||
2026-10-29 firing is that cost being paid.
|
||||
- *Baking jq into `docker/ci/Dockerfile` cannot reach the gate.* `review-verdict.yml` is
|
||||
`runs-on: small` with no toolchain-image pin and `ci.small-lane-git-only` keeps that lane
|
||||
git-only, so it gets the host's jq whatever the toolchain image holds. The fleet pin in
|
||||
server-management#1080 is the only lever that reaches it.
|
||||
- *The parse is strictly fail-closed.* `jq-preflight.sh` accepts only a first line of the form
|
||||
`jq-<X>.<Y>` or `jq version <X>.<Y>`; anything else exits 1. If a wrapper banner ever breaks that
|
||||
in the required `review-verdict.yml` job, widen the accepted forms, never relax fail-closed: an
|
||||
unparsed version must not satisfy the floor.
|
||||
- *The three constructs to avoid:* never infer "empty input" from a jq exit status (check the string
|
||||
in shell first); never use `contains("\u0000")` (use `explode | index(0)`); never read `jq`'s
|
||||
exit code as a specific parse-error signal on ambiguous input (validate the response shape).
|
||||
|
||||
Full rationale for how the three divergences were found is in
|
||||
`docs/decisions/archive/ci/jq-version-contract.md` and `docs/decisions/records/ci/script-tests-job.md`.
|
||||
@@ -91,7 +91,7 @@ a judgement that type checking does not belong.
|
||||
|
||||
**The pin is the second half of the fix.** An unpinned `pip install ruff` re-introduces exactly the
|
||||
divergence the config closes, one layer up: the verdict becomes a function of *when* the job ran. Same
|
||||
argument as the `jq` pin in the same job (`ci.jq-version-contract`), and the same consequence — a bump
|
||||
argument as the `jq` pin in the same job (`ci.jq-accepted-version-set`), and the same consequence — a bump
|
||||
is a PR someone reads. `pytest` and `pyyaml` in the same job stay unpinned, and the
|
||||
asymmetry is the point rather than an oversight: a pytest release does not add assertions to your
|
||||
suite, a ruff release adds rules to your lint.
|
||||
|
||||
@@ -56,7 +56,7 @@ both reds were real:
|
||||
PyYAML). It passed locally only because the author's machine had PyYAML.
|
||||
2. A **fail-open in the merge-consent gate itself.** `jq -e` over EMPTY input exits 4 on jq ≥ 1.7
|
||||
but **0 on jq 1.6** (verified against both binaries), and the docs-only pagination guard leaned
|
||||
on that exit status to reject a transport failure. On jq 1.6 — which the CI runner ships — the
|
||||
on that exit status to reject a transport failure. On jq 1.6 — which the CI runner shipped on 2026-07-26 — the
|
||||
failed page passed the guard, the loop walked past it, the next page legitimately returned `[]`,
|
||||
and `files_complete=yes` was set over a PARTIAL list: the docs-only exemption firing over unread
|
||||
pages that may be pure code. Fixed by rejecting an empty body explicitly instead of inferring it
|
||||
@@ -68,7 +68,7 @@ on a developer Mac (jq 1.8) — only in CI, where the suite had never run. Two f
|
||||
the same shape (a `contains("\u0000")` false positive and a colliding parse-error exit code) turned
|
||||
up the same day; the durable contract that came out of all three — the exact constructs to avoid, and
|
||||
why `jq-preflight.sh` pins in `script-tests` but only floors the version in `review-verdict.yml` — is
|
||||
recorded once, in `ci.jq-version-contract` (`docs/decisions/records/ci/jq-version-contract.md`), and
|
||||
recorded once, in `ci.jq-accepted-version-set` (`docs/decisions/records/ci/jq-accepted-version-set.md`), and
|
||||
is not restated here.
|
||||
|
||||
An independent cross-family review of that fix then found **two further fail-opens in the same
|
||||
|
||||
@@ -100,8 +100,8 @@ classifier="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib/branch-rule-classi
|
||||
# comparison was made", which is the truth for a non-array body.
|
||||
#
|
||||
# "To produce a finding" rather than "before the classifier sees it", and the distinction is the
|
||||
# `ci.jq-version-contract` gap (ersatztv#643): on jq 1.6 — the version `scripts/jq-preflight.sh`
|
||||
# pins for `script-tests` — `jq -e` over EMPTY input exits 0, so an empty 200 body is not diverted
|
||||
# `ci.jq-accepted-version-set` gap (ersatztv#643): on jq 1.6 — the floor `scripts/jq-preflight.sh`
|
||||
# admits for `script-tests` — `jq -e` over EMPTY input exits 0, so an empty 200 body is not diverted
|
||||
# here and does reach the classifier. Measured on both 1.6 and 1.8.2: the class word is `unreadable`
|
||||
# either way, because the classifier then produces nothing and the catch-all answers. The outcome is
|
||||
# version-independent; only the mechanism differs, so the comment states the outcome.
|
||||
|
||||
@@ -139,8 +139,8 @@ comments=$(cat)
|
||||
# NOTE the NUL test is `explode | index(0)`, NOT `contains("\u0000")` (ersatztv#647). On jq 1.6 the
|
||||
# escape truncates the literal to the EMPTY string, and every string contains "" — so that form
|
||||
# returns true for ALL input, making this guard reject every comment body as malformed. Verified
|
||||
# against both binaries: 1.6 says true for "hello", 1.7+ says false. The CI runner ships jq 1.6, so
|
||||
# the whole verdict classifier was inert there. `explode | index(0)` agrees on both.
|
||||
# against both binaries: 1.6 says true for "hello", 1.7+ says false. A CI runner reporting jq 1.6 (the floor)
|
||||
# had the whole verdict classifier inert. `explode | index(0)` agrees on both.
|
||||
#
|
||||
# `Review<NUL>-verdict: MERGEABLE @ <head>` would arrive at the matcher as a valid verdict line —
|
||||
# text that is not a verdict silently becoming one.
|
||||
|
||||
+42
-19
@@ -2,7 +2,9 @@
|
||||
# Make the jq version a job's shell gates run under OBSERVABLE, and any drift LOUD.
|
||||
#
|
||||
# ersatztv#648. Every shell gate in this repo is authored and tested on a developer Mac shipping
|
||||
# jq 1.8.x. The CI runner ships jq 1.6. Nothing pinned or checked that, and until ersatztv#631 the one
|
||||
# jq 1.8.x. The CI runners shipped jq 1.6 when this was written (2026-07-26); as of 2026-10-29 the
|
||||
# `small` fleet is MIXED, 1.6 or 1.7 depending on which host takes the job (server-management#1080).
|
||||
# Nothing pinned or checked that, and until ersatztv#631 the one
|
||||
# thing that could have noticed (scripts/tests/) never ran on the runner. Three independent divergences
|
||||
# surfaced in a single day:
|
||||
#
|
||||
@@ -20,11 +22,15 @@
|
||||
# WHY A FLOOR AND NOT A PIN EVERYWHERE. The obvious fix — bake a pinned jq into the CI toolchain image
|
||||
# (docker/ci/Dockerfile) — provably does NOT cover the gate that actually broke. `.gitea/workflows/
|
||||
# review-verdict.yml` is `runs-on: small`, carries no toolchain-image pin, and per `ci.small-lane-git-only`
|
||||
# the small lane is git-only. It therefore gets the HOST's jq 1.6 no matter what the image contains.
|
||||
# the small lane is git-only. It therefore gets the HOST's jq, whatever the image contains.
|
||||
# That was checked, not assumed (ersatztv#648's first Done-when box).
|
||||
#
|
||||
# So the contract is the other way round: 1.6 is the FLOOR every gate must work on, and it is the
|
||||
# runner's own jq that provides the 1.6 coverage `scripts/tests/` runs under.
|
||||
# So the contract is the other way round: 1.6 is the FLOOR every gate must work on. Both `small`
|
||||
# runners map to the floating runner-images:ubuntu-latest image with no digest pin, so each host keeps
|
||||
# whatever it last pulled (2026-10-29: 1.6 on some, 1.7 on others). Which jq `scripts/tests/` runs
|
||||
# under is therefore decided by which runner takes the job: the 1.6 coverage is INCIDENTAL, not
|
||||
# guaranteed. server-management#1080 proposes a fleet digest pin; when it settles one version, narrow
|
||||
# the `--expect` set in pr-checks.yml to that version.
|
||||
#
|
||||
# TWO MODES, deliberately asymmetric:
|
||||
#
|
||||
@@ -34,14 +40,14 @@
|
||||
# `main`, so a hard pin there would turn any jq upgrade on the runner into a
|
||||
# repo-wide merge deadlock. Observability without a deadlock risk.
|
||||
#
|
||||
# --expect X.Y Additionally assert the version is exactly X.Y, and FAIL if not. Used by the
|
||||
# `script-tests` job. This is the tripwire: `scripts/tests/` currently exercises
|
||||
# the 1.6 path only because the runner happens to ship 1.6. If the runner were
|
||||
# upgraded, that coverage would vanish SILENTLY and the whole class of bug above
|
||||
# would go untested again. Going red forces a human to decide — re-pin, or add a
|
||||
# real 1.6 matrix leg — rather than letting the coverage evaporate unnoticed.
|
||||
# --expect X.Y[,X.Y] Additionally assert the version is exactly one member of the comma-separated
|
||||
# set, and FAIL if not. Used by the `script-tests` job with `1.6,1.7` (since
|
||||
# 2026-10-29, until server-management#1080 pins the fleet). This is the
|
||||
# tripwire: a version outside the set means the runner image moved on and the
|
||||
# coverage the suite assumed has changed, so going red forces a human to decide
|
||||
# rather than letting it change unnoticed.
|
||||
#
|
||||
# Usage: jq-preflight.sh [--expect <major.minor>]
|
||||
# Usage: jq-preflight.sh [--expect <major.minor>[,<major.minor>...]]
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
@@ -156,18 +162,35 @@ if [ "$major" -lt "$min_major" ] || { [ "$major" -eq "$min_major" ] && [ "$minor
|
||||
fi
|
||||
|
||||
if [ -n "$expect" ]; then
|
||||
if [ "${major}.${minor}" != "$expect" ]; then
|
||||
echo "jq-preflight: expected jq ${expect}, found ${major}.${minor}." >&2
|
||||
# --expect is a comma-separated SET. Validate the WHOLE string first: every member must be a plain
|
||||
# <digits>.<digits>, so an empty member (`1.6,`, `,1.6`, `1.6,,1.7`) or a free-form one is a usage
|
||||
# error, never a member that silently matches nothing (or, via a later split, something it should not).
|
||||
member_re='[0-9]{1,9}\.[0-9]{1,9}'
|
||||
if [[ ! "$expect" =~ ^${member_re}(,${member_re})*$ ]]; then
|
||||
echo "jq-preflight: --expect must be <major.minor>[,<major.minor>...]; got '${expect}'" >&2
|
||||
exit 2
|
||||
fi
|
||||
matched=""
|
||||
IFS=',' read -r -a expect_set <<<"$expect"
|
||||
for member in "${expect_set[@]}"; do
|
||||
if [ "$member" = "${major}.${minor}" ]; then
|
||||
matched="yes"
|
||||
fi
|
||||
done
|
||||
if [ -z "$matched" ]; then
|
||||
echo "jq-preflight: expected jq in {${expect}}, found ${major}.${minor}." >&2
|
||||
echo "" >&2
|
||||
echo "This is a TRIPWIRE, not a defect in your change (ersatztv#648). scripts/tests/ was pinned to" >&2
|
||||
echo "jq ${expect} because that is what this runner shipped; it now reports ${major}.${minor}. The ${expect}" >&2
|
||||
echo "coverage the suite assumed has therefore just disappeared, silently — and jq 1.7 altered NUL" >&2
|
||||
echo "handling, exit codes, @base64d and number precision, every one of which a gate here depends on." >&2
|
||||
echo "This is a TRIPWIRE, not a defect in your change (ersatztv#648, #935). scripts/tests/ is run" >&2
|
||||
echo "under whichever jq the runner that took this job ships, and the accepted set is {${expect}}." >&2
|
||||
echo "This runner reports ${major}.${minor}, so the jq the suite assumed has changed — jq 1.7 altered" >&2
|
||||
echo "NUL handling, exit codes, @base64d and number precision, every one of which a gate here" >&2
|
||||
echo "depends on." >&2
|
||||
echo "" >&2
|
||||
echo "Decide explicitly, then update the --expect value in .gitea/workflows/pr-checks.yml:" >&2
|
||||
echo " * re-pin to the new version after re-reading docs/ci-cd.md -> 'The jq contract', or" >&2
|
||||
echo " * widen or re-pin the set after re-reading docs/ci-cd.md -> 'The jq contract'" >&2
|
||||
echo " (server-management#1080 is the fleet digest pin that narrows it to one version), or" >&2
|
||||
echo " * add a real matrix leg that runs the suite under ${MIN_VERSION} as well." >&2
|
||||
exit 1
|
||||
fi
|
||||
echo "jq-preflight: version matches the expected pin (${expect})."
|
||||
echo "jq-preflight: version ${major}.${minor} is a member of the expected set (${expect})."
|
||||
fi
|
||||
|
||||
@@ -79,7 +79,8 @@
|
||||
# ETV_GITEA_URL | GITEA_BASE_URL -> API base; defaults to the homelab Gitea. A value ending in
|
||||
# /api/v1 is used as-is, otherwise /api/v1 is appended.
|
||||
#
|
||||
# jq COMPATIBILITY (ersatztv#648). This runs on the CI runner, which ships **jq 1.6**, while it is
|
||||
# jq COMPATIBILITY (ersatztv#648). This runs on CI runners whose jq is 1.6 or 1.7 (mixed fleet, 2026-10-29;
|
||||
# floor 1.6), while it is
|
||||
# authored on Macs shipping 1.8.x. It is therefore written to the 1.6-compatible subset:
|
||||
# * never rely on `jq -e`'s exit status over EMPTY input — 1.6 exits 0 where >=1.7 exits 4, which is
|
||||
# precisely the fail-open that ersatztv#647 found live in the enforced gate. Emptiness is always
|
||||
@@ -180,7 +181,7 @@ while [ "$page" -le "$MAX_PAGES" ]; do
|
||||
raw=$(gq "repos/$owner/$repo/pulls/$pr/files?limit=${PAGE_SIZE}&page=${page}")
|
||||
|
||||
# An EMPTY body is rejected in SHELL, before jq sees it. `jq -e` over empty input exits 4 on
|
||||
# jq >= 1.7 but 0 on jq 1.6, and the runner ships 1.6 — leaving this to jq's exit status is the
|
||||
# jq >= 1.7 but 0 on jq 1.6, and a runner may still report 1.6 (mixed fleet, 2026-10-29) — leaving this to jq's exit status is the
|
||||
# exact fail-open ersatztv#647 found in the enforced copy. A transport failure must never
|
||||
# masquerade as a legitimate short final page.
|
||||
if [ -z "${raw//[[:space:]]/}" ]; then
|
||||
|
||||
@@ -1,7 +1,8 @@
|
||||
"""Tests for `scripts/jq-preflight.sh` — the jq version contract (ersatztv#648).
|
||||
|
||||
The axis this guards. Every shell gate in this repo is authored on a Mac shipping jq 1.8.x; the CI
|
||||
runner ships jq 1.6. Nothing pinned or checked that, and three independent divergences surfaced in a
|
||||
runners shipped jq 1.6 (2026-07-26) and are now a mix of 1.6 and 1.7 (2026-10-29). Nothing pinned
|
||||
or checked that, and three independent divergences surfaced in a
|
||||
single day — `jq -e` over empty input (exit 4 vs 0), `contains("<NUL>")` (false vs true for every
|
||||
string), and the parse-error exit code (5 vs 4, colliding with "no output"). Each was patched with a
|
||||
version-stable construct, but patching constructs one at a time leaves the AXIS untested.
|
||||
@@ -15,6 +16,7 @@ reproducible: it re-runs on every PR instead of living in one CI run's history.
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
@@ -132,13 +134,16 @@ def test_build_suffixes_still_parse_as_1_6(preflight, version_line):
|
||||
assert preflight.run("--expect", "1.6").returncode == 0, version_line
|
||||
|
||||
|
||||
ACCEPTED = "1.6,1.7" # the set pr-checks.yml passes until server-management#1080 pins the fleet
|
||||
|
||||
|
||||
def test_expect_mismatch_is_LOUD(preflight):
|
||||
"""THE TRIPWIRE. scripts/tests exercises the jq 1.6 path only because the runner ships 1.6. If
|
||||
the runner were upgraded that coverage would vanish silently, so the pin must go red instead."""
|
||||
"""THE TRIPWIRE. A version outside the accepted set must go red, so a runner image moving on
|
||||
changes the suite's jq visibly instead of silently."""
|
||||
preflight.with_jq("jq-1.7.1")
|
||||
r = preflight.run("--expect", "1.6")
|
||||
assert r.returncode == 1
|
||||
assert "expected jq 1.6, found 1.7" in r.stderr
|
||||
assert "expected jq in {1.6}, found 1.7" in r.stderr
|
||||
|
||||
|
||||
def test_expect_match_passes(preflight):
|
||||
@@ -146,6 +151,34 @@ def test_expect_match_passes(preflight):
|
||||
assert preflight.run("--expect", "1.6").returncode == 0
|
||||
|
||||
|
||||
@pytest.mark.parametrize("version_line", ["jq-1.6", "jq-1.7", "jq-1.7.1", "jq-1.6-dirty"])
|
||||
def test_the_accepted_set_passes_on_either_member(preflight, version_line):
|
||||
"""The mixed fleet (server-management#1080): whichever runner takes the job must be green."""
|
||||
preflight.with_jq(version_line)
|
||||
r = preflight.run("--expect", ACCEPTED)
|
||||
assert r.returncode == 0, r.stderr
|
||||
assert "member of the expected set" in r.stdout
|
||||
|
||||
|
||||
@pytest.mark.parametrize("version_line", ["jq-1.8.1", "jq-1.10", "jq-2.6", "jq-1.71"])
|
||||
def test_a_version_outside_the_accepted_set_is_LOUD(preflight, version_line):
|
||||
"""The witnessed red. 1.8 is what the dev Macs ship and 1.10/1.71 are string-prefix lookalikes of
|
||||
1.7/1.6 members: membership is exact on major.minor, not a prefix or numeric-range test."""
|
||||
preflight.with_jq(version_line)
|
||||
r = preflight.run("--expect", ACCEPTED)
|
||||
assert r.returncode == 1, version_line
|
||||
assert "expected jq in {1.6,1.7}" in r.stderr
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad", ["1.6,", ",1.6", "1.6,,1.7", "1.6 1.7", "1.x", "1", "1.6;1.7", "1.6,1.7,"])
|
||||
def test_a_malformed_expect_set_is_a_usage_error(preflight, bad):
|
||||
"""An empty or free-form member must not become a member that matches nothing (or everything)."""
|
||||
preflight.with_jq("jq-1.6")
|
||||
r = preflight.run("--expect", bad)
|
||||
assert r.returncode == 2, bad
|
||||
assert "--expect must be" in r.stderr
|
||||
|
||||
|
||||
def test_unknown_argument_is_a_usage_error(preflight):
|
||||
preflight.with_jq("jq-1.6")
|
||||
assert preflight.run("--pin", "1.6").returncode == 2
|
||||
@@ -345,6 +378,11 @@ def test_script_tests_pins_the_jq_version():
|
||||
"""
|
||||
pr_checks = (WORKFLOWS / "pr-checks.yml").read_text()
|
||||
assert "jq-preflight.sh --expect" in pr_checks, "script-tests must pin the jq version — that pin is the tripwire"
|
||||
# Exactly the mixed-fleet set until server-management#1080 settles one version (#935): not 1.6 alone
|
||||
# (red on a 1.7 runner), not a wider set (a 1.8 runner must stay red).
|
||||
assert re.search(rf"jq-preflight\.sh --expect {re.escape(ACCEPTED)}\s*$", pr_checks, re.M), (
|
||||
f"script-tests must run --expect {ACCEPTED}"
|
||||
)
|
||||
|
||||
|
||||
def test_review_verdict_never_pins_a_jq_version():
|
||||
|
||||
@@ -498,7 +498,7 @@ def test_a_SHORT_page_does_not_end_the_enumeration(enumerate_files):
|
||||
# `test_transport_failure_mid_pagination_fails_closed` above does NOT isolate the explicit
|
||||
# `if [ -z "${raw//[[:space:]]/}" ]` clause — mutation confirms it: deleting that clause leaves
|
||||
# the whole suite green on a developer Mac, because jq 1.8 rejects empty
|
||||
# input on its own. jq 1.6 does not, and the runner ships 1.6 — so the one environment where the
|
||||
# input on its own. jq 1.6 does not, and a runner may report 1.6 — so the one environment where the
|
||||
# clause is load-bearing was the one environment with no coverage. That is the #643/#647 failure
|
||||
# class exactly, reproduced in the test suite meant to prevent it.
|
||||
#
|
||||
@@ -2184,7 +2184,7 @@ def test_a_transport_failure_under_jq_1_6_STILL_refuses_to_classify(tmp_path):
|
||||
"""The ersatztv#647 fail-open, tested where it actually lives: jq 1.6.
|
||||
|
||||
The shell emptiness check exists because `jq -e` over EMPTY input exits 4 on jq >= 1.7 but **0 on
|
||||
jq 1.6, which is what the runner ships**. Remove that check and, on a dev Mac's 1.8, the guard
|
||||
jq 1.6, which a runner may report (the floor)**. Remove that check and, on a dev Mac's 1.8, the guard
|
||||
still fires and every test stays green — the bug is invisible locally.
|
||||
|
||||
Pinning the construct STRUCTURALLY instead, on the grounds that "no behavioural test can catch
|
||||
@@ -3391,7 +3391,7 @@ def test_count_pr_mutations_uses_NO_jq_e(tmp_path):
|
||||
offenders = [ln.strip() for ln in joined if _uses_jq_exit_status(ln)]
|
||||
assert not offenders, (
|
||||
"`jq -e` appeared inside `count_pr_mutations`, whose own comment bans it because the runner "
|
||||
"ships jq 1.6 and `-e`'s exit status over empty input diverges there (ersatztv#647/#803). "
|
||||
"may report jq 1.6 and `-e`'s exit status over empty input diverges there (ersatztv#647/#803). "
|
||||
"Read the value with `jq -r` and range-check it in shell instead:\n " + "\n ".join(offenders)
|
||||
)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user