Merge pull request 'Issue 305: a recheck left the old verdict standing' (#190) from issues/305-a-recheck-left-the-old-verdict-standing into main
This commit was merged in pull request #190.
This commit is contained in:
@@ -0,0 +1,84 @@
|
||||
---
|
||||
status: resolved
|
||||
opened: 2026-10-08
|
||||
located-in: [mesh-catalog modules/mesh-delivery (verbs.go, the recheck verb), mesh-catalog modules/gitea (delivery.ts, the commit status tool)]
|
||||
fixed-by: novox/mesh-catalog PR #118
|
||||
replay: R305
|
||||
amended-design:
|
||||
---
|
||||
|
||||
# 305. A recheck left the old verdict standing
|
||||
|
||||
## Symptom
|
||||
|
||||
On 2026-10-07 at 23:04 a pull request of the controller's repository was rechecked with
|
||||
`mesh-delivery.recheck`. Straight after, `mesh-delivery.checks` for that pull request said:
|
||||
|
||||
- the delivery: proposed, waiting for its check;
|
||||
- `mesh/delivery`: pending;
|
||||
- `mesh/merge-gate` and `mesh/repo-check`: **success**, set at 16:59, the verdict from before;
|
||||
- `merge.mergeable`: **true**.
|
||||
|
||||
Both merge-check statuses are required by the branch's protection. With them still green, the forge would
|
||||
have let the pull request merge on the old verdict while its fresh check ran. That old verdict was not safe.
|
||||
Once the fresh checks came back, some of the rechecked pull requests turned red: judged against the trunk
|
||||
of that day, they no longer composed. A merge in that window would have let a broken change in.
|
||||
|
||||
## Why it is a design issue
|
||||
|
||||
[ADR 0239](../../02-DECISIONS/0239-a-delivery-is-owned-by-the-mesh-delivery-module-and-runs-from-commit-to-delivered.md)
|
||||
§5 gives the forge's holder the statuses, and §6 lets the delivery's owner ask the controller to
|
||||
check one head again. [ADR 0237](../../02-DECISIONS/0237-a-change-is-judged-against-the-mesh-that-runs-before-it-merges-on-the-build-seat.md)
|
||||
says a change is judged before it merges. The two never met on one point: the forge keeps the newest
|
||||
status of each context on a commit, and a recheck asks about the same commit. Nothing said that the
|
||||
old verdict stops counting when a new check is asked.
|
||||
|
||||
The forge's holder says "pending" only when it announces a new head. A recheck does not go through that
|
||||
announcement. The controller runs its own path for a pull request directly, because the delivery's owner
|
||||
asked, so nobody marked the head as being checked. And since
|
||||
[issue 293](../293-a-warning-on-a-required-check-blocked-the-merge/00-report.md), the forge's holder
|
||||
refuses to set the merge check's statuses by hand at all. So the delivery's owner could not have reset
|
||||
them either, even if it had tried.
|
||||
|
||||
## Fix
|
||||
|
||||
**A recheck puts the merge check back to pending before it asks.** The delivery's owner first checks,
|
||||
on a copy, that its state table takes the recheck. It then asks the forge's holder to set
|
||||
`mesh/merge-gate` to pending, with "checking again: <why>". It does the same for `mesh/repo-check`, but
|
||||
only when the head already holds one: a verdict that says no repository check would otherwise leave a
|
||||
pending status standing for ever. Only after that does the delivery move back to proposed and its check
|
||||
get asked. If the forge cannot be told, the whole recheck is refused. The delivery stays where it was and
|
||||
no check is asked on a verdict that is still green. The person asks again once the forge answers.
|
||||
|
||||
**The forge's holder takes pending, and only pending, on the merge check's statuses.** Pending is not a
|
||||
verdict. It is what the holder already says on every new head until the verdict comes. A pass, a warning
|
||||
or a failure is still set only from the controller's verdict, so issue 293's rule stands: nothing passes
|
||||
or fails a merge outside the check.
|
||||
|
||||
The controller does not change. Its recheck path still says the fresh verdict as `checked`, and the
|
||||
forge's holder sets it from that, as for any check.
|
||||
|
||||
## How it is checked
|
||||
|
||||
- **The replay, R305**, in the delivery's owner (`TestARecheck*`). A head is judged green and is then
|
||||
rechecked. The replay asserts three things: at the moment the check is asked, the forge holds both
|
||||
statuses as pending and would not merge; a head that never had a repository check does not get one;
|
||||
and a forge that cannot be told refuses the recheck, leaving the state as it was and asking nothing.
|
||||
On the commit before the fix it fails with "as its check was asked, the forge held mesh/merge-gate
|
||||
success … the old verdict still lets it merge". On the fix it passes. It is registered in mesh-lab's
|
||||
replays register (novox/mesh-lab PR #62). The register gains the directory a catalogue module's own Go
|
||||
module lives in, so the prover can run a replay that lives there.
|
||||
- The forge holder's test (`test/delivery.test.ts`) checks that the commit status tool accepts pending on
|
||||
`mesh/merge-gate` and `mesh/repo-check` and still refuses success, warning and failure there.
|
||||
|
||||
Live: after a recheck, `mesh-delivery.checks` shows both statuses pending with "checking again: …", and
|
||||
`merge.mergeable` false, until the fresh verdict sets them.
|
||||
|
||||
The trail is in [01-diagnosis.md](01-diagnosis.md).
|
||||
|
||||
## Resolved — 2026-10-08
|
||||
|
||||
Fixed by novox/mesh-catalog PR #118. The replay is R305 in mesh-lab's replays register (novox/mesh-lab
|
||||
PR #62). Run against the fix's branch, it fails on the commit before the fix and passes on it. The forge's
|
||||
holder must go out before the delivery's owner. Until then every recheck is refused, which is safe but
|
||||
blocks rechecks.
|
||||
@@ -0,0 +1,24 @@
|
||||
# 305 — diagnosis
|
||||
|
||||
**2026-10-08.** Who writes which status, read from the code:
|
||||
|
||||
- `mesh/delivery` and `mesh/delivery-group`: written by the delivery's owner, as effects it owes after
|
||||
each transition, asked of the forge's holder through its commit status tool (ADR 0239 §5). The recheck
|
||||
moved the delivery to proposed, which set `mesh/delivery` to pending. That matches what was seen.
|
||||
- `mesh/merge-gate` and `mesh/repo-check`: written only by the forge's holder. It sets them from the
|
||||
controller's `checked` verdict, and sets `mesh/merge-gate` to pending when it announces a new head of a
|
||||
pull request. Its commit status tool refused both contexts outright (issue 293).
|
||||
- The controller writes no status. Its `delivery-check` with one head and no group runs its own
|
||||
pull-request path directly. That path asks the build seat and says the verdict as `checked`. It never
|
||||
passes through the forge's announcement, so nothing says pending.
|
||||
|
||||
**Ruled out:** that the controller owns these statuses and needs to reset them. It owns the verdict, not
|
||||
the status. Also ruled out: letting the delivery's owner write a verdict. Issue 293 forbids that, and a
|
||||
reset is not a verdict.
|
||||
|
||||
**Located** in the recheck verb of the delivery's owner, which asked the check without first resetting
|
||||
the statuses, and in the forge holder's commit status tool, which had no way to accept that reset.
|
||||
|
||||
**Order matters.** The reset is done before the check is asked, and synchronously, not as a retried
|
||||
effect. A check that touches nothing can say its verdict within the controller's own call. A pending
|
||||
status landing after that verdict would then leave the pull request blocked until its next push.
|
||||
Reference in New Issue
Block a user