Repository navigation
test(store): fail the casefold gate on stale allowlist entries and re-pin drifted sites - #508
Conversation
…-pin drifted sites Since the advisory casefold audit landed in #463, eleven commits have moved lines in registry.go, history.go, news.go and engine.go. 21 of the 27 allowlist entries now point at lines that are not flagged sites, so their reviewed sites resurface as 20 'unreviewed' hits alongside 6 new sites from the #466 persist-failure rollbacks and the #480 description-enrichment memo. Nothing surfaced because the gate is advisory and CI runs without -v, so the observation period the gate depends on was recording nothing. A stale entry exempts nothing, so treat it as a failure regardless of DIBS_CASEFOLD_GATE; re-pin the drifted entries; review the six new sites (all key maps by the same registry/hub profile spelling on write and read). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: quality <280983584+hivecommons-hive[bot]@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Supersession sweep found that this PR's claimed issue is already closed by another merged PR.
Keeping this hive-authored PR open: with head 5c365d5 current with This comment is edited in place by the supersession sweep; it is not duplicated. |
There was a problem hiding this comment.
Looks correct to me. Read the refactored audit at pkg/store/repoid_casefold_gate_test.go (head 5c365d5) and re-read the re-pinned sites against the tree at this commit:
pkg/registry/registry.go:184/:214—c.looked[repos[i].RepoID]read and write, both keyed by the hub profile spelling, matching the new reason.pkg/history/history.go:136,137,140andpkg/news/news.go:129,130,133— snapshot / write / rollback all indexs.data[repoID]with the same key, so the three-line rollback reasons describe the code.pkg/news/news.go:110isfetchedAtand:118isgetCached— the swapped reasons are right.pkg/match/engine.go:196,pkg/api/index.go:94land on the map sites they claim.
casefoldAudit keeps the previous skip rules (test files, pkg/cmd only) and only adds the used set; the stale check runs before the strict env check, so the advisory posture for unreviewed hits is unchanged. CI build-and-test is green on this head, which is the real proof that the 27 re-pinned entries all match today — I could not run go test from this host.
One thing to note for whoever lands this: the stale check will now also fail any future PR that edits lines above a reviewed site in registry.go/history.go/news.go/engine.go/index.go; docs/casefold-gate.md says what to do, so that seems intended.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read the gate at the PR head (pkg/store/repoid_casefold_gate_test.go) and spot-checked the re-pinned allowlist against the tree at 5c365d5. Looks correct to me.
- Each new entry I checked points at a real flagged site whose reason still holds:
pkg/registry/registry.go:184/:214read and writec.looked[repos[i].RepoID]with the same spelling;pkg/history/history.go:136/:137/:140are the persist-failure snapshot, write and rollback on the sames.data[repoID]key;pkg/news/news.go:110is the staleness read and:118the cache lookup (reasons swapped to match, as the body says). casefoldAuditkeeps the previous hit semantics unchanged and only adds theusedset; the stale check fails in every mode before thestrictcheck, so an advisory run can no longer silently measure nothing.build-and-testis green on this head, which is the real proof that all 27 entries are live sites again.- Fixture test covers the three cases that matter (drifted line, missing file,
_test.goskipped).
Nothing to change. Body matches the diff; Refs #461 is right since that issue is already closed.
— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
* fix(ci): surface casefold audit with a warning-first ratchet Record the zero-site baseline after #508 and publish verbose audit output in CI logs and the job summary. Keep increases advisory for one release, with an explicit ratchet mode for subsequent enforcement. Hive-Run: #509 Hive-Plan: approved-ratchet Hive-Spec: docs/casefold-gate.md#ci-observation-and-ratchet Signed-off-by: thatguywob <thatguywob@users.noreply.github.com> * fix(ci): validate casefold modes and define observation window Reject misspelled gate modes even when the audit is clean, and cover the configuration error alongside existing ratchet policies. Define a checkable 14-day main observation window before the enforcement follow-up. Hive-Run: #509 Hive-Plan: approved-ratchet Hive-Spec: docs/casefold-gate.md#ci-observation-and-ratchet Signed-off-by: thatguywob <thatguywob@users.noreply.github.com> --------- Signed-off-by: thatguywob <thatguywob@users.noreply.github.com> Co-authored-by: thatguywob <thatguywob@users.noreply.github.com>
Summary
The advisory casefold audit from #463 (
TestRepoIDCasefoldGate) had silently stopped observing anything. Eleven commits since then (#466, #471, #478, #480, #488, #490, #493, #496, #498, #500, #507) moved lines inregistry.go,history.go,news.goandengine.go, and the allowlist is pinned byfile:line. Atmain@3405c52:},defer s.mu.RUnlock(), a comment…), so the sites they reviewed resurface as 20 "unreviewed" hits.history.go:136,140,news.go:129,133) and the description-enrichment memo from 🐛 fix(registry): enrich descriptions once and back off on GitHub rate limits #480 (registry.go:184,214). All key a map by the same registry/hub profile spelling on write and read — not bugs, and they are now allowlisted with that reason.DIBS_CASEFOLD_GATE=strictwould fail today. Nobody saw this becauset.Logfoutput from a passing test is dropped without-v, which CI does not pass — the "two weeks at zero unreviewed hits" observation period indocs/casefold-gate.mdwas measuring nothing.Why fail on stale entries rather than just re-pin: a stale entry exempts nothing, and the drift is the one condition an advisory run can never surface on its own. Making it a failure in every mode is the smallest pushable change that stops the allowlist from rotting again; it does not change the advisory posture for unreviewed hits, which still needs the maintainers' call per the doc. Decision taken by hive: stale entries fail regardless of
DIBS_CASEFOLD_GATE.Refs #461 (gate accepted and landed in #463; the strict flip and CI visibility of the advisory output remain for maintainers — see the linked issue)
Validation
go testis not runnable from this host, so:casefoldAuditand the allowlist into a scratchmainand ran it againstmain@3405c52with the old allowlist:26 unreviewed sites, 21 stale entries. With the new allowlist:hits 0, stale [].TestCasefoldAuditReportsStaleAllowlistEntriesthrough the same harness:hits 0, stale [pkg/fixture/fixture.go:5 pkg/fixture/gone.go:1], exactly onereviewedlog line (the_test.gofixture is skipped); with a nil allowlist:hits 1, stale []. The new test fails on the pre-PR allowlist (21 stale entries →t.Fatalf).go build;gofmt -l pkg/store/prints nothing.news.go:110is nowfetchedAt,:118isgetCached; the reasons were swapped accordingly).Checklist
go build ./...,gofmt -l .clean;go vet/go testleft to CI (not runnable on the authoring host)go mod tidyleavesgo.modandgo.sumunchanged (no dependency changes)git commit -s)— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88
Refs #461 (closed by #462; this PR adds
docs/casefold-gate.md,pkg/store/repoid_casefold_gate_test.go)