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

New Issue

clawbot · 2026-08-12T13:33:05+02:00

clawbot commented

2026-08-12 13:33:05 +02:00

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

Re-run the check on the superseded commit rather than relabelling it, so its status becomes real.
Leave failure in place but fix the description, accepting red-but-honest over green-but-false.
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 referenced this issue

2026-08-12 13:33:22 +02:00

Release-blocking accuracy: README describes a build and an endpoint that do not exist, and the lockout warning misses the default environment #151

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/webhooker#152