From 69ed892392247abb9acb50cfed9c35cacb94375d Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 14 Jul 2026 20:11:46 +0200 Subject: [PATCH] fix(canary): distinguish a goldens harness error from real drift The HEAD-drift canary set drift=true on ANY non-zero exit from sync-validator-goldens.sh --check. But that exit also covers harness failures -- missing deps, a bad DATA_INGESTORS_DIR, a generator crash -- so the tracking issue reported "DRIFT -- verdicts/values changed" even when parity was never evaluated, sending someone down the pin-bump playbook for an infra bug. (Cursor Bugbot, Medium, surfaced on the #323 promotion.) sync-validator-goldens.sh --check now uses reserved exit codes so consumers can tell the two apart: 0 in sync 2 HARNESS ERROR -- could not regenerate; parity NOT evaluated (not drift) 3 DRIFT -- verdicts / read-path values changed The required goldens-drift check is unaffected (it reds on any non-zero). The canary adapts the WHOLE tracking issue to the signal via a pin_actionable flag (real drift OR source diff OR unreachable pin): - pin_actionable -> drift framing: the pin-bump Playbook, the "HEAD has drifted" title, and the "close when the pin bump lands" footer. - harness-only (evaluated=false, no source diff, pin reachable) -> harness framing: NO Playbook, a "could not run (harness error)" title, and a "fix the harness, do NOT bump the pin" footer. The harness-error note is purely factual (no bump/don't-bump wording) so it never contradicts the Playbook when a harness error co-occurs with a real source diff. The title is recomputed from the current signal and refreshed on EVERY run (issue edit, not only create) so a persistent issue's title never lags the latest failure mode. "All clear" requires evaluated=true so it cannot mask a harness failure. Verified: --check with no data-ingestors exits 2 with a HARNESS ERROR message and does not mutate goldens.json; the reporting script, exercised across harness-only, harness+source-diff, real-drift, and existing-vs-new-issue scenarios, produces a self-consistent row / note / playbook / footer / title each time; actionlint + shellcheck clean; YAML valid. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/head-drift-canary.yml | 73 +++++++++++++++++++++---- scripts/sync-validator-goldens.sh | 19 ++++++- 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/.github/workflows/head-drift-canary.yml b/.github/workflows/head-drift-canary.yml index e351e214..4d174af2 100644 --- a/.github/workflows/head-drift-canary.yml +++ b/.github/workflows/head-drift-canary.yml @@ -140,22 +140,38 @@ jobs: id: goldens # Same harness the goldens-drift job runs against the PIN, pointed at # HEAD instead. Failure here must not red the run — capture the verdict - # and let the reporting step raise the issue. + # and let the reporting step raise the issue. Distinguish the script's + # exit codes so a HARNESS failure (deps / DATA_INGESTORS_DIR / generator + # crash → exit 2) is reported as "could not evaluate", NOT as a + # verdict/value DRIFT (exit 3): parity was never evaluated, so calling it + # drift would send someone down the pin-bump playbook for an infra bug. env: DATA_INGESTORS_DIR: ${{ github.workspace }}/data-ingestors run: | - if ./scripts/sync-validator-goldens.sh --check >goldens-out.txt 2>&1; then - echo "drift=false" >> "$GITHUB_OUTPUT" - echo "goldens check vs HEAD: in sync" - else - echo "drift=true" >> "$GITHUB_OUTPUT" - echo "goldens check vs HEAD: DRIFT" - fi + rc=0 + ./scripts/sync-validator-goldens.sh --check >goldens-out.txt 2>&1 || rc=$? + case "$rc" in + 0) + echo "drift=false" >> "$GITHUB_OUTPUT" + echo "evaluated=true" >> "$GITHUB_OUTPUT" + echo "goldens check vs HEAD: in sync" ;; + 3) + echo "drift=true" >> "$GITHUB_OUTPUT" + echo "evaluated=true" >> "$GITHUB_OUTPUT" + echo "goldens check vs HEAD: DRIFT" ;; + *) + # Exit 2 (harness error) or any unexpected non-zero: parity was + # never evaluated. Not drift — surface it truthfully. + echo "drift=false" >> "$GITHUB_OUTPUT" + echo "evaluated=false" >> "$GITHUB_OUTPUT" + echo "goldens check vs HEAD: HARNESS ERROR (exit $rc) — parity NOT evaluated" ;; + esac tail -n 20 goldens-out.txt || true - name: Open or update the tracking issue if: >- steps.goldens.outputs.drift == 'true' || + steps.goldens.outputs.evaluated == 'false' || steps.srcdiff.outputs.changed != '' || steps.srcdiff.outputs.pin_reachable == 'false' env: @@ -165,9 +181,25 @@ jobs: HEAD_SHA: ${{ steps.srcdiff.outputs.head_sha }} PIN_REACHABLE: ${{ steps.srcdiff.outputs.pin_reachable }} GOLDENS_DRIFT: ${{ steps.goldens.outputs.drift }} + GOLDENS_EVALUATED: ${{ steps.goldens.outputs.evaluated }} CHANGED: ${{ steps.srcdiff.outputs.changed }} RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} run: | + if [ "$GOLDENS_DRIFT" = "true" ]; then + goldens_cell='DRIFT — verdicts/values changed' + elif [ "$GOLDENS_EVALUATED" = "false" ]; then + goldens_cell='could not evaluate — harness error (deps / DATA_INGESTORS_DIR / generator); parity NOT checked this run' + else + goldens_cell='in sync' + fi + # pin_actionable = there is a real pin/drift signal the pin-bump + # playbook addresses. A harness-only failure (evaluated=false with no + # source diff and a reachable pin) must NOT push the pin-bump path. + if [ "$GOLDENS_DRIFT" = "true" ] || [ -n "$CHANGED" ] || [ "$PIN_REACHABLE" = "false" ]; then + pin_actionable=true + else + pin_actionable=false + fi # shellcheck disable=SC2016 # single-quoted printf formats carry markdown backticks, not expansions { printf '\n' @@ -175,30 +207,46 @@ jobs: printf '| | |\n|---|---|\n' printf '| pin (`scripts/.data-ingestors-ref`) | `%s` |\n' "$PIN" printf '| data-ingestors develop HEAD | `%s` |\n' "$HEAD_SHA" - printf '| goldens check vs HEAD | %s |\n' "$([ "$GOLDENS_DRIFT" = "true" ] && echo 'DRIFT — verdicts/values changed' || echo 'in sync')" + printf '| goldens check vs HEAD | %s |\n' "$goldens_cell" printf '| pin reachable from HEAD | %s |\n\n' "${PIN_REACHABLE:-unknown}" if [ "$PIN_REACHABLE" = "false" ]; then printf '**The pinned SHA is not reachable in the upstream clone** — upstream history was rewritten or the ref file is wrong. Fix the pin first.\n\n' fi + if [ "$GOLDENS_EVALUATED" = "false" ]; then + printf '**The goldens check could not run at HEAD** (harness error — missing deps, bad `DATA_INGESTORS_DIR`, or a generator crash). Parity was NOT evaluated this run, so goldens drift at HEAD is unverified — see the canary run logs. What to do is in the footer below.\n\n' + fi if [ -n "$CHANGED" ]; then printf '**Mirrored validator source files changed between pin and HEAD** (may include verdict-neutral changes with no corpus case yet — the di#365 class):\n\n```\n' printf '%s\n' "$CHANGED" printf '```\n\n' fi - printf '**Playbook** (pin-bump doctrine, backend#1009 / cli#286 shape): bump `scripts/.data-ingestors-ref` to the new HEAD, run `scripts/sync-schema.sh` + `scripts/sync-validator-goldens.sh`, adopt/audit the intervening upstream changes in the `internal/push` mirror, and ship the regenerated goldens in the same PR.\n\n' - printf '_Advisory only — this canary never blocks CI. Close this issue when the pin bump lands._\n' + if [ "$pin_actionable" = "true" ]; then + printf '**Playbook** (pin-bump doctrine, backend#1009 / cli#286 shape): bump `scripts/.data-ingestors-ref` to the new HEAD, run `scripts/sync-schema.sh` + `scripts/sync-validator-goldens.sh`, adopt/audit the intervening upstream changes in the `internal/push` mirror, and ship the regenerated goldens in the same PR.\n\n' + printf '_Advisory only — this canary never blocks CI. Close this issue when the pin bump lands._\n' + else + printf '_Advisory only — this canary never blocks CI. No drift or source change this run — only the goldens harness failed to run. Fix the harness (see run logs); close this issue once the next canary run is clean. Do NOT bump the pin for this._\n' + fi } > body.md gh label create head-drift-canary --repo "$REPO" --force \ --description "auto-filed by head-drift-canary.yml (cli#289)" --color D93F0B existing="$(gh issue list --repo "$REPO" --label head-drift-canary --state open \ --json number --jq '.[0].number // empty')" + # Title reflects the CURRENT signal; refresh it on every run so a + # persistent issue's title (and board grouping) never lags the latest + # failure mode when only the body is updated. + if [ "$pin_actionable" = "true" ]; then + title="data-ingestors HEAD has drifted from the CLI's pin (canary)" + else + title="data-ingestors HEAD-drift canary could not run (harness error)" + fi if [ -n "$existing" ]; then echo "updating existing tracking issue #$existing" + gh issue edit "$existing" --repo "$REPO" --title "$title" gh issue comment "$existing" --repo "$REPO" --body-file body.md else echo "opening new tracking issue" gh issue create --repo "$REPO" \ - --title "data-ingestors HEAD has drifted from the CLI's pin (canary)" \ + --title "$title" \ --body-file body.md \ --label head-drift-canary fi @@ -206,6 +254,7 @@ jobs: - name: All clear if: >- steps.goldens.outputs.drift != 'true' && + steps.goldens.outputs.evaluated == 'true' && steps.srcdiff.outputs.changed == '' && steps.srcdiff.outputs.pin_reachable == 'true' env: diff --git a/scripts/sync-validator-goldens.sh b/scripts/sync-validator-goldens.sh index 9170e1b0..597bed21 100755 --- a/scripts/sync-validator-goldens.sh +++ b/scripts/sync-validator-goldens.sh @@ -11,11 +11,26 @@ set -euo pipefail cd "$(dirname "$0")/.." GOLDENS="internal/push/testdata/parity/goldens.json" PYTHON="${PYTHON:-python3}" +# --check exit codes are a contract consumers rely on to tell a real drift +# apart from a broken harness (the HEAD-drift canary must NOT report an infra +# failure as a verdict/value drift): +# 0 in sync +# 2 HARNESS ERROR — could not regenerate (deps / DATA_INGESTORS_DIR / +# generator crash); parity was NEVER evaluated. Not drift. +# 3 DRIFT — the ingestor's verdicts or read-path VALUES changed. if [[ "${1:-}" == "--check" ]]; then tmp="$(mktemp -d)" trap 'rm -rf "$tmp"' EXIT cp "$GOLDENS" "$tmp/committed.json" - "$PYTHON" scripts/gen-validator-goldens.py >/dev/null + # Regenerating from the REAL validators is the HARNESS step. A failure here + # means parity was never evaluated — surface it as the reserved harness code, + # not as drift. (The generator writes GOLDENS in place, so restore first.) + if ! "$PYTHON" scripts/gen-validator-goldens.py >/dev/null; then + cp "$tmp/committed.json" "$GOLDENS" # restore — check must not mutate + echo "HARNESS ERROR: could not regenerate goldens (deps / DATA_INGESTORS_DIR /" >&2 + echo "generator crash) — parity was NOT evaluated. This is not drift." >&2 + exit 2 + fi # Compare VERDICTS only — error text may drift harmlessly (and embeds # fixture paths); verdicts may not. VALUE-level goldens (resolved label + # row count + class set) carry no paths, so compare them too — a value-only @@ -32,7 +47,7 @@ sys.exit(0 if view(a)==view(b) else 1) echo "DRIFT: the ingestor's validator verdicts or read-path VALUES changed. Re-run" >&2 echo "the generator, commit the new goldens, and update cases.json (+ the Go preview)" >&2 echo "consciously." >&2 - exit 1 + exit 3 fi cp "$tmp/committed.json" "$GOLDENS" # keep the committed copy (paths etc. unchanged) echo "validator goldens in sync"