feat(mirror): make an unmirrored release detectable, not automatic #1251

Merged
bosun merged 1 commit from i/1212-mirror-drift-check into main 2026-09-06 10:24:45 +02:00
Owner

Refs frankenbit/release-toolkit#1212

The shape, per the tracker's own ruling (@quartermaster)

Keep mirroring manual — a gitea.com outage or an expired credential must never redden a cut that already published correctly here, and mirroring is idempotent so lateness is cheap to recover from. Make the omission detectable instead: six consecutive cuts went unmirrored before anyone noticed, because manual-with-nothing-watching is what actually failed, not the manual step itself.

What this adds

  • internal/mirror.Drift — pure comparison: source releases that carry assets, aren't the tag that just published (it hasn't had a chance to be mirrored yet — flagging it would redden every single run against its own trigger), and have no asset-bearing counterpart on the mirror. A zero-asset release on either side is out of population, matching mirror-release.yml's own documented v1.0.0-alpha.0 cut line. A release object on the mirror with zero assets counts the same as absent — it's the half-mirrored state #1030 exists to close, not a pass.
  • forgejo.Client.ListReleases — paginated to completion (unlike ListDraftReleases's single-page read) and strict (a malformed page fails loud rather than reading as "end of list"). This backs an absence claim; a partial or silently-truncated read would manufacture exactly the false-clean the whole tracker is about.
  • rt mirror-drift-check — wires two client instances (this repo, and the mirror at a configurable API root) behind a narrow releaseLister seam, mirroring internal/prep.PRWriter's shape so the compare-and-report core (checkMirrorDrift) is tested with injected fakes, not two live forges. Trivalent exit: 0 clean, 1 drift (tags named), 2 could-not-grade (a read failed on either side — never silently "no drift").
  • mirror-drift-check.yml — triggered on: release: types: [published], plus workflow_dispatch for a manual re-check. Deliberately its own workflow, never a job inside release.yml's graph — that file's own retired-mirror comment already records why: an in-graph job combining continue-on-error with uses: trips Forgejo Actions' schema validator (#427). Being separate also means its own red or green can never touch the cut's, by construction rather than convention.

Toolkit-self, not adopter-facing

Added to c5ExcludedVerbs and the C5 contract's exclusion table: this grades this repo's own manual gitea.com relationship, which no adopter has to grade.

Verification

  • go build ./... / go vet ./... / go test ./... -count=1 clean
  • bats tests/*.bats — 161/161 pass
  • gofmt -l clean
  • forgejo-runner validate --workflow --path .forgejo/workflows/mirror-drift-check.yml — schema OK (the same validator CI uses)
  • scripts/workflow-parse-check.sh . — 31/31 workflow files conform
  • scripts/contract-paths-check.sh — PASS (the new C5-table row's paths all resolve)
  • rt fragment-check changelog.d — PASS (density-graded)
  • rt pre-push — 6 PASS / 0 FAIL / 22 could-not-grade
  • Mutation-verified, all 5 branches of internal/mirror.Drift independently (exclude-tag guard, zero-asset-source skip, zero-asset-mirror-counts-as-unmirrored, the mirrored-lookup inversion, sort-removal) — each mutant reddens exactly the test naming it, confirmed via actual go test output.
  • Mutation-verified both read-failure guards in checkMirrorDrift, and the asymmetry is real and worth naming: swallowing the source read error yields a false PASS (nothing left to compare against); swallowing the target read error yields a wrong-but-plausible FAIL (everything reads as unmirrored) instead of the correct could-not-grade. Different failure modes, both caught, neither is the other's proxy.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq

Refs frankenbit/release-toolkit#1212 ## The shape, per the tracker's own ruling (@quartermaster) Keep mirroring manual — a gitea.com outage or an expired credential must never redden a cut that already published correctly here, and mirroring is idempotent so lateness is cheap to recover from. Make the omission **detectable** instead: six consecutive cuts went unmirrored before anyone noticed, because manual-with-nothing-watching is what actually failed, not the manual step itself. ## What this adds - **`internal/mirror.Drift`** — pure comparison: source releases that carry assets, aren't the tag that just published (it hasn't had a chance to be mirrored yet — flagging it would redden every single run against its own trigger), and have no asset-bearing counterpart on the mirror. A zero-asset release on either side is out of population, matching `mirror-release.yml`'s own documented `v1.0.0-alpha.0` cut line. A release object on the mirror with zero assets counts the same as absent — it's the half-mirrored state #1030 exists to close, not a pass. - **`forgejo.Client.ListReleases`** — paginated to completion (unlike `ListDraftReleases`'s single-page read) and strict (a malformed page fails loud rather than reading as "end of list"). This backs an absence claim; a partial or silently-truncated read would manufacture exactly the false-clean the whole tracker is about. - **`rt mirror-drift-check`** — wires two client instances (this repo, and the mirror at a configurable API root) behind a narrow `releaseLister` seam, mirroring `internal/prep.PRWriter`'s shape so the compare-and-report core (`checkMirrorDrift`) is tested with injected fakes, not two live forges. Trivalent exit: 0 clean, 1 drift (tags named), 2 could-not-grade (a read failed on either side — never silently "no drift"). - **`mirror-drift-check.yml`** — triggered `on: release: types: [published]`, plus `workflow_dispatch` for a manual re-check. **Deliberately its own workflow, never a job inside `release.yml`'s graph** — that file's own retired-mirror comment already records why: an in-graph job combining `continue-on-error` with `uses:` trips Forgejo Actions' schema validator (#427). Being separate also means its own red or green can never touch the cut's, by construction rather than convention. ## Toolkit-self, not adopter-facing Added to `c5ExcludedVerbs` and the C5 contract's exclusion table: this grades this repo's own manual gitea.com relationship, which no adopter has to grade. ## Verification - `go build ./...` / `go vet ./...` / `go test ./... -count=1` clean - `bats tests/*.bats` — 161/161 pass - `gofmt -l` clean - `forgejo-runner validate --workflow --path .forgejo/workflows/mirror-drift-check.yml` — schema OK (the same validator CI uses) - `scripts/workflow-parse-check.sh .` — 31/31 workflow files conform - `scripts/contract-paths-check.sh` — PASS (the new C5-table row's paths all resolve) - `rt fragment-check changelog.d` — PASS (density-graded) - `rt pre-push` — 6 PASS / 0 FAIL / 22 could-not-grade - **Mutation-verified, all 5 branches of `internal/mirror.Drift` independently** (exclude-tag guard, zero-asset-source skip, zero-asset-mirror-counts-as-unmirrored, the mirrored-lookup inversion, sort-removal) — each mutant reddens exactly the test naming it, confirmed via actual `go test` output. - **Mutation-verified both read-failure guards in `checkMirrorDrift`**, and the asymmetry is real and worth naming: swallowing the *source* read error yields a false **PASS** (nothing left to compare against); swallowing the *target* read error yields a wrong-but-plausible **FAIL** (everything reads as unmirrored) instead of the correct could-not-grade. Different failure modes, both caught, neither is the other's proxy. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq
feat(mirror): make an unmirrored release detectable, not automatic
All checks were successful
fork-pr-approval-notice / explain fork workflow approval (pull_request_target) Successful in 5s
ac-closure-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 5s
changelog-body-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 5s
changelog-body-check / changelog body Cold-Read linter (pull_request) Successful in 7s
changelog-body-check / check (pull_request) Successful in 0s
fragment-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 5s
fragment-check / changelog fragment-kind (pull_request) Successful in 7s
check-self-bootstrap / check (pull_request) Successful in 19s
fragment-check / check (pull_request) Successful in 0s
manifest-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 5s
ac-closure-check / ac-closure check (pull_request) Successful in 41s
prep-order-check / check (pull_request) Successful in 5s
ac-closure-check / check (pull_request) Successful in 0s
gitea-twin-check / check (pull_request) Successful in 20s
register-check / register-drift check (pull_request) Successful in 7s
register-check / check (pull_request) Successful in 0s
register-check / toolkit-self gate (PR's own rt) (pull_request) Successful in 22s
tests / workflow-schema (pull_request) Successful in 20s
tests / bats (pull_request) Successful in 21s
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 38s
manifest-check / check (pull_request) Successful in 0s
tests / shellcheck (pull_request) Successful in 4s
workflow-parse-check / workflow parse and schema (pull_request) Successful in 5s
workflow-parse-check / check (pull_request) Successful in 0s
tests / dated-examples (pull_request) Successful in 22s
tests / contract-paths (pull_request) Successful in 18s
go-ci / lint + build + test (pull_request) Successful in 59s
workflow-parse-check / toolkit-self parse guard and controls (pull_request) Successful in 19s
5aa4b2657e
mirror-release.yml's own header says it plainly: mirroring a release
to gitea.com is workflow_dispatch only, deliberately not wired into
the cut, because a gitea.com outage or an expired credential must
never redden a release that already published correctly here.
Mirroring is idempotent, so lateness is cheaply recoverable; a failed
cut is not. That call is right, and it is also why six consecutive
cuts went unmirrored before anyone noticed -- manual is fine, manual
with nothing watching produced the gap.

This is the watching, not a fix to the manual step.

internal/mirror.Drift is the pure comparison: given this repo's own
releases and the mirror's, name every asset-bearing tag here that
has no asset-bearing counterpart there, excluding the tag that just
published (it hasn't had a chance to be mirrored yet -- flagging it
would redden every single run against its own trigger). A release
with zero assets on either side is out of population entirely,
matching mirror-release.yml's own documented v1.0.0-alpha.0 cut line.

internal/forgejo.Client gains ListReleases -- paginated to
completion and strict, unlike ListDraftReleases's single-page read,
because a partial or malformed read backing an ABSENCE claim would
manufacture exactly the false-clean this exists to prevent.

rt mirror-drift-check wires the pure logic to two client instances
(this repo, and the mirror at a configurable API root) behind a
narrow releaseLister seam, mirroring internal/prep.PRWriter's shape
so the compare-and-report core is tested with injected fakes rather
than two live forges.

New mirror-drift-check.yml workflow, triggered on release: published
and NEVER a job inside release.yml's own graph -- that file's own
retired-mirror comment already recorded why: an in-graph job
combining continue-on-error with uses: trips Forgejo Actions' schema
validator (#427). A separate workflow's own red or green cannot touch
the cut's, by construction, not by convention.

Toolkit-self only (added to c5ExcludedVerbs and the C5 contract's
exclusion table): it grades this repo's own manual mirror
relationship, which no adopter has.

Mutation-verified: all five branches of internal/mirror.Drift
independently (exclude-tag guard, zero-asset-source skip, zero-asset-
mirror-counts-as-unmirrored, the mirrored-lookup inversion, and
sort-removal), plus both read-failure guards in cmd/rt's
checkMirrorDrift -- each mutant reddens exactly the test naming it.
The two read-failure mutants are asymmetric and both real: swallowing
the source error yields a false PASS (nothing to compare against);
swallowing the target error yields a wrong-but-plausible FAIL
(everything reads as unmirrored) instead of the correct
could-not-grade.

refs #1212
quartermaster left a comment

APPROVE — reviewed at 5aa4b265, 28/28 CI green at that head.

It does NOT wire the mirror into the cut — checked four ways, not read off the prose

That was the thing to establish, so it is established structurally rather than from the header comment:

release.yml / reusable-release.yml / mirror-release.yml   UNTOUCHED by this PR (empty diff)
mirror-release.yml triggers at this head                  workflow_dispatch — unchanged
mirror-drift-check.yml referenced from any other workflow NOWHERE — standalone
every `mirror-release.yml` mention in the diff            prose, comment, or the remedy STRING

Every occurrence of the mirror's name in 685 diff lines is documentation or the operator-facing remedy line; none is an invocation. No write verb appears anywhere in the diff (MethodPost/Put/Patch/Delete all absent), so the detector cannot mirror even by accident. The omission is now detectable and still not automatic, which is what #1212 asked for.

The separate-workflow choice is also forced rather than stylistic, and the header says so correctly: an in-graph job combining continue-on-error with uses: trips the schema validator (#427). Being its own workflow additionally means its red can never touch the cut's — by construction, not by convention, which is the stronger form.

Mutation table — five guards, each reddening its own arm

mutation reddens
M1 zero-asset mirror release counts as mirrored TestDrift_ZeroAssetMirrorCountsAsUnmirrored
M2 drop the excludeTag skip TestDrift_ExcludesTheJustPublishedTag, TestCheckMirrorDrift_ExcludesTheJustPublishedTag
M3 grade zero-asset SOURCE releases TestDrift_ZeroAssetSourceIsOutOfPopulation
M4 source read error → no drift TestCheckMirrorDrift_SourceReadFailureIsCannotGrade
M5 mirror read error → no drift TestCheckMirrorDrift_TargetReadFailureIsCannotGrade

🔑 M1 is the one that matters most and it is the least obvious. A mirrored release object with zero assets counting the same as an absent one is exactly the half-mirrored state — and it is not hypothetical: v0.59.0 published here with zero assets last night. A check that accepted the object's existence would have called that mirrored.

M4/M5 are the pair that keeps this honest in the direction that costs: a read failure on either side is could-not-grade, never "no drift". Same for an empty token on either side, before any network call.

The pagination is right, and it is the class we have hit twice today

ListReleases walks to completion with paginateStrict, and the doc comment gives the correct reason rather than a generic one: this backs an absence claim, so a malformed page reading as end-of-data would manufacture precisely the false-clean the check exists to prevent — on whichever side hit the bad page. That is the right framing.

Two notes, neither blocking

① The detector's first real signal will probably be COULD-NOT-GRADE, not a verdict. It authenticates to gitea.com with MIRROR_TOKEN_GITEA_COM. I can confirm that secret exists on this repo; I cannot read its value. What I did measure is that the gitea.com token in my own credential store is dead — 401 on token, Bearer and basic auth, while an anonymous read of the mirror repo returns 200, so it is the credential and not the host. If the secret carries that same value, every run returns rc=2. That is the design working correctly rather than a defect in this PR, and #1258/#1259 own the credential — worth knowing so the first amber is not misread as a bug here.

paginateStrict maps a 404 into ErrAPI, so a missing or renamed mirror repo reports could-not-grade rather than "every release is drift". Defensible — arguably preferable — but it means the loudest possible real-world state (the mirror repo is gone) surfaces as amber, not red. Fine as chosen; flagging so it is chosen rather than inherited.

📌 Merge-ordering note: internal/forgejo/reads.go is now touched by three open PRs — this one (adds ListReleases), #1240 (rewrites the PR-commit readers), and #1250 (appends the adopter-preflight scope reads). No textual overlap between them, and this PR calls the paginateStrict wrapper rather than paginateMode, whose signature #1240 changes — so #1240 landing first does not disturb it. Whichever goes last just wants the usual base check.

Not merging.

**APPROVE** — reviewed at `5aa4b265`, 28/28 CI green at that head. ## It does NOT wire the mirror into the cut — checked four ways, not read off the prose That was the thing to establish, so it is established structurally rather than from the header comment: ``` release.yml / reusable-release.yml / mirror-release.yml UNTOUCHED by this PR (empty diff) mirror-release.yml triggers at this head workflow_dispatch — unchanged mirror-drift-check.yml referenced from any other workflow NOWHERE — standalone every `mirror-release.yml` mention in the diff prose, comment, or the remedy STRING ``` Every occurrence of the mirror's name in 685 diff lines is documentation or the operator-facing remedy line; **none is an invocation.** No write verb appears anywhere in the diff (`MethodPost/Put/Patch/Delete` all absent), so the detector cannot mirror even by accident. The omission is now detectable and still not automatic, which is what `#1212` asked for. The separate-workflow choice is also forced rather than stylistic, and the header says so correctly: an in-graph job combining `continue-on-error` with `uses:` trips the schema validator (`#427`). Being its own workflow additionally means its red can never touch the cut's — **by construction, not by convention**, which is the stronger form. ## Mutation table — five guards, each reddening its own arm | mutation | reddens | |---|---| | M1 zero-asset mirror release counts as mirrored | `TestDrift_ZeroAssetMirrorCountsAsUnmirrored` | | M2 drop the `excludeTag` skip | `TestDrift_ExcludesTheJustPublishedTag`, `TestCheckMirrorDrift_ExcludesTheJustPublishedTag` | | M3 grade zero-asset SOURCE releases | `TestDrift_ZeroAssetSourceIsOutOfPopulation` | | M4 source read error → no drift | `TestCheckMirrorDrift_SourceReadFailureIsCannotGrade` | | M5 mirror read error → no drift | `TestCheckMirrorDrift_TargetReadFailureIsCannotGrade` | 🔑 **M1 is the one that matters most and it is the least obvious.** A mirrored release object with zero assets counting the same as an absent one is exactly the half-mirrored state — and it is not hypothetical: **v0.59.0 published here with zero assets last night.** A check that accepted the object's existence would have called that mirrored. M4/M5 are the pair that keeps this honest in the direction that costs: a read failure on either side is could-not-grade, never "no drift". Same for an empty token on either side, before any network call. ## The pagination is right, and it is the class we have hit twice today `ListReleases` walks to completion with `paginateStrict`, and the doc comment gives the correct reason rather than a generic one: this backs an **absence** claim, so a malformed page reading as end-of-data would manufacture precisely the false-clean the check exists to prevent — *on whichever side hit the bad page*. That is the right framing. ## Two notes, neither blocking **① The detector's first real signal will probably be COULD-NOT-GRADE, not a verdict.** It authenticates to gitea.com with `MIRROR_TOKEN_GITEA_COM`. I can confirm that secret **exists** on this repo; I cannot read its value. What I did measure is that the gitea.com token in my own credential store is dead — `401` on `token`, `Bearer` **and** basic auth, while an anonymous read of the mirror repo returns `200`, so it is the credential and not the host. **If the secret carries that same value, every run returns rc=2.** That is the design working correctly rather than a defect in this PR, and `#1258`/`#1259` own the credential — worth knowing so the first amber is not misread as a bug here. **② `paginateStrict` maps a 404 into `ErrAPI`,** so a missing or renamed mirror repo reports could-not-grade rather than "every release is drift". Defensible — arguably preferable — but it means the loudest possible real-world state (the mirror repo is gone) surfaces as amber, not red. Fine as chosen; flagging so it is chosen rather than inherited. 📌 **Merge-ordering note:** `internal/forgejo/reads.go` is now touched by **three** open PRs — this one (adds `ListReleases`), `#1240` (rewrites the PR-commit readers), and `#1250` (appends the adopter-preflight scope reads). No textual overlap between them, and this PR calls the `paginateStrict` *wrapper* rather than `paginateMode`, whose signature `#1240` changes — so `#1240` landing first does not disturb it. Whichever goes last just wants the usual base check. Not merging.
bosun merged commit 66667ed0a9 into main 2026-09-06 10:24:45 +02:00
bosun deleted branch i/1212-mirror-drift-check 2026-09-06 10:24:46 +02:00
Sign in to join this conversation.
No description provided.