heavy-duty/ceremony

Fork 2

fix: a queue-cancelled duplicate check is not a verdict — checks_state discards it when a real one stands #140

Merged

claude-bot-andresmgsl merged 1 commit from build/139-cancelled-not-verdict into main

2026-07-24 13:14:37 +00:00

claude-bot-andresmgsl commented

2026-07-24 12:35:18 +00:00

(Migrated from github.com)

Closes #139.

A labels / reconcile run queued in the repo-global concurrency group attaches its check run to the PR's head before any other repo event — a sibling PR's push, a triage issue write — evicts it as the queued duplicate. checks_state then read that CANCELLED entry as the context's newest verdict: blocker:ci-red on a PR whose real checks were all green, evictable only by an empty commit (this happened to both #133 at 4002924 and #136 at a17e497).

The fix is one function deep, exactly per the spec's rule: inside a context group, a CANCELLED entry is discarded when the group holds at least one non-cancelled entry, applied before the sort so an eviction that arrived newest cannot outvote the verdict beside it. A context whose entries are all cancelled never reported at all — a killed or timed-out required job — so it keeps CANCELLED and still classifies FAILURE. The comment block above checks_state states the carve-out in the same list-what-passes voice, including why it does not widen unknown-into-green. blockers(), decide_state, and .github/workflows/labels.yml (repo-global group, cancel-in-progress: false) are untouched.

The fixture that pinned the old behaviour — "the reverse order is not a re-run passing, it is one failing" — was written for a cancelled run that replaced a success and never saw one that replaced nothing; it is rewritten with that reason in its comment, not deleted.

Acceptance criteria (from #139)

{SUCCESS 12:16:17→12:17:06, CANCELLED 12:16:41→12:16:41} — the recorded a17e497 shape, as a recorded payload — classifies SUCCESS
A context whose only entry is CANCELLED classifies FAILURE; so does a group of two cancelled entries
{FAILURE older, CANCELLED newest} still classifies FAILURE
Every other existing fixture passes unchanged — "a CANCELLED run blocks" (different context), "a re-run supersedes the cancelled original", "a replacement in flight for a CANCELLED run is pending", both wind-down cases: suite 142/142
ERROR, STALE, unknown outcomes still block; NEUTRAL/SKIPPED pass; UNREADABLE unchanged
Rewritten fixture's comment says why a cancelled entry that replaced nothing is not a verdict
blockers() and decide_state untouched — reader fix, one function deep
labels.yml unchanged: repo-global group, cancel-in-progress: false
changelog.d/139.md exists
test/run.sh green

Worklog

Amend checks_state: pre-sort CANCELLED discard with non-cancelled-survivor guard
Comment block: the carve-out and its limit, in the list-what-passes voice
Rewrite the reverse-order fixture with its reason
New fixtures: recorded a17e497 payload, only-CANCELLED, two-CANCELLED, FAILURE-older-CANCELLED-newest
changelog.d/139.md
Local: test/run.sh 17/17 files (labels-reconcile 142/142), .github/scripts/shellcheck-all.sh clean, actionlint clean
Mark ready, request the panel

Closes #139. A `labels / reconcile` run queued in the repo-global concurrency group attaches its check run to the PR's head **before** any other repo event — a sibling PR's push, a triage issue write — evicts it as the queued duplicate. `checks_state` then read that `CANCELLED` entry as the context's newest verdict: `blocker:ci-red` on a PR whose real checks were all green, evictable only by an empty commit (this happened to both #133 at `4002924` and #136 at `a17e497`). The fix is one function deep, exactly per the spec's rule: inside a context group, a `CANCELLED` entry is **discarded when the group holds at least one non-cancelled entry**, applied **before** the sort so an eviction that arrived newest cannot outvote the verdict beside it. A context whose entries are *all* cancelled never reported at all — a killed or timed-out required job — so it keeps `CANCELLED` and still classifies `FAILURE`. The comment block above `checks_state` states the carve-out in the same list-what-passes voice, including why it does not widen unknown-into-green. `blockers()`, `decide_state`, and `.github/workflows/labels.yml` (repo-global group, `cancel-in-progress: false`) are untouched. The fixture that pinned the old behaviour — *"the reverse order is not a re-run passing, it is one failing"* — was written for a cancelled run that *replaced* a success and never saw one that replaced nothing; it is rewritten with that reason in its comment, not deleted. ## Acceptance criteria (from #139) - [x] `{SUCCESS 12:16:17→12:17:06, CANCELLED 12:16:41→12:16:41}` — the recorded `a17e497` shape, as a recorded payload — classifies `SUCCESS` - [x] A context whose only entry is `CANCELLED` classifies `FAILURE`; so does a group of two cancelled entries - [x] `{FAILURE older, CANCELLED newest}` still classifies `FAILURE` - [x] Every other existing fixture passes unchanged — *"a CANCELLED run blocks"* (different context), *"a re-run supersedes the cancelled original"*, *"a replacement in flight for a CANCELLED run is pending"*, both wind-down cases: suite 142/142 - [x] `ERROR`, `STALE`, unknown outcomes still block; `NEUTRAL`/`SKIPPED` pass; `UNREADABLE` unchanged - [x] Rewritten fixture's comment says why a cancelled entry that replaced nothing is not a verdict - [x] `blockers()` and `decide_state` untouched — reader fix, one function deep - [x] `labels.yml` unchanged: repo-global group, `cancel-in-progress: false` - [x] `changelog.d/139.md` exists - [x] `test/run.sh` green ## Worklog - [x] Amend `checks_state`: pre-sort CANCELLED discard with non-cancelled-survivor guard - [x] Comment block: the carve-out and its limit, in the list-what-passes voice - [x] Rewrite the reverse-order fixture with its reason - [x] New fixtures: recorded `a17e497` payload, only-CANCELLED, two-CANCELLED, FAILURE-older-CANCELLED-newest - [x] `changelog.d/139.md` - [x] Local: `test/run.sh` 17/17 files (labels-reconcile 142/142), `.github/scripts/shellcheck-all.sh` clean, actionlint clean - [x] Mark ready, request the panel

danmt (Migrated from github.com) reviewed 2026-07-24 12:35:18 +00:00

claude-bot-andresmgsl commented

2026-07-24 12:41:04 +00:00

(Migrated from github.com)

⟲ resuming from d8f54aab04

⟲ resuming from d8f54aab04c44d34281875826c7895ed489e0b5b

codex-bot-andresmgsl commented

2026-07-24 12:45:46 +00:00

(Migrated from github.com)

🔎 reviewing head d8f54aab04

🔎 reviewing head d8f54aab04c44d34281875826c7895ed489e0b5b

grok-bot-andresmgsl commented

2026-07-24 12:46:02 +00:00

(Migrated from github.com)

🔎 reviewing head d8f54aab04

🔎 reviewing head d8f54aab04c44d34281875826c7895ed489e0b5b

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-24 12:47:08 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

The implementation matches #139’s acceptance criteria and is correctly scoped to the reader.

Verified:

checks_state discards CANCELLED only when the same context retains a non-cancelled entry, before newest-entry selection; all-cancelled contexts and {FAILURE older, CANCELLED newest} remain FAILURE.
The recorded a17e497 timing shape and the rewritten reverse-order fixture are pinned with the required rationale.
Existing enum behavior remains covered (ERROR, STALE, unknown outcomes block; NEUTRAL/SKIPPED pass; unreadable rollups remain distinct).
blockers(), decide_state, and .github/workflows/labels.yml are unchanged, and changelog.d/139.md is present.
test/run.sh passed all 17 test files, including labels-reconcile 142/142; shellcheck passed for all 37 tracked scripts; actionlint passed.

The current GitHub check-run payload also reproduces the bug shape on this exact head: a successful labels / reconcile at 12:35 sits beside a later cancelled duplicate at 12:41, while the substantive CI jobs are green. The patched classifier resolves that context to the real successful verdict as specified.

The implementation matches #139’s acceptance criteria and is correctly scoped to the reader. Verified: - `checks_state` discards `CANCELLED` only when the same context retains a non-cancelled entry, before newest-entry selection; all-cancelled contexts and `{FAILURE older, CANCELLED newest}` remain `FAILURE`. - The recorded `a17e497` timing shape and the rewritten reverse-order fixture are pinned with the required rationale. - Existing enum behavior remains covered (`ERROR`, `STALE`, unknown outcomes block; `NEUTRAL`/`SKIPPED` pass; unreadable rollups remain distinct). - `blockers()`, `decide_state`, and `.github/workflows/labels.yml` are unchanged, and `changelog.d/139.md` is present. - `test/run.sh` passed all 17 test files, including `labels-reconcile` 142/142; shellcheck passed for all 37 tracked scripts; actionlint passed. The current GitHub check-run payload also reproduces the bug shape on this exact head: a successful `labels / reconcile` at 12:35 sits beside a later cancelled duplicate at 12:41, while the substantive CI jobs are green. The patched classifier resolves that context to the real successful verdict as specified.

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-24 12:48:41 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: approve

Reviewed head d8f54aab04c44d34281875826c7895ed489e0b5b against #139.

Acceptance criteria (#139)

Criterion	Result
Recorded `a17e497` shape `{SUCCESS …, CANCELLED same-second start/end}` → SUCCESS	Met — fixture with recorded timestamps; pre-sort discard of CANCELLED when a non-cancelled sibling exists.
Only-CANCELLED / two-CANCELLED contexts → FAILURE	Met — both fixtures pin FAILURE (no unknown-as-green widening).
`{FAILURE older, CANCELLED newest}` → FAILURE	Met — survivor is the FAILURE.
Existing fixtures still pass; reverse-order case rewritten with reason	Met — suite green; rewrite comment explains a cancelled entry that replaced nothing is not a verdict.
ERROR/STALE/unknown block; NEUTRAL/SKIPPED pass; UNREADABLE unchanged	Met — list-what-passes path untouched; only the CANCELLED discard sits before the existing sort/latest collapse.
`blockers()` / `decide_state` untouched; `labels.yml` concurrency unchanged	Met — diff is one function deep in `checks_state` plus tests/changelog; zero lines in `labels.yml`.
`changelog.d/139.md` + `test/run.sh` green	Met.

Verification (this tick)

Full suite at head: 17/17 files (worktree).
test/labels-reconcile.test.sh: 142/142.
Hosted CI: test, self-guards, and release/docs/action exercises all success.
On this head, labels / reconcile has both a success and a cancelled check run (empty steps on the cancelled job) — exactly the queue-eviction shape #139 records. The blocker:ci-red on the PR is the bug under fix (main's checks_state still lets the newest CANCELLED outvote the SUCCESS sibling); it is not evidence of a red real check. After merge, this reader fix is what clears that false red.

Spec fit

Reader-only carve-out, survivor required before discard, no workflow concurrency change. Approve.

## Verdict: approve Reviewed head `d8f54aab04c44d34281875826c7895ed489e0b5b` against #139. ### Acceptance criteria (#139) | Criterion | Result | |---|---| | Recorded a17e497 shape `{SUCCESS …, CANCELLED same-second start/end}` → SUCCESS | **Met** — fixture with recorded timestamps; pre-sort discard of CANCELLED when a non-cancelled sibling exists. | | Only-CANCELLED / two-CANCELLED contexts → FAILURE | **Met** — both fixtures pin FAILURE (no unknown-as-green widening). | | `{FAILURE older, CANCELLED newest}` → FAILURE | **Met** — survivor is the FAILURE. | | Existing fixtures still pass; reverse-order case rewritten with reason | **Met** — suite green; rewrite comment explains a cancelled entry that replaced nothing is not a verdict. | | ERROR/STALE/unknown block; NEUTRAL/SKIPPED pass; UNREADABLE unchanged | **Met** — list-what-passes path untouched; only the CANCELLED discard sits before the existing sort/latest collapse. | | `blockers()` / `decide_state` untouched; `labels.yml` concurrency unchanged | **Met** — diff is one function deep in `checks_state` plus tests/changelog; zero lines in `labels.yml`. | | `changelog.d/139.md` + `test/run.sh` green | **Met**. | ### Verification (this tick) - Full suite at head: **17/17 files** (worktree). - `test/labels-reconcile.test.sh`: **142/142**. - Hosted CI: `test`, `self-guards`, and release/docs/action exercises all **success**. - On this head, `labels / reconcile` has both a **success** and a **cancelled** check run (empty steps on the cancelled job) — exactly the queue-eviction shape #139 records. The `blocker:ci-red` on the PR is the bug under fix (main's `checks_state` still lets the newest CANCELLED outvote the SUCCESS sibling); it is not evidence of a red real check. After merge, this reader fix is what clears that false red. ### Spec fit Reader-only carve-out, survivor required before discard, no workflow concurrency change. Approve.

kimi-bot-andresmgsl commented

2026-07-24 12:49:49 +00:00

(Migrated from github.com)

🔎 reviewing head d8f54aab04

🔎 reviewing head d8f54aab04c44d34281875826c7895ed489e0b5b

kimi-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-24 13:02:16 +00:00

kimi-bot-andresmgsl (Migrated from github.com) left a comment

Approved.

Verified against #139 in a detached worktree at the head (d8f54aab, git worktree add --detach), base origin/main for comparison.

Classifier change (AC: reader fix, one function deep). The diff touches exactly three files: actions/labels-reconcile/labels-reconcile.sh (two hunks, both inside checks_state — the comment block at :204 and the jq at :268-275), test/labels-reconcile.test.sh, and new changelog.d/139.md. blockers() and decide_state are untouched; .github/workflows/labels.yml is not in the diff. The discard is applied inside the context group before the sort (map(select(.outcome != "CANCELLED")) as $live | (if ($live|length) > 0 then $live else . end) | sort_by ... | last), matching the spec's rule and its illustrative jq verbatim. An all-cancelled group keeps its entries and still classifies FAILURE.

Tests. This box lacks jq/node, so test/run.sh is red here — but identically red on origin/main (13/17 files, failures only in package-json node-dependent cases in changelog-armed, drill-recorded, facts, version; pre-existing, unrelated to the diff). With a static jq 1.7.1 on PATH: test/labels-reconcile.test.sh 142/142 at the head (138/138 on main; the PR nets +4 fixtures), and all 13 files that can pass in this environment do.

Fixture discrimination (ran the fixture payloads through four implementations, not just the shipped one):

Old code (main): the a17e497 recorded shape and the rewritten {SUCCESS older, CANCELLED newest} both classify FAILURE — the bug; both fixtures pin the fix.
Head d8f54aa: a17e497 → SUCCESS, rewritten reverse-order → SUCCESS, only-CANCELLED → FAILURE, two-CANCELLED → FAILURE, {FAILURE older, CANCELLED newest} → FAILURE. All five as specified.
"Cancelled never blocks" (CANCELLED added to $passing): only-CANCELLED, two-CANCELLED, and FAILURE-older all wrongly read SUCCESS — the three guard fixtures catch it, per the test plan.
Discard applied after the sort (strip CANCELLED from the collapsed $latest): the a17e497 shape reads NONE, not SUCCESS — caught, per the test plan.

Other ACs. The rewritten reverse-order fixture keeps its place with a comment explaining why a cancelled run that replaced nothing is not a verdict — rewritten, not deleted. Existing fixtures for ERROR/STALE/unknown-outcome blocking, NEUTRAL/SKIPPED passing, UNREADABLE, "a CANCELLED run blocks" (different context), both wind-down cases, and the in-flight cases are all present and pass unchanged. The comment block above checks_state states the carve-out and why it does not widen unknown-into-green ("discard needs a surviving verdict, never an empty context"). changelog.d/139.md follows the fragment convention (<issue>.md, publishable prose, per changelog.d/README.md).

#133 overlap. #133 is still open; its hunks in labels-reconcile.sh sit at ~:175, ~:498, ~:581-614, ~:656-696 (release_shape_warning/reconcile_pr/main) and its test hunk at ~:92 — disjoint from this diff's checks_state (:204-289) and test (:363-407) regions, as #139's Dependencies note predicted. git merge-tree of this head against current main: no conflicts. Whoever lands second rebases, but no same-line collision is expected.

The standing blocker:ci-red on this PR is the bug itself. On head d8f54aab there is a real labels / reconcile success (12:35:37Z → 12:36:18Z) beside a later queue-cancelled zero-step duplicate (started == completed 12:41:53Z, run 30094035344) that gh pr checks surfaces as "fail". Under this PR's own rule the cancelled entry is discarded and the context reads SUCCESS; all other checks on the head are green. The label should clear on the next reconcile sweep.

Non-blocking nit: with the survivor guard removed entirely (unconditional pre-sort drop), all five fixtures still classify identically — an emptied group collapses to a null outcome, which is in neither $passing nor $waiting and still blocks. The guard is therefore defensive clarity rather than load-bearing, and it matches the issue's sketch verbatim; fine as shipped, no change requested.

Approved. Verified against #139 in a detached worktree at the head (`d8f54aab`, `git worktree add --detach`), base `origin/main` for comparison. **Classifier change (AC: reader fix, one function deep).** The diff touches exactly three files: `actions/labels-reconcile/labels-reconcile.sh` (two hunks, both inside `checks_state` — the comment block at :204 and the jq at :268-275), `test/labels-reconcile.test.sh`, and new `changelog.d/139.md`. `blockers()` and `decide_state` are untouched; `.github/workflows/labels.yml` is not in the diff. The discard is applied inside the context group **before** the sort (`map(select(.outcome != "CANCELLED")) as $live | (if ($live|length) > 0 then $live else . end) | sort_by ... | last`), matching the spec's rule and its illustrative jq verbatim. An all-cancelled group keeps its entries and still classifies FAILURE. **Tests.** This box lacks `jq`/`node`, so `test/run.sh` is red here — but identically red on `origin/main` (13/17 files, failures only in `package-json` node-dependent cases in changelog-armed, drill-recorded, facts, version; pre-existing, unrelated to the diff). With a static jq 1.7.1 on PATH: **`test/labels-reconcile.test.sh` 142/142 at the head** (138/138 on main; the PR nets +4 fixtures), and all 13 files that can pass in this environment do. **Fixture discrimination (ran the fixture payloads through four implementations, not just the shipped one):** - Old code (main): the a17e497 recorded shape and the rewritten `{SUCCESS older, CANCELLED newest}` both classify FAILURE — the bug; both fixtures pin the fix. - Head d8f54aa: a17e497 → SUCCESS, rewritten reverse-order → SUCCESS, only-CANCELLED → FAILURE, two-CANCELLED → FAILURE, `{FAILURE older, CANCELLED newest}` → FAILURE. All five as specified. - "Cancelled never blocks" (CANCELLED added to `$passing`): only-CANCELLED, two-CANCELLED, and FAILURE-older all wrongly read SUCCESS — the three guard fixtures catch it, per the test plan. - Discard applied after the sort (strip CANCELLED from the collapsed `$latest`): the a17e497 shape reads NONE, not SUCCESS — caught, per the test plan. **Other ACs.** The rewritten reverse-order fixture keeps its place with a comment explaining why a cancelled run that replaced nothing is not a verdict — rewritten, not deleted. Existing fixtures for ERROR/STALE/unknown-outcome blocking, NEUTRAL/SKIPPED passing, UNREADABLE, "a CANCELLED run blocks" (different context), both wind-down cases, and the in-flight cases are all present and pass unchanged. The comment block above `checks_state` states the carve-out and why it does not widen unknown-into-green ("discard needs a surviving verdict, never an empty context"). `changelog.d/139.md` follows the fragment convention (`<issue>.md`, publishable prose, per `changelog.d/README.md`). **#133 overlap.** #133 is still open; its hunks in `labels-reconcile.sh` sit at ~:175, ~:498, ~:581-614, ~:656-696 (`release_shape_warning`/`reconcile_pr`/`main`) and its test hunk at ~:92 — disjoint from this diff's `checks_state` (:204-289) and test (:363-407) regions, as #139's Dependencies note predicted. `git merge-tree` of this head against current main: no conflicts. Whoever lands second rebases, but no same-line collision is expected. **The standing `blocker:ci-red` on this PR is the bug itself.** On head d8f54aab there is a real `labels / reconcile` success (12:35:37Z → 12:36:18Z) beside a later queue-cancelled zero-step duplicate (started == completed 12:41:53Z, run 30094035344) that `gh pr checks` surfaces as "fail". Under this PR's own rule the cancelled entry is discarded and the context reads SUCCESS; all other checks on the head are green. The label should clear on the next reconcile sweep. Non-blocking nit: with the survivor guard removed entirely (unconditional pre-sort drop), all five fixtures still classify identically — an emptied group collapses to a `null` outcome, which is in neither `$passing` nor `$waiting` and still blocks. The guard is therefore defensive clarity rather than load-bearing, and it matches the issue's sketch verbatim; fine as shipped, no change requested.

claude-bot-andresmgsl commented

2026-07-24 13:10:54 +00:00

(Migrated from github.com)

Round summary — handoff

One round, no fixes owed: codex (12:47), grok (12:48) and kimi (13:02) all approved head d8f54aa — the head has not moved since ready-for-review. What was verified: checks_state now discards a CANCELLED entry inside a context group only when that group holds a non-cancelled sibling (a real verdict about the same bytes); a group whose entries are all cancelled still reads FAILURE — the unknown-as-green shape stays refused. The L366 fixture is rewritten with its reason, the three spec fixtures are in, full suite green.

On the standing blocker:ci-red — it is the #139 flap itself, manufactured on this PR's own head. The evidence, from the check-runs API for d8f54aa:

check run	`labels / reconcile`	started
`89482378022`	success	12:35:37
`89483703568`	cancelled (queue-evicted from the global `labels-reconcile` group; run `30094035344`)	12:41:53

Every real check on this head is green; the newest rollup entry is the cancelled duplicate, and main's pre-fix classifier reads newest-wins → FAILURE. This PR is the fix for exactly that read, but the base branch governs its own checks. gh run rerun refuses the cancelled run, and the documented empty-commit remedy (#139: "the only builder-available fix") would stale three current-head approvals to change zero bytes.

So this handoff uses the designed path instead: the state:needs-human write below is the optimistic write BUILDER.md describes, and its labeled event wakes the sweep at this head — that run's own labels / reconcile check run supersedes the cancelled entry in the rollup, and the same sweep validates (or corrects) the label. If it corrects me, the completed run still heals the rollup and the next tick converges; no head move, no re-round either way.

Requesting @danmt — over to you.

## Round summary — handoff **One round, no fixes owed**: codex (12:47), grok (12:48) and kimi (13:02) all approved head `d8f54aa` — the head has not moved since ready-for-review. What was verified: `checks_state` now discards a `CANCELLED` entry inside a context group **only when that group holds a non-cancelled sibling** (a real verdict about the same bytes); a group whose entries are all cancelled still reads `FAILURE` — the unknown-as-green shape stays refused. The L366 fixture is rewritten with its reason, the three spec fixtures are in, full suite green. **On the standing `blocker:ci-red` — it is the #139 flap itself, manufactured on this PR's own head.** The evidence, from the check-runs API for `d8f54aa`: | check run | `labels / reconcile` | started | |---|---|---| | `89482378022` | **success** | 12:35:37 | | `89483703568` | **cancelled** (queue-evicted from the global `labels-reconcile` group; run `30094035344`) | 12:41:53 | Every real check on this head is green; the newest rollup entry is the cancelled duplicate, and main's pre-fix classifier reads newest-wins → `FAILURE`. This PR is the fix for exactly that read, but the base branch governs its own checks. `gh run rerun` refuses the cancelled run, and the documented empty-commit remedy (#139: "the only builder-available fix") would stale three current-head approvals to change zero bytes. So this handoff uses the designed path instead: the `state:needs-human` write below is the optimistic write BUILDER.md describes, and its `labeled` event wakes the sweep at this head — that run's own `labels / reconcile` check run supersedes the cancelled entry in the rollup, and the same sweep validates (or corrects) the label. If it corrects me, the completed run still heals the rollup and the next tick converges; no head move, no re-round either way. Requesting @danmt — over to you.