Uh oh!
There was an error while loading. Please reload this page.
fix(sync): watch the data-ingestors pin for contract-version drift (backend#2704) - #595
Conversation
…ackend#2704) `scripts/sync-schema.sh --check` verifies the vendored contract matches the PINNED SHA — it answers "does my copy match what I pinned?", never "is what I pinned still current?". So the pin silently went a whole contract VERSION behind (layout v2 vs data-ingestors@develop v3, di#535) while every drift gate stayed green. The pin is deliberate and stays (backend#1009: a floating ref reds every open CLI PR on unrelated upstream commits). Add a watcher for the pin itself: - scripts/check-pin-version.sh resolves the pin and data-ingestors' current default branch, fetches layout.v1.json at both, and compares the top-level "version" — VERSION, not commit distance (180 unrelated commits are fine). FAILS CLOSED: absent/malformed pin, moved path, unreachable raw URL, non-JSON, or missing "version" all exit non-zero. Exit 0 in sync, 1 drift, 2 can't-eval. - .github/workflows/pin-version-drift.yml runs it weekly (Mon 06:30 UTC, after head-drift-canary) + workflow_dispatch. No pull_request trigger — gating PRs on live upstream is the backend#1009 failure. The pinned PR comparison is unchanged. - scripts/tests/pin-version-verify.sh mutation-proves it hermetically (drift reddens; each cannot-evaluate reddens), wired into build.yml alongside the other fail-closed harnesses. - BUGBOT.md: record the new scheduled-only, never-PR-gating workflow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
left a comment
There was a problem hiding this comment.
This is the systemic fix for the drift I chased this morning (data-ingestors layout contract v2 → v3 landing while e2e-test-agent still declared {"2"}), and it's built the way the house bar asks. I verified rather than took it on trust.
What I checked
It derives the answer. It fetches the real layout.v1.json at both refs and compares the top-level "version" — no second copy of the rule. Version rather than commit distance is the right axis: it's what makes 180 unrelated upstream commits stay green.
The harness calls the code under test, and refuses to report a pass if the checker is missing:
CHECKER="${REPO_ROOT}/scripts/check-pin-version.sh"
[ -f"$CHECKER" ] || { echo"... refusing to report pass">&2;exit 2; }That's the thing usually got wrong — a verifier that re-implements the rule inline goes on proving a copy of itself.
Mutation-proved, both directions, anchors asserted first. 7/7 green as shipped, then:
MUT 1 `if [[ "$pin_ver" == "$head_ver" ]]` -> `if true` -> 6 passed, 1 failed (only the drift case — correct)
MUT 2 die_closed's `exit 2` -> `exit 0` -> 2 passed, 5 failed (all five fail-closed arms)
Mutation 1 reddening only the drift case is the right shape — the fail-closed cases never reach the comparison, so a broader redness would have meant they were coupled to something they shouldn't be.
And the plumbing is right where it's easy to get wrong: both scripts are 100755 in the tree (a run: ./script step on a non-executable file is a classic), the action is SHA-pinned, permissions: contents: read, timeout-minutes, and the private-repo caveat about CROSS_REPO_READ_TOKEN is written down rather than discovered later.
One thing I'd settle before merge, and it's two facts that compound
Separately each is fine; together they're the failure this whole class is about.
- It lands red on purpose — you say so plainly, and the reasoning (behaviour and tooling in separate PRs) is our own rule.
- A red run is the only signal. No issue is opened, no one is assigned. Compare
head-drift-canary.yml, which this file names as its complement — that one carriesissues: writeand doesgh issue create/edit/commentwith ahead-drift-canarylabel, so its finding lands somewhere a person will see.
So the first Monday cron reds, nobody is told, and red becomes this workflow's normal state. That is "a mechanism that appears to verify something but is not connected to what it claims to check" — it just gets there by a different route than usual, since the checker itself is genuinely correct.
Either of these closes it, and I'll approve on either:
- Order the landing: merge this, then land the re-pin (
.data-ingestors-refbump +sync-schema.sh) before Monday 06:30 UTC, so the gate arms green. Two PRs, as you want — just sequenced. - Or give it the canary's reporting path:
issues: writeplus a create-or-update on apin-version-driftlabel. Then a red run reaches someone even if it's red from run one.
Smaller — the one path it restates
readonly LAYOUT_SUBPATH="tracebloc_ingestor/schema/layout.v1.json"# keep in lockstep with the UPSTREAM_BASE/layout.v1.json path in scripts/sync-schema.shA comment saying "keep in lockstep" is a claim that could be a machine check, and here it's cheap — sync-schema.sh:64 declares the segment and :75 names the file:
readonly UPSTREAM_BASE=".../data-ingestors/${DATA_INGESTORS_REF}/tracebloc_ingestor/schema""${UPSTREAM_BASE}/layout.v1.json|internal/schema/layout.v1.json"Deriving it makes this the env-vocabulary-agreement.sh shape — parse the real declaration, hold no copy. It does fail closed today (a moved path 404s to exit 2), so this is hygiene rather than a hole; I'd take it now while the file is open.
Good call self-flagging the .client-ref / .backend-ref / .client-runtime-ref class instead of leaving it implied — that's the part of the class rule people skip.
…yout path (backend#2704) Addresses LukasWodka's review on cli#595. - A red scheduled run was the ONLY signal — no issue, no assignee — so the first Monday cron would red, nobody would be told, and red would become the workflow's normal state. pin-version-drift.yml now opens/updates ONE `pin-version-drift`-labelled tracking issue (create-or-update, like head-drift-canary.yml) on drift OR any cannot-evaluate, AND still fails closed. Red run + filed issue: the finding lands where a person sees it, and "cannot tell" is still never a green pass. Chose this over sequencing the re-pin so the watcher's value doesn't hinge on landing a second PR before Monday 06:30 UTC. - The vendored layout path is now DERIVED from sync-schema.sh (derive_layout_subpath) instead of held behind a "keep in lockstep" comment — the claim is now a machine check, asserted by pin-version-verify.sh, and fails closed if sync-schema.sh's declarations move. main derives it only when a URL isn't overridden, so the hermetic harness stays offline. check-pin-version.sh is source-guarded so the test can call the function directly. Verify: 9/9 hermetic cases (added unparseable-sync-schema fail-closed + the derivation assertion); shellcheck --severity=error + bash -n clean; both workflows YAML-valid; live: real pin v2 vs develop v3 -> exit 1, develop vs develop -> exit 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aptracebloc
commented
Aug 27, 2026
Thanks — this is exactly the failure mode the whole class is about, and you're right that a red run with no assignee just becomes the workflow's normal state. Both points addressed in The reporting path (your Option B). I took the canary's reporting path rather than sequencing the re-pin, so the watcher's value doesn't hinge on landing a second PR before Monday 06:30 UTC.
So it's a red run and a filed issue a person sees — "cannot tell" is still never a green pass, and the finding now lands somewhere even from run one. The re-pin stays the separate ordinary sync the issue calls for; when it lands, closing the tracking issue is the documented step. The one restated path. Verify: 9/9 hermetic cases (added an unparseable- |
aptracebloc
commented
Aug 27, 2026
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 51fdb2f. Configure here.
LukasWodka
left a comment
There was a problem hiding this comment.
Re-reviewed 51fdb2f5. Both points are closed and I verified each. Holding approval only on the three still-pending checks — nothing is waiting on you; I'll approve on green.
The reporting path — and you took the stronger option
You did the canary's create-or-update and kept the run red:
permissions: { issues: write }gh issue list --label pin-version-drift --state open # at most ONE, keyed by label-> edit title + comment, or create
- name: Fail closed on drift or cannot-evaluateif: steps.check.outputs.rc != '0'I'd have accepted either; both is right. The issue is what reaches a person, the red run is what a human scanning Actions sees, and neither alone survives the first-run-is-red problem — an issue with no red looks advisory, a red with no issue is the thing people learn to ignore. Keying on the label rather than the title also means a month of drift is one thread instead of four.
That also removes my sequencing concern entirely: the watcher's value no longer depends on landing the re-pin before Monday 06:30 UTC.
The derived path — the lockstep comment is now a real mechanism
derive_layout_subpath parses both declarations out of sync-schema.sh rather than holding a copy. Against the real file:
derived: tracebloc_ingestor/schema/layout.v1.json ✓ (matches the hardcoded value it replaced)
And it actually follows, which is the whole point — I moved the schema dir in sync-schema.sh (anchor asserted first) and the derivation tracked it:
sync-schema.sh: ${DATA_INGESTORS_REF}/tracebloc_ingestor/schema -> ${DATA_INGESTORS_REF}/newlocation/schema
derived now: newlocation/schema/layout.v1.json
A hardcoded LAYOUT_SUBPATH would have gone on 404-ing at exit 2 until someone read the log. This is the env-vocabulary-agreement.sh shape: parse the real declaration, hold none.
The harness grew the two cases it needed
9 passed, 0 failed, up from 7 — and the two additions are exactly the right pair:
ok: fail-closed: unparseable sync-schema.sh -> exit 2
ok: derive_layout_subpath matches sync-schema.sh (tracebloc_ingestor/schema/layout.v1.json)
One pins the value, one pins that "cannot tell" is a finding rather than a pass. Deriving a path introduces a new way to not-know, and covering that in the same commit is what stops the fix from trading one silent failure for another.
Nothing further from me — this is a good piece of work, and the create-or-update block is now the second in-house instance of that pattern, which makes it worth lifting somewhere reusable the next time a third repo needs it.
LukasWodka
left a comment
There was a problem hiding this comment.
Approving 51fdb2f5 — green, no open threads, and the head is byte-identical to the one I verified last pass (I re-compared check-pin-version.sh and pin-version-verify.sh rather than assuming, since the re-request came without a new commit).
Both of my points were closed, and the verification stands:
- The reporting path. You took create-or-update on a
pin-version-drift-labelled issue and kept the run red. Either would have satisfied me; both is right, because an issue with no red reads as advisory and a red with no issue is what people learn to ignore. It also removes the sequencing concern entirely — the watcher no longer depends on landing the re-pin before Monday 06:30 UTC. - The derived path.
derive_layout_subpathgenuinely follows: moving the schema dir insync-schema.shmoved the derived value tonewlocation/schema/layout.v1.json, where the old hardcoded constant would have 404'd to exit 2 indefinitely. The lockstep comment is now a mechanism rather than a hope. - The harness grew the right pair —
9 passed, adding one case that pins the derived value and one that makes an unparseablesync-schema.sha finding rather than a pass. Deriving a path introduces a new way to not-know, and covering that in the same commit is what stops the fix trading one silent failure for another.
The whole thing is the shape the house bar asks for: parse the real declaration, hold no copy, fail closed on "cannot tell", and prove it by breaking the thing.
One forward-looking note, no action here: this is now the second in-house instance of the create-or-update-a-labelled-issue block (head-drift-canary.yml being the first, and this one modelled on it). A third repo needing it is the point at which it wants to be a reusable workflow in tracebloc/.github rather than a third copy — worth remembering when someone reaches for it again.
Uh oh!
There was an error while loading. Please reload this page.
Fixes tracebloc/backend#2704 — https://github.com/tracebloc/backend/issues/2704
The gap
scripts/sync-schema.sh --check(build.yml, and the drift-workflow family) compares the vendoredcontract against the pinned SHA in
scripts/.data-ingestors-ref. It answers "does my copymatch what I pinned?" — never "is what I pinned still current?" So the pin silently went a whole
contract VERSION behind while every drift gate stayed green:
scripts/.data-ingestors-ref)8f89aece— 2026-07-13cli@developdata-ingestors@developThe pin is deliberate and stays (backend#1009: a floating ref reds every open CLI PR on
unrelated upstream commits). This PR does not float it — it adds a watcher for the pin itself.
What this adds
scripts/check-pin-version.sh— resolves the pin and data-ingestors' current defaultbranch, fetches
tracebloc_ingestor/schema/layout.v1.jsonat both, and compares the top-level"version". Version, not commit distance — 180 unrelated commits stay green; a version bumpis the finding. Fails closed: absent/malformed pin, moved path, unreachable raw URL, non-JSON,
or missing
"version"all exit non-zero (0 = in sync, 1 = drift, 2 = cannot-evaluate)..github/workflows/pin-version-drift.yml— runs it weekly (Mon 06:30 UTC, afterhead-drift-canary) +
workflow_dispatch. Nopull_requesttrigger — gating PRs on liveupstream is the backend#1009 failure this repo removed. The pinned PR comparison
(
sync-schema.sh --check) is unchanged.scripts/tests/pin-version-verify.sh— hermetic (file:// fixtures, no network/gh),mutation-proves that drift reddens and every cannot-evaluate reddens. Wired into
build.ymlalongside the other fail-closed harnesses (shellcheck +
bash -n+ run)..cursor/BUGBOT.md— records the new scheduled-only, never-PR-gating workflow.Verification
shellcheck (
--severity=error) +bash -nclean on both scripts; temp-file-leak delta confirmed 0.Notes for the reviewer
Clearing it is the separate, ordinary re-pin the issue calls for (bump
.data-ingestors-ref,re-run
sync-schema.sh) — intentionally not bundled here (behavior vs. tooling stay inseparate PRs).
.client-ref,.backend-ref,and
.client-runtime-refshare the same pin-freshness shape, but each guards a different contractwith its own drift job (chart-drift, backend-fixtures-drift, envelope-contract-drift) and a
different notion of "version". #2704 is scoped to the data-ingestors layout contract; extending
the pattern to the others is follow-up work, not silently covered by this PR.
Note
Low Risk
Additive CI/scripts only; no runtime CLI, ingest validation, or pin file changes in this PR.
Overview
Adds a scheduled watcher so the CLI can detect when
scripts/.data-ingestors-refpoints at a stale layout contract version (top-levelversionin upstreamlayout.v1.json), not just when vendored files match that pin — the gap that let v2 stay pinned while upstream moved to v3 with all PR gates green.scripts/check-pin-version.shcompares contract version at the pin vs data-ingestors’ current default branch (version bump = drift, not commit distance). It fails closed on cannot-evaluate cases (exit 2) and reports drift (exit 1).pin-version-drift.ymlruns weekly +workflow_dispatchonly — nopull_requesttrigger — and on failure opens or updates a singlepin-version-driftissue before reddening the job.build.ymlwires hermeticpin-version-verify.sh(file fixtures, drift + fail-closed cases) plus shellcheck/bash -nfor the new scripts. BUGBOT.md records that this workflow never gates merges.This PR does not bump the pin or regenerate
internal/schema/*; the first scheduled run is expected to go red until a separate re-pin lands.Reviewed by Cursor Bugbot for commit 51fdb2f. Bugbot is set up for automated code reviews on this repo. Configure here.