From d44b949223485f1c23c5a479f19454721448c29f Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Wed, 8 Jul 2026 13:27:21 +0200 Subject: [PATCH 1/3] ci: pin the data-ingestors schema ref + add a coverage floor (#1009 P0.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two of the three CI drift tripwires from #1009. (The third — a scheduled `sync-validator-goldens.sh --check` against the pinned ingestor — waits for #340 to land on a stable ref, since the value-aware check regenerates against the ingestor read path and needs the #340 fix; the Go-side parity test already enforces the committed goldens on every PR.) (b) Pin the schema-drift check to a data-ingestors SHA, not floating master. sync-schema.sh now builds the fetch URL from scripts/.data-ingestors-ref (a pinned commit SHA), overridable via DATA_INGESTORS_REF. Before, any upstream commit touching the schema reddened every open CLI PR until someone synced; now adopting upstream is a deliberate SHA bump + re-sync in one PR. Pinned to 0de1f148 (current master; the embedded schema matches it — check stays green). (c) Add a per-package coverage floor. `go test -cover` printed numbers and asserted nothing, so the two load-bearing, historically thin-tested packages could silently rot. scripts/coverage-floor.sh fails the build if internal/cli or internal/submit drops below its floor; wired into the Test job after `go test`. Floors are a RATCHET (bump UP only, lowering is a reviewed edit), set just under current develop: internal/cli 68% (now 70.1%), internal/submit 72% (now 74.8%). Bump these up once #186/#187 land (they lift internal/cli to ~72%). The script is bash-3.2-portable (no associative arrays — macOS default). Verified locally: sync-schema.sh --check matches at the pinned SHA; coverage-floor.sh passes and bites (a 99% floor fails, a below-current floor passes); shellcheck clean; build.yml parses; full suite green. Part of backend#1009 (P0 CI drift tripwires). Remaining #1009: the goldens drift job (post-#340), the cross-repo taxonomy contract test, one content-compared ingest e2e. Part of the data-ingest epic backend#1008. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/build.yml | 20 ++++++++++++---- scripts/.data-ingestors-ref | 12 ++++++++++ scripts/coverage-floor.sh | 47 +++++++++++++++++++++++++++++++++++++ scripts/sync-schema.sh | 21 ++++++++++++++--- 4 files changed, 92 insertions(+), 8 deletions(-) create mode 100644 scripts/.data-ingestors-ref create mode 100755 scripts/coverage-floor.sh diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f0a63d6..13432eb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,11 +23,13 @@ jobs: timeout-minutes: 10 name: Schema drift check # Verifies the embedded internal/schema/ingest.v1.json matches - # tracebloc/data-ingestors' master. A green PR that silently - # diverges from upstream is a real correctness hazard — a - # customer's YAML could pass `tracebloc ingest validate` locally - # but be rejected by jobs-manager (or vice versa). Forcing the - # sync as a PR step keeps drift visible. + # tracebloc/data-ingestors at the PINNED ref (scripts/.data-ingestors-ref), + # not a floating branch. A green PR that silently diverges from the schema + # jobs-manager enforces is a real correctness hazard — a customer's YAML + # could pass `tracebloc ingest validate` locally but be rejected in-cluster + # (or vice versa). Pinning stops an unrelated upstream commit from redding + # every open CLI PR; adopting upstream is a deliberate SHA bump + re-sync + # (backend#1009). runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 @@ -77,6 +79,14 @@ jobs: # us from having to retrofit it later. run: go test -race -cover ./... + - name: Coverage floor (internal/cli, internal/submit must not rot) + # `go test -cover` above prints numbers but asserts nothing. This + # enforces a per-package floor on the two load-bearing, historically + # thin-tested packages (the money path + submit orchestration) so a + # test deletion can't silently drop coverage. Floors ratchet UP only — + # see scripts/coverage-floor.sh (backend#1009). + run: ./scripts/coverage-floor.sh + lint: timeout-minutes: 10 name: Lint diff --git a/scripts/.data-ingestors-ref b/scripts/.data-ingestors-ref new file mode 100644 index 0000000..c441fc2 --- /dev/null +++ b/scripts/.data-ingestors-ref @@ -0,0 +1,12 @@ +# Pinned tracebloc/data-ingestors commit the CLI's embedded schema (and, when +# the goldens drift job lands, the validator goldens) are synced from. CI +# checks against THIS ref, not a floating branch — so an unrelated upstream +# commit can't red every open CLI PR (backend#1009). +# +# To adopt upstream changes: bump this SHA, then run `scripts/sync-schema.sh` +# (and `scripts/sync-validator-goldens.sh` once its CI job exists) and commit +# the regenerated files together in one deliberate PR. +# +# Format: the first non-comment, non-blank line is the ref (a full commit SHA +# preferred; a branch name works but reintroduces floating drift). +0de1f148f9f19c8838d275ab9e5295ae224385c2 diff --git a/scripts/coverage-floor.sh b/scripts/coverage-floor.sh new file mode 100755 index 0000000..afc05a3 --- /dev/null +++ b/scripts/coverage-floor.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# Fail if a load-bearing package's statement coverage drops below its floor. +# +# internal/cli (the money path — submit → classify → JSON → reclaim) and +# internal/submit (the jobs-manager orchestration) were historically the +# thinnest-tested, highest-stakes code in the CLI (backend#1009). A bare +# `go test -cover` prints the number and asserts nothing, so coverage could +# silently rot. This gate makes a regression loud. +# +# The floors are a RATCHET: set just under the current numbers, and bumped UP +# as coverage improves — never silently down. Lowering a floor must be a +# deliberate, reviewed edit here (with a reason), not a side effect of deleting +# tests. Current (develop): internal/cli ~70%, internal/submit ~75%. +# +# Usage: scripts/coverage-floor.sh (run from the repo root) +# +# Portable to bash 3.2 (macOS default): no associative arrays. +set -euo pipefail + +# "package:floor" entries. Keep floors integers; coverage is compared as a +# float against them. +FLOORS=" +internal/cli:68 +internal/submit:72 +" + +status=0 +for entry in $FLOORS; do + pkg="${entry%%:*}" + min="${entry##*:}" + line="$(go test -cover "./$pkg/" 2>/dev/null | grep -E 'coverage: [0-9]' || true)" + pct="$(printf '%s\n' "$line" | sed -nE 's/.*coverage: ([0-9]+(\.[0-9]+)?)% of statements.*/\1/p' | head -1)" + if [ -z "$pct" ]; then + echo "::error::could not read coverage for ./$pkg/ (did any test run?)" >&2 + status=1 + continue + fi + # awk exits 0 when pct < min (i.e. below the floor → failure). + if awk "BEGIN{exit !($pct < $min)}"; then + echo "::error::./$pkg/ coverage ${pct}% is below the floor ${min}% — add tests, or (with a reason) lower the floor in scripts/coverage-floor.sh" >&2 + status=1 + else + echo "ok: ./$pkg/ ${pct}% >= ${min}%" + fi +done + +exit "$status" diff --git a/scripts/sync-schema.sh b/scripts/sync-schema.sh index a7f13d4..c17ab67 100755 --- a/scripts/sync-schema.sh +++ b/scripts/sync-schema.sh @@ -16,8 +16,15 @@ # scripts/sync-schema.sh --check # verify in-tree copy matches upstream; exit non-zero on drift # # Env knobs: -# SCHEMA_SOURCE_URL override the upstream URL (default: data-ingestors' master) -# SCHEMA_OUT override the in-tree destination (default: internal/schema/ingest.v1.json) +# SCHEMA_SOURCE_URL override the upstream URL (default: built from the +# pinned ref below) +# DATA_INGESTORS_REF override the data-ingestors ref (default: the pinned +# SHA in scripts/.data-ingestors-ref, else master) +# SCHEMA_OUT override the in-tree destination (default: internal/schema/ingest.v1.json) +# +# The ref is PINNED (scripts/.data-ingestors-ref), not a floating branch, so an +# unrelated upstream commit doesn't red every open CLI PR — adopting upstream +# is a deliberate SHA bump + re-sync (backend#1009). # # Future: when we cut a v2 schema, this script will need to learn # about multiple versions (e.g. embed v1 AND v2 side-by-side, picked @@ -26,7 +33,15 @@ set -euo pipefail -readonly DEFAULT_URL="https://raw.githubusercontent.com/tracebloc/data-ingestors/master/tracebloc_ingestor/schema/ingest.v1.json" +# The pinned data-ingestors ref: first non-comment, non-blank line of the ref +# file (a full commit SHA), overridable via DATA_INGESTORS_REF, falling back to +# master if the file is somehow absent. +REF_FILE="$(cd "$(dirname "$0")" && pwd)/.data-ingestors-ref" +readonly REF_FILE +_pinned_ref="$(grep -vE '^[[:space:]]*(#|$)' "$REF_FILE" 2>/dev/null | head -1 | tr -d '[:space:]' || true)" +DATA_INGESTORS_REF="${DATA_INGESTORS_REF:-${_pinned_ref:-master}}" + +readonly DEFAULT_URL="https://raw.githubusercontent.com/tracebloc/data-ingestors/${DATA_INGESTORS_REF}/tracebloc_ingestor/schema/ingest.v1.json" readonly DEFAULT_OUT="internal/schema/ingest.v1.json" SCHEMA_SOURCE_URL="${SCHEMA_SOURCE_URL:-$DEFAULT_URL}" From 97186d3c74c96f1fe6e0c6dd443adff3be0aedf5 Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 8 Jul 2026 21:03:55 +0500 Subject: [PATCH 2/3] ci: fail loudly on a malformed coverage-floor entry (#1009 P0.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dropped ":floor" in FLOORS left min="$entry" (the whole token); the awk comparison then errored on that as bare source and exited non-zero, which `if awk` read as "not below floor" and printed a bogus "ok" — the ratchet became a silent no-op for that package. Validate each entry has a "package:INT" shape before the awk call. Co-Authored-By: Claude Opus 4.8 --- scripts/coverage-floor.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/scripts/coverage-floor.sh b/scripts/coverage-floor.sh index afc05a3..e9775f4 100755 --- a/scripts/coverage-floor.sh +++ b/scripts/coverage-floor.sh @@ -28,6 +28,16 @@ status=0 for entry in $FLOORS; do pkg="${entry%%:*}" min="${entry##*:}" + # A malformed entry (no ":floor", or a non-integer floor) must fail loudly, + # not slip through. Without this, a dropped colon leaves min="$entry" (the + # whole token); the awk comparison below then errors on that as bare source + # and exits non-zero — which `if awk` reads as "not below floor", prints a + # bogus "ok", and turns the ratchet into a silent no-op for that package. + if [ "$pkg" = "$entry" ] || ! printf '%s' "$min" | grep -qE '^[0-9]+$'; then + echo "::error::malformed FLOORS entry '$entry' (want 'package:INT') — fix scripts/coverage-floor.sh" >&2 + status=1 + continue + fi line="$(go test -cover "./$pkg/" 2>/dev/null | grep -E 'coverage: [0-9]' || true)" pct="$(printf '%s\n' "$line" | sed -nE 's/.*coverage: ([0-9]+(\.[0-9]+)?)% of statements.*/\1/p' | head -1)" if [ -z "$pct" ]; then From 18b4e4314a3669639ed10bbef9fdba827147392c Mon Sep 17 00:00:00 2001 From: Asad Iqbal Date: Wed, 8 Jul 2026 21:10:04 +0500 Subject: [PATCH 3/3] ci: validate the data-ingestors ref before it enters the schema URL (#1009 P0.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ref (pinned file value, or the DATA_INGESTORS_REF override) is interpolated into a raw.githubusercontent.com URL. An unvalidated ref could inject path traversal ("../..") or extra path segments — the same class scripts/install.sh already guards for its release tag. Restrict the ref to a SHA/branch/tag shape (alnum start; alnum . _ - / ; no "..") and fail with exit 2 otherwise. Co-Authored-By: Claude Opus 4.8 --- scripts/sync-schema.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/scripts/sync-schema.sh b/scripts/sync-schema.sh index c17ab67..d4b42a4 100755 --- a/scripts/sync-schema.sh +++ b/scripts/sync-schema.sh @@ -41,6 +41,19 @@ readonly REF_FILE _pinned_ref="$(grep -vE '^[[:space:]]*(#|$)' "$REF_FILE" 2>/dev/null | head -1 | tr -d '[:space:]' || true)" DATA_INGESTORS_REF="${DATA_INGESTORS_REF:-${_pinned_ref:-master}}" +# The ref is interpolated into a download URL, so validate it before use +# (like scripts/install.sh does for its release tag): a crafted ref — most +# plausibly via the DATA_INGESTORS_REF override — could otherwise inject path +# traversal ("../..") or extra segments into the raw.githubusercontent path. +# Allow only a SHA / branch / tag shape: alnum start, then alnum . _ - / and +# no ".." component. +if ! printf '%s' "$DATA_INGESTORS_REF" | grep -qE '^[A-Za-z0-9][A-Za-z0-9._/-]*$' \ + || printf '%s' "$DATA_INGESTORS_REF" | grep -q '\.\.'; then + echo "error: invalid data-ingestors ref '$DATA_INGESTORS_REF' — expected a commit SHA, branch, or tag" >&2 + echo "(set it in scripts/.data-ingestors-ref or via DATA_INGESTORS_REF)" >&2 + exit 2 +fi + readonly DEFAULT_URL="https://raw.githubusercontent.com/tracebloc/data-ingestors/${DATA_INGESTORS_REF}/tracebloc_ingestor/schema/ingest.v1.json" readonly DEFAULT_OUT="internal/schema/ingest.v1.json"