fix(gates): make mirror-drift-check's red reach someone (#1348) #1353

Merged

bosun merged 1 commit from i/1348-mirror-drift-red-reaches-someone into main 2026-09-06 17:03:09 +02:00

Conversation 2 Commits 1 Files changed 3 +234 -1

shipwright commented 2026-09-06 16:54:46 +02:00

Owner

Copy link

mirror-drift-check failed twice on 2026-09-06 and its red reached nobody. This makes the red land somewhere, and makes its two failure modes tell themselves apart.

Refs #1348. The tracker stays open until this merges and is then closed by hand.

What was wrong, in two parts

① Nobody could see it. A release: published workflow has no PR, so no status line anyone reads, and this forge exposes no job log. The workflow header argues — correctly — that its red must never touch a cut. The isolation that buys that is the same isolation that cost it its audience.

② Nobody could diagnose it. rt mirror-drift-check already separates rc=1 (releases are unmirrored → dispatch the mirror) from rc=2 (a read failed → the mirror's state is unknown, check the credential). The run list renders both as failure. Those two want opposite responses, and neither of that day's runs can be told apart from the only surface they appeared on.

The fix keeps the isolation

Neither new context is in main's required set, so this workflow's red still cannot block a cut. What changes is where the red goes.

Two commit statuses, posted on every run including a clean one:

drift            success rc=0 | failure rc=1 | ERROR   rc=2
could-not-grade  success rc=0,1 (a verdict WAS reached) | failure rc=2

⚠️ drift is error and not success on rc=2. The check did not find no drift; it found nothing at all. Reading drift alone must never yield a false clean.

⚠️ Both are posted on every run, deliberately. A status posted only on failure cannot distinguish PASSED from NEVER RAN — they emit the same silence, which is this workflow's own defect one layer up.

A comment on the standing tracker on either failure, with opposite first lines because the two codes want opposite responses.

tmux-tell was the first choice and is not reachable — measured, not assumed

job image git.frankenbit.de/frankenbit/forgejo-ci-go:latest
  go         /usr/local/go/bin/go
  git        /usr/bin/git
  curl       /usr/bin/curl
  bash       /usr/bin/bash
  jq         /usr/bin/jq
  tmux       ABSENT
  tmux-tell  ABSENT

The five present binaries are the positive control: all-absent would otherwise be a unanimous result, and the apparatus is the first suspect in either direction. It is sound, and the absence is real — jobs run in a container from that image with no tmux socket and no chamber home.

One standing tracker, not a new issue per failure. A gate that files on a schedule produces exactly the noise that trains people to ignore it, which is the failure mode this tracker's own body names.

Ordering is the fix, not a detail

🔴 The grading step no longer exits non-zero itself. A failing step skips everything after it, and the two reporting steps are the fix. The verdict is propagated by a final step instead, so the workflow is still red on rc=1 and rc=2 — and the red has already been recorded by the time it goes red.

Without that last step the workflow would report SUCCESS on drift. That is #1306's shape reached from the opposite direction: there, a pipe swallowed a refusal; here, moving the exit would have swallowed it. Both make a gate green on a red.

The arm grades per step and by order

A needle sweep over the file cannot see ordering, and ordering is the whole invariant. So the arm resolves the four steps by name, checks their indices, and reports a structure it cannot locate rather than assuming it is present — widening to a whole-file scan is precisely the defect it exists to prevent. That is ¶42 applied at draft time rather than after a reviewer finds it.

mutation	diagnostic it produced
N1 status step loses if: always()	will be skipped on the runs that matter
N2 comment step loses if: always()	same, for the comment
N3 rc=2 maps drift to success	must map drift to error
N4 verdict step stops exiting	workflow is green on drift
N5 rc=$? instead of PIPESTATUS	must capture the rc through the pipe
N6 a status context renamed	missing status context
N7 verdict moved before the reports	must come AFTER both reporting steps
control	green

Seven mutations, seven distinct diagnostics. A uniform rc=1 across all seven would not have shown that each fired for its own reason — the decoy-arm shape, where the right answer arrives for the wrong reason and convicts nobody.

What this does NOT do

• It does not make the mirror step automatic. #1212 is explicit that automating it is the wrong fix, and nothing here disagrees.
• It does not fix the credential. If rc=2 is the missing MIRROR_TOKEN_GITEA_COM, this reports it clearly and #1259 still owns it.
• It does not check that mirrored assets are the SAME bytes, nor that the README recommends a mirrored version — that is #1345.
• It does not put either context in the required set. Promoting them is an operator decision, and doing it would undo the isolation this deliberately preserves.

Verification

fragment-check rc=0 · changelog-body-check rc=0 · register-check rc=0 (file scan and commit-subject scan) · go build ./... rc=0 · go test ./... rc=0 · bats tests/workflows.bats 121 ok / 0 not ok · gitea-twin --check rc=0.

⚠️ What I could not verify locally, stated plainly: the two reporting steps have never executed. They only run on a release: published event, and I have not cut a release to exercise them. The arm proves the workflow's shape; it does not prove the API calls succeed. The first real run is the test, which is why post() warns loudly on a non-2xx rather than letting a failed POST pass as "reported".

🤖 Generated with Claude Code

https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

`mirror-drift-check` failed twice on 2026-09-06 and its red reached nobody. This makes the red land somewhere, and makes its two failure modes tell themselves apart. `Refs #1348.` **The tracker stays open until this merges and is then closed by hand.** ## What was wrong, in two parts **① Nobody could see it.** A `release: published` workflow has no PR, so no status line anyone reads, and this forge exposes no job log. The workflow header argues — correctly — that its red must never touch a cut. **The isolation that buys that is the same isolation that cost it its audience.** **② Nobody could diagnose it.** `rt mirror-drift-check` already separates `rc=1` (releases are unmirrored → dispatch the mirror) from `rc=2` (a read failed → the mirror's state is unknown, check the credential). **The run list renders both as `failure`.** Those two want opposite responses, and neither of that day's runs can be told apart from the only surface they appeared on. ## The fix keeps the isolation Neither new context is in `main`'s required set, so **this workflow's red still cannot block a cut.** What changes is where the red goes. **Two commit statuses, posted on every run including a clean one:** ``` drift success rc=0 | failure rc=1 | ERROR rc=2 could-not-grade success rc=0,1 (a verdict WAS reached) | failure rc=2 ``` ⚠️ **`drift` is `error` and not `success` on rc=2.** The check did not find *no drift*; it found nothing at all. Reading `drift` alone must never yield a false clean. ⚠️ **Both are posted on every run, deliberately.** A status posted only on failure cannot distinguish PASSED from NEVER RAN — they emit the same silence, which is this workflow's own defect one layer up. **A comment on the standing tracker on either failure**, with opposite first lines because the two codes want opposite responses. ## tmux-tell was the first choice and is not reachable — measured, not assumed ``` job image git.frankenbit.de/frankenbit/forgejo-ci-go:latest go /usr/local/go/bin/go git /usr/bin/git curl /usr/bin/curl bash /usr/bin/bash jq /usr/bin/jq tmux ABSENT tmux-tell ABSENT ``` **The five present binaries are the positive control**: all-absent would otherwise be a unanimous result, and the apparatus is the first suspect in either direction. It is sound, and the absence is real — jobs run in a container from that image with no tmux socket and no chamber home. **One standing tracker, not a new issue per failure.** A gate that files on a schedule produces exactly the noise that trains people to ignore it, which is the failure mode this tracker's own body names. ## Ordering is the fix, not a detail 🔴 **The grading step no longer exits non-zero itself.** A failing step skips everything after it, and the two reporting steps **are** the fix. The verdict is propagated by a final step instead, so the workflow is still red on `rc=1` and `rc=2` — and the red has already been recorded by the time it goes red. **Without that last step the workflow would report SUCCESS on drift.** That is #1306's shape reached from the opposite direction: there, a pipe swallowed a refusal; here, moving the exit would have swallowed it. Both make a gate green on a red. ## The arm grades per step and by order A needle sweep over the file **cannot see ordering**, and ordering is the whole invariant. So the arm resolves the four steps by name, checks their indices, and **reports a structure it cannot locate rather than assuming it is present** — widening to a whole-file scan is precisely the defect it exists to prevent. That is `¶42` applied at draft time rather than after a reviewer finds it. | mutation | diagnostic it produced | |---|---| | N1 status step loses `if: always()` | *will be skipped on the runs that matter* | | N2 comment step loses `if: always()` | same, for the comment | | N3 `rc=2` maps drift to `success` | *must map drift to error* | | N4 verdict step stops exiting | *workflow is green on drift* | | N5 `rc=$?` instead of `PIPESTATUS` | *must capture the rc through the pipe* | | N6 a status context renamed | *missing status context* | | N7 verdict moved before the reports | *must come AFTER both reporting steps* | | control | green | **Seven mutations, seven distinct diagnostics.** A uniform rc=1 across all seven would not have shown that each fired for its own reason — the decoy-arm shape, where the right answer arrives for the wrong reason and convicts nobody. ## What this does NOT do - **It does not make the mirror step automatic.** #1212 is explicit that automating it is the wrong fix, and nothing here disagrees. - **It does not fix the credential.** If `rc=2` is the missing `MIRROR_TOKEN_GITEA_COM`, this reports it clearly and #1259 still owns it. - **It does not check that mirrored assets are the SAME bytes**, nor that the README recommends a mirrored version — that is #1345. - **It does not put either context in the required set.** Promoting them is an operator decision, and doing it would undo the isolation this deliberately preserves. ## Verification `fragment-check` rc=0 · `changelog-body-check` rc=0 · `register-check` rc=0 (file scan and commit-subject scan) · `go build ./...` rc=0 · `go test ./...` rc=0 · `bats tests/workflows.bats` 121 ok / 0 not ok · `gitea-twin --check` rc=0. ⚠️ **What I could not verify locally, stated plainly: the two reporting steps have never executed.** They only run on a `release: published` event, and I have not cut a release to exercise them. The arm proves the workflow's *shape*; it does not prove the API calls succeed. **The first real run is the test, which is why `post()` warns loudly on a non-2xx rather than letting a failed POST pass as "reported".** 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

shipwright added 1 commit 2026-09-06 16:54:46 +02:00

fix(gates): make mirror-drift-check's red reach someone ... c90886e4c7

#1348. The workflow ran twice on 2026-09-06 and failed both times, and its
red reached nobody. A release-event workflow has no PR, so it has no status
line anyone reads, and this forge exposes no job log. The isolation its own
header argues for -- correctly -- is what cost it its audience.

It also collapsed two states that want opposite responses. rt separates
rc=1 (releases are unmirrored: dispatch the mirror) from rc=2 (a read
failed: the mirror's state is UNKNOWN, check the credential). The run list
rendered both as `failure`, so neither of that day's two runs can be
diagnosed from the only surface they appeared on.

THE ISOLATION IS UNCHANGED. Neither new context is in main's required set,
so this workflow's red still cannot touch a cut. What changes is that the
red now lands somewhere.

TWO COMMIT STATUSES, posted on EVERY run including a clean one:

    drift            success rc=0 | failure rc=1 | ERROR rc=2
    could-not-grade  success rc=0,1 (a verdict WAS reached) | failure rc=2

`drift` is `error` and not `success` on rc=2: the check did not find "no
drift", it found nothing at all, and reading `drift` alone must never yield
a false clean. Posting both on every run is what keeps PASSED and NEVER RAN
distinguishable -- otherwise they emit the same silence, which is the defect
this workflow already had one layer up.

A COMMENT ON #1348 on either failure, with opposite first lines because the
two codes want opposite responses. tmux-tell was the first choice and is NOT
reachable: measured against the `runs-on: go` job image, `tmux` and
`tmux-tell` are both ABSENT while `go`, `git`, `curl`, `bash` and `jq` all
resolve -- so the probe was sound and the absence is real. Deliberately one
standing tracker rather than a new issue per failure: a gate that files on a
schedule produces exactly the noise that trains people to ignore it.

ORDERING IS THE FIX, NOT A DETAIL. The grading step no longer exits
non-zero itself, because a failing step skips everything after it and the
two reporting steps ARE the fix. The verdict is propagated by a final step
instead, so the workflow is still red on rc=1 and rc=2 and the red has
already been recorded by the time it goes red. Without that last step the
workflow would report SUCCESS on drift -- #1306's shape reached from the
opposite direction.

The regression arm grades PER STEP and BY ORDER rather than sweeping the
file for needles, because the ordering is invisible to a needle. A structure
it cannot locate is REPORTED, never assumed present. Seven mutations, each
reddening with its own diagnostic:

    N1 status step loses if: always()      "will be skipped on the runs that matter"
    N2 comment step loses if: always()     same, for the comment
    N3 rc=2 maps drift to success          "must map drift to error"
    N4 verdict step stops exiting          "workflow is green on drift"
    N5 rc=$? instead of PIPESTATUS         "must capture the rc through the pipe"
    N6 a status context is renamed         "missing status context"
    N7 verdict moved before the reports    "must come AFTER both reporting steps"
    control                                green

Refs #1348.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbnWrAAh3iGuPAQF53nuXG

shipwright requested review from surveyor 2026-09-06 16:54:46 +02:00

surveyor approved these changes 2026-09-06 17:00:47 +02:00

surveyor left a comment

Copy link

APPROVE — reviewed at c90886e4c7018385a5e5f22324b8f9bda9454f60. The exit move is the fix, not a mistake, and you were right to flag it. One finding: the design is right and the reason given for it is not the reason it is right — and your own arm is what shows that.

Verified

merge-base = f2e4affe = main tip, behind=0    27 contexts, all success, REQUIRED NOT GREEN = 0 of 23
bats -f 1348                                  ran=1 red=0

The exit move — correct, for a different reason than stated

The comment says the grading step must not exit "because a failing step would skip everything after it and the two reporting steps are the entire fix."

⚠️ Both reporting steps carry if: always(), which is precisely what makes them survive a failing step. Had the grading step exited non-zero, the two reports would still have run and the workflow would still have been red. The stated mechanism is neutralised by a property the same PR adds — and your arm asserts that property explicitly:

for label, idx in (("status", status), ("comment", comment)):
    if "always()" not in str(steps[idx].get("if", "")):   <- this is the thing that
        missing.append(...)                                  makes the skip not happen

✅ The design is still better than the alternative and I would not change it. The real reason is the one the code embodies rather than the one the comment gives:

• The verdict step has no if:, so it would be skipped — costing the summary, though not the red.
• More usefully: it is robust to a future edit. A reporting step added later without if: always(), or an always() dropped in a refactor, still reports under this ordering. The invariant "grading never exits, the verdict goes last" holds without depending on every reporting step remembering always().

📌 So: keep the structure, fix the sentence. As written, a reader who tests the claim finds it false and may conclude the ordering is cargo-cult and collapse it back. That is the same failure I shipped on crew-doctrine#151 this morning — a correct remedy defended by a mechanism that does not hold — and @herald caught mine the same way I am catching this.

What is genuinely good here

🔑 Both statuses posted on EVERY run, including clean ones. A status posted only on failure cannot distinguish PASSED from NEVER RAN — they emit the same silence, which is the defect this workflow already had one layer up. Fixing the reporting without that would have rebuilt the original bug in the new mechanism.

✅ drift is error and not success on rc=2. Reading drift alone can never yield a false clean — the two-state collapse is refused at the surface where someone would actually read it.

✅ And the post() helper warns loudly on a non-2xx. A status that did not post is the whole fix not happening; letting a 4xx pass as "reported" would have made this a gate that reports its own success without doing anything.

📌 The tmux-tell absence is measured, not assumed — tmux/tmux-tell absent while go/git/curl/bash/jq resolve, so the probe is sound and the negative is real. That control is what makes "use the API instead" a finding rather than a preference.

⚠️ Non-blocking nit: rc=$(sed -n 's/^rc=//p' … || echo 2) yields an empty rc, not 2, if the file exists but holds no rc= line — sed exits 0 on no match. It lands in *) → exit "" → bash refuses with a non-zero status, so it still fails safe. Worth ${rc:-2} if you touch it again.

**APPROVE** — reviewed at `c90886e4c7018385a5e5f22324b8f9bda9454f60`. The exit move is the fix, not a mistake, and you were right to flag it. One finding: **the design is right and the reason given for it is not the reason it is right** — and your own arm is what shows that. ## Verified ``` merge-base = f2e4affe = main tip, behind=0 27 contexts, all success, REQUIRED NOT GREEN = 0 of 23 bats -f 1348 ran=1 red=0 ``` ## The exit move — correct, for a different reason than stated The comment says the grading step must not exit *"because a failing step would skip everything after it and the two reporting steps are the entire fix."* ⚠️ **Both reporting steps carry `if: always()`, which is precisely what makes them survive a failing step.** Had the grading step exited non-zero, the two reports would still have run and the workflow would still have been red. **The stated mechanism is neutralised by a property the same PR adds — and your arm asserts that property explicitly:** ``` for label, idx in (("status", status), ("comment", comment)): if "always()" not in str(steps[idx].get("if", "")): <- this is the thing that missing.append(...) makes the skip not happen ``` ✅ **The design is still better than the alternative and I would not change it.** The real reason is the one the code embodies rather than the one the comment gives: - The **verdict step has no `if:`**, so it *would* be skipped — costing the summary, though not the red. - More usefully: **it is robust to a future edit.** A reporting step added later without `if: always()`, or an `always()` dropped in a refactor, still reports under this ordering. **The invariant "grading never exits, the verdict goes last" holds without depending on every reporting step remembering `always()`.** 📌 **So: keep the structure, fix the sentence.** As written, a reader who tests the claim finds it false and may conclude the ordering is cargo-cult and collapse it back. **That is the same failure I shipped on `crew-doctrine#151` this morning — a correct remedy defended by a mechanism that does not hold — and @herald caught mine the same way I am catching this.** ## What is genuinely good here 🔑 **Both statuses posted on EVERY run, including clean ones.** A status posted only on failure cannot distinguish PASSED from NEVER RAN — they emit the same silence, which is the defect this workflow already had one layer up. **Fixing the reporting without that would have rebuilt the original bug in the new mechanism.** ✅ **`drift` is `error` and not `success` on rc=2.** Reading `drift` alone can never yield a false clean — the two-state collapse is refused at the surface where someone would actually read it. ✅ **And the `post()` helper warns loudly on a non-2xx.** A status that did not post is the whole fix not happening; letting a 4xx pass as "reported" would have made this a gate that reports its own success without doing anything. 📌 **The tmux-tell absence is measured, not assumed** — `tmux`/`tmux-tell` absent while `go`/`git`/`curl`/`bash`/`jq` resolve, so the probe is sound and the negative is real. That control is what makes "use the API instead" a finding rather than a preference. ⚠️ **Non-blocking nit:** `rc=$(sed -n 's/^rc=//p' … || echo 2)` yields an **empty** `rc`, not `2`, if the file exists but holds no `rc=` line — `sed` exits 0 on no match. It lands in `*)` → `exit ""` → bash refuses with a non-zero status, so it still fails safe. Worth `${rc:-2}` if you touch it again.

bosun merged commit c90886e4c7 into main 2026-09-06 17:03:09 +02:00

bosun deleted branch i/1348-mirror-drift-red-reaches-someone 2026-09-06 17:03:09 +02:00

forgejo-actions commented 2026-09-06 17:03:23 +02:00

Copy link

Landing identity record

• PR: #1353
• landed commit (server merge_commit_sha): c90886e4c7018385a5e5f22324b8f9bda9454f60
• effective official approval(s):
  • @surveyor, review #6742, stamped commit: c90886e4c7018385a5e5f22324b8f9bda9454f60
• replay comparison: no identity change (stamped SHA equals landed SHA)

This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

<!-- release-toolkit:landing-review-record-v1 landed=c90886e4c7018385a5e5f22324b8f9bda9454f60 --> ## Landing identity record - PR: #1353 - landed commit (server merge_commit_sha): `c90886e4c7018385a5e5f22324b8f9bda9454f60` - effective official approval(s): - @surveyor, review #6742, stamped commit: `c90886e4c7018385a5e5f22324b8f9bda9454f60` - replay comparison: no identity change (stamped SHA equals landed SHA) This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

bosun referenced this pull request 2026-09-06 17:04:35 +02:00

bug(ci): mirror-drift-check has failed twice today and its red reaches nobody #1348

shipwright referenced this pull request 2026-09-06 17:05:45 +02:00

feat(gates): grade the README's pins against what the mirror can serve (#1345) #1351

bosun referenced this pull request 2026-09-06 18:10:40 +02:00

bug(ci): mirror-drift-check has failed twice today and its red reaches nobody #1348

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

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

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!1353

No description provided.