sneak/webhooker
1
0
Fork 0
Code Issues 67 Pull Requests Actions Packages Projects Releases Wiki Activity

Superseded-run status laundering: a never-tested commit reads green in the combined status #152

New Issue
Closed
opened 2026-08-12 13:33:05 +02:00 by clawbot · 1 comment
clawbot commented 2026-08-12 13:33:05 +02:00
Collaborator
Copy Link

Found by the final integration review of #111. Not milestoned 1.0.0 — next is verified green by cache-defeated container runs, which is the stronger evidence and is what the tag actually rests on. This is about the badge, not the tree.

#119 fixed two real problems and its fix works: be57609 carries a genuine 2m52s run. But its cancellation half has a side effect worth naming.

Gitea cancels an in-flight run when a newer commit lands and records the cancellation as failure. The repair rewrites that exact failure / "Has been cancelled" status to skipped / "Superseded by a newer commit; never tested". Gitea's Combine() folds skipped into success — so the combined-status API returns "state":"success" for a commit that was never tested.

Observed on next: 0e397b3 and 95161c7 both carry skipped / "never tested" and both report combined success. 95161c7 is the aggregate rate-limit fix (#139) — the very change the prior integration review failed this PR for.

The description attached is honest; the rollup is not. Anything reading combined status programmatically — a branch protection rule, a release script, a future manager — sees green for a commit nothing ever ran.

This is distinct from #147, which is about the context string being hardcoded.

The alternative considered and rejected in #119 was leaving those commits pending, which would block them permanently. That trade was reasonable; the question is whether there is a third option.

Options worth weighing

  1. Re-run the check on the superseded commit rather than relabelling it, so its status becomes real.
  2. Leave failure in place but fix the description, accepting red-but-honest over green-but-false.
  3. Keep skipped and accept it, documenting that combined status is not a reliable per-commit signal on this repo.

Definition of done

  • Either a never-tested commit no longer reports combined success, or the limitation is documented where someone building on the status API will see it.
  • Whatever is chosen, git bisect-style archaeology over next should be able to distinguish "passed" from "never ran".

Implementation requirements

  • Branch from next, PR based on next, single commit, title ending (closes #N).
  • Do not modify TODO.md.
  • Gate on make check plus the Docker lint path with the cache defeated.
Found by the final integration review of https://git.eeqj.de/sneak/webhooker/pulls/111. Not milestoned 1.0.0 — `next` is verified green by cache-defeated container runs, which is the stronger evidence and is what the tag actually rests on. This is about the badge, not the tree. https://git.eeqj.de/sneak/webhooker/issues/119 fixed two real problems and its fix works: `be57609` carries a genuine 2m52s run. But its cancellation half has a side effect worth naming. Gitea cancels an in-flight run when a newer commit lands and records the cancellation as `failure`. The repair rewrites that exact `failure` / "Has been cancelled" status to `skipped` / "Superseded by a newer commit; never tested". Gitea's `Combine()` folds `skipped` into success — so the combined-status API returns `"state":"success"` for a commit that was never tested. Observed on `next`: `0e397b3` and `95161c7` both carry `skipped` / "never tested" and both report combined `success`. `95161c7` is the aggregate rate-limit fix (https://git.eeqj.de/sneak/webhooker/issues/139) — the very change the prior integration review failed this PR for. The description attached is honest; the rollup is not. Anything reading combined status programmatically — a branch protection rule, a release script, a future manager — sees green for a commit nothing ever ran. This is distinct from https://git.eeqj.de/sneak/webhooker/issues/147, which is about the context string being hardcoded. The alternative considered and rejected in https://git.eeqj.de/sneak/webhooker/issues/119 was leaving those commits `pending`, which would block them permanently. That trade was reasonable; the question is whether there is a third option. ## Options worth weighing 1. Re-run the check on the superseded commit rather than relabelling it, so its status becomes real. 2. Leave `failure` in place but fix the description, accepting red-but-honest over green-but-false. 3. Keep `skipped` and accept it, documenting that combined status is not a reliable per-commit signal on this repo. ## Definition of done - Either a never-tested commit no longer reports combined `success`, or the limitation is documented where someone building on the status API will see it. - Whatever is chosen, `git bisect`-style archaeology over `next` should be able to distinguish "passed" from "never ran". ## Implementation requirements - Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`. - Do not modify `TODO.md`. - Gate on `make check` plus the Docker lint path with the cache defeated.
clawbot self-assigned this 2026-08-12 13:33:05 +02:00
clawbot added this to the 1.0.0 milestone 2026-08-17 22:29:41 +02:00
clawbot commented 2026-08-17 22:29:59 +02:00
Author
Collaborator
Copy Link

Moved INTO the 1.0.0 milestone, reversing the "Not milestoned" line in the body above.

Reason: the body is right that the tag rests on cache-defeated container runs, not the badge — but 1.0 is the point where the repo starts being consumed programmatically. A combined status that returns success for a commit nothing ever ran is a false green that branch protection, a release script, or the next manager will read as fact. That makes the release record unverifiable, which is a release blocker even though the tree itself is fine.

Decision on the three options in the body: take option 1 where it is achievable — re-run the check on the superseded commit so its status becomes real — and fall back to option 2 (leave it failure, fix only the description) rather than option 3. Never-tested must not fold into green. Red-but-honest is acceptable on a superseded commit; green-but-false is not, because it is indistinguishable from a passed run at exactly the moment someone is doing git bisect archaeology.

Moved INTO the `1.0.0` milestone, reversing the "Not milestoned" line in the body above. Reason: the body is right that the tag rests on cache-defeated container runs, not the badge — but 1.0 is the point where the repo starts being consumed programmatically. A combined status that returns `success` for a commit nothing ever ran is a false green that branch protection, a release script, or the next manager will read as fact. That makes the release record unverifiable, which is a release blocker even though the tree itself is fine. Decision on the three options in the body: take option 1 where it is achievable — re-run the check on the superseded commit so its status becomes real — and fall back to option 2 (leave it `failure`, fix only the description) rather than option 3. Never-tested must not fold into green. Red-but-honest is acceptable on a superseded commit; green-but-false is not, because it is indistinguishable from a passed run at exactly the moment someone is doing `git bisect` archaeology.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees clawbot
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#152
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?