docs(readme): scope the ci badge to push events - #59
Merged
Conversation
The flags on the two curls that download the vendored spec were reasoned
about at length and enforced by nothing. A future edit could restore -L or
drop --remove-on-error and every gate would stay green; the comments
explaining them are documentation, not a control. So is the decision that
update-spec must never compute contract.SpecSHA256 itself — a recipe that
regenerated the digest would satisfy the content pin on every re-vendor and
assert nothing at all. Both are now checks.
scripts/fetch-flags.sh asserts that the Makefile's update-spec recipe and
drift.yaml's fetch step each contain exactly one spec-fetch curl, that it
still passes --proto '=https', --tlsv1.2, -fsS, --max-time and
--remove-on-error, and that it passes neither -L nor --location.
Getting at "the fetch" is the whole difficulty, and the first draft got it
wrong in three ways that all had the same root: it matched a joined line of
text rather than a command. Review found working bypasses for each, and
they are now fixtures. Hoisting the URL into a Make variable moved it off
the recipe line, so the gate anchored on a commented-out canonical fetch
left for reference and blessed a live `curl -L -k`. Demoting the flags to a
trailing comment satisfied every substring check. And a compliant probe
fetch sharing one logical line with an unsafe one vouched for it, because
grep -c counted lines. The extractor now drops comments before joining,
joins continuations, and splits on ; and && so each command stands alone.
The same narrowing removed two false positives, also fixtures now: the
whole update-spec recipe is one continued line, so a `cp -L` three commands
later read as the curl passing -L, and a --max-time anywhere in the recipe
satisfied the check for the curl.
The digest check reads only the recipe's own command lines. A sed range
ending at the next line starting with a letter swept in the following
comment block — so a comment DOCUMENTING this very rule ("never regenerate
SpecSHA256 with sha256sum") turned local-ci red and accused the maintainer
of what it says not to do.
awk rather than sed for both: joining and splitting want a newline in the
output, and \n in a sed replacement is a GNU extension BSD sed rejects.
That trap already cost this stack one commit; CI is Linux-only and would
not have caught it either time.
The gate deliberately does not police every curl in the repo.
go-latest-check and check-version.sh both pass -L legitimately — they parse
a string into a variable rather than vendoring bytes to disk, so a redirect
costs them nothing. A repo-wide ban would either break them or teach the
next maintainer that this gate is noise to route around, and a gate people
route around is worse than no gate. A compliant pair with a legitimate -L
beside it is a passing fixture.
It also fails if it can no longer FIND the fetch it guards, in either
direction: zero matches or two. A grep gate's characteristic failure is
passing on everything, and "the file was restructured and my pattern
stopped matching" is how that happens quietly.
fetch-flags-selftest drives 27 hand-written fixtures, no network. Fixtures
are hand-written rather than derived from the real files, because the
gate's whole job is to notice those files changing. An earlier review
injected 11 independent mutations into the gate — an emptied flag loop,
inverted case arms, fail=1 removed from bad(), the -L regex neutered, the
final exit turned into 0 — and all 11 went red.
Both scripts are shellcheck-clean. Wired into local-ci and ci.yaml beside
the other selftests. Tooling only: no Go file, no module byte, no public
surface, and no CHANGELOG line — CONTRIBUTING scopes that to user-visible
changes.
GitHub's native badge reports the latest run on the default branch regardless of trigger. ci.yaml runs on pull_request as well as push: [main, v1], so a PR whose HEAD branch is main — a main-to-v1 backport, say — would be attributed to main and could colour the README while it is still in flight. Measured at zero occurrences: across all 84 ci.yaml runs, every run attributed to main is a push run, because PR runs carry their head branch. And the shields.io URL this replaced carried the identical exposure via ?branch=main, which also does not filter by event. So this is hardening, not a regression being fixed — the window has simply never opened. Only the ci badge. The same parameter on spec-drift returns "no status", measured: that workflow triggers on schedule and workflow_dispatch and has no push events at all, so filtering to them leaves nothing to report. The asymmetry is not an oversight.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub's native badge reports the latest run on the default branch
regardless of trigger. ci.yaml runs on pull_request as well as
push: [main, v1], so a PR whose HEAD branch is main — a main-to-v1 backport,
say — would be attributed to main and could colour the README while it is
still in flight.
Measured at zero occurrences: across all 84 ci.yaml runs, every run
attributed to main is a push run, because PR runs carry their head branch.
And the shields.io URL this replaced carried the identical exposure via
?branch=main, which also does not filter by event. So this is hardening,
not a regression being fixed — the window has simply never opened.
Only the ci badge. The same parameter on spec-drift returns "no status",
measured: that workflow triggers on schedule and workflow_dispatch and has
no push events at all, so filtering to them leaves nothing to report. The
asymmetry is not an oversight.
Stacked on #58 (
ci/pin-fetch-flags); contains its commit until it lands. The README hunk is the single line above.