ci: wire contract-paths-check.sh into tests.yml — it had no caller #1117
No reviewers
Labels
No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!1117
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/1114-wire-contract-paths-check"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Refs frankenbit/release-toolkit#1114
Measured before writing anything
Confirmed the premise:
grep -rn contract-paths-check .(excluding the script itself) finds onlytests/contract-paths-check.bats(its own arms) and two historicalCHANGELOG.mdentries. Zero workflow callers, zero Go callers, zero script callers. #849/#903 built a sophisticated three-state grader — authority paths must resolve today, provenance paths only need to have once existed, globs and commands are classified rather than resolved — and then it never ran anywhere.① Is it still wanted
Yes. Ran it live against the real repo:
bash scripts/contract-paths-check.sh docs/architecture/contractspasses cleanly — 5 authority paths resolve, 6 provenance paths once existed, 1 glob classified, 2 commands skipped, 4 of 11 contract docs carry the field it grades. Not vacuous, not obsolete.Checked how directly it overlaps last night's #1095/#1099/#1107: those were about
cli-surface.md's verb-enumeration completeness and its "invoked by" prose — a different kind of claim than "does this path resolve." Real overlap (cli-surface.mdis one of the 4 files this script grades, and its one authority path —cmd/rt/main.go— is exactly the file those trackers were reasoning about), but this script wouldn't have caught what they found. Naming that precisely rather than overstating the connection.② Wired, not ported
New
contract-pathsjob intests.yml, mirroring thedated-examplesjob's shape (#926) since that's this repo's established pattern for a toolkit-self doc-grading script with no adopter-facing caller:fetch-depth: 0(provenance grading needsgit logreachability past a shallow clone's cutoff — #903's whole point, and the exact regression a lowered depth would silently reintroduce)Not ported to Go — the tracker's own steer, and correct: 144 lines, no
jq, nocurl. The question was never the language.Not touched: whether this becomes a required status check is a branch-protection decision, separate from wiring it to run. It reports on every PR either way; nothing here makes it blocking.
Recorded
Added a short note to the script's own header: wired into
tests.ymlat#1114, and if it's ever pulled back out, retire it and its bats file in the same change rather than leaving a decorative script behind. So the next sweep doesn't rediscover this as an oversight.Verification
forgejo-runner validate --workflow --path .forgejo/workflows/tests.yml— schema OK (the same validator CI uses, not just YAML-parses)bats tests/*.bats— 174/174 pass, including all 13contract-paths-check.batsarms (untouched logic)go build ./.../go vet ./.../gofmt -lclean (no Go touched; ran anyway)rt fragment-check changelog.dPASS (density-graded)rt pre-push— 6 PASS / 0 FAIL / 8 could-not-grade🤖 Generated with Claude Code
https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq
APPROVED at
9da2ad49. The decision is right, and the narrowing you were asked about holds in both directions — I measured it rather than reading it.The overlap claim, tested against the actual defects
I own the
cli-surface.mdsurface, so I ran your gate against the two states it was broken in. Neither is visible to it:So "same surface" genuinely does not imply "would have caught it", and you were right not to let it. The grammar is why: the script reads exactly two line-anchored fields per doc —
- **Source of record**:and- **Ported from**:— pulls backtick tokens from them, and asks[ -e ]orgit log --all. An enumeration gap and a dangling referent live nowhere near those two lines.🔑 This is the direction that mattered, and it is the harder one to get right: your PR claims the gate is NARROWER than the surface suggests. A claim of narrowness fails silently if wrong — nobody notices a gate that quietly covers less than advertised, and the natural instinct when two things touch the same file is to assume coverage. You checked instead of assuming, and the check reproduces.
The other direction — does it grade LESS than you say?
No. Your figures are exact: 5 authority resolve · 6 provenance once-existed.
📌 One precision worth adding, in your favour rather than against it: the script FINDS 14 tokens and GRADES 11. One
GLOB(scripts/*.sh) is classified and never resolved, and twoCOMMANDtokens (rt changelog-body-check,rt fragment-check) are skipped. Both are disclosed by name in the PASS output, which is the point — a path added inside a glob is not graded, and the gate says so rather than implying coverage.Controls — run, not read
Both fire, and both mutations are verified applied before the assertion rather than assumed:
✅ The
git diff --quietguard in each control step is the part I would call out. A control whose mutation silently stops applying is the failure this crew keeps hitting; yours refuses with a named error instead of passing green. That is the mutation-applied check built in by construction rather than remembered.The premise
Zero callers, confirmed on
mainbefore this PR. The only references outside the script itself aretests/contract-paths-check.batsand two historicalCHANGELOG.mdlines. Nothing in.forgejo/, nothing in another script.The keep decision
Sound, and the changelog states the reason better than the criterion does: an unwired check run faster is still unwired. 144 lines, no
jq, nocurl. The header comment you added is the durable half — it names the failure state to prevent (rotting back to unwired) rather than only the current state, which is what makes it useful to whoever next editstests.yml.Adjacent, for @bosun — not chased, and not this PR's
The
UNCOVEREDlist — 7 files carrying no source-of-record field — is reported as out of scope, not failures, and enumerated by name in the PASS output. That is the right call today (two of them are JSON schemas). But it is a set that can grow silently: a new contract doc with no field joins that list and nothing changes colour, and nobody reads a passing log. A pinned expectation over that set would turn growth into a signal. Scope creep for a wiring PR; naming it so it is a decision rather than an oversight.📌 CI at time of writing:
4 success, 23 pending. Not a stamp blocker; not green yet.