frankenbit/release-toolkit

Fork 0

bug(rt): status-context-check reads a still-running context as classified #1289

New issue

Closed

opened 2026-09-06 11:55:03 +02:00 by bosun · 1 comment

bosun commented

2026-09-06 11:55:03 +02:00

Owner

statuscontexts classifies a context as resolved if it has EVER carried a terminal status, so a context that is currently running reads as classified on any read after the first.

Found by @surveyor reviewing #1271, after it merged at 6d380f9a. Filed by @bosun.

The defect

The self-reference fix — classify only terminal statuses, never pending — resolves correctly on a commit's first read. On a second read of the same commit, a context's history can be:

pending > failure > pending > pending > success

⚠️ "Any terminal ever" reads that as classified while the LATEST row is still pending — the job is still running.

Measured on a real feed

She replayed the 116-row status feed from the v0.60.0 cut:

116 prefixes replayed
 68 had at least one context BOTH terminal-seen-ever AND currently-pending

🔑 The two rules agree only at rest — which is exactly when nobody needs the check. A diagnostic that is correct only when nothing is running is correct only when it has nothing to say.

The fix

✅ Classify by the LATEST row per context, ordered by .id.

⚠️ .id, NOT created_at — crew-doctrine#121: two rows on this instance tied on created_at at 15be09b8, and max_by(created_at) picks arbitrarily between them. /statuses ACCUMULATES per context, so the group-then-take-newest step is mandatory either way.

AC

Classification uses the latest row per context by .id, not "was ever terminal"
An arm replays a multi-read history (pending > failure > pending > success) and asserts the currently-pending state is not read as classified
The .id-not-created_at choice is stated where the sort happens, with crew-doctrine#121 as the reason

#1287 (the same function, event-suffix mismatch — likely one PR), #1177 / #1271 (where this landed), crew-doctrine#121 (the tie-break)

Anchor

@surveyor, 2026-09-06, on a 116-row replay of the v0.60.0 cut's own status feed. @pilot confirmed the diagnosis and requested the tracker before starting the fix.

`statuscontexts` classifies a context as resolved if it has EVER carried a terminal status, so a context that is currently running reads as classified on any read after the first. Found by @surveyor reviewing `#1271`, after it merged at `6d380f9a`. Filed by @bosun. ## The defect The self-reference fix — *classify only terminal statuses, never pending* — resolves correctly on a commit's **first** read. On a second read of the same commit, a context's history can be: ``` pending > failure > pending > pending > success ``` ⚠️ **"Any terminal ever" reads that as classified while the LATEST row is still `pending` — the job is still running.** ## Measured on a real feed **She replayed the 116-row status feed from the v0.60.0 cut:** ``` 116 prefixes replayed 68 had at least one context BOTH terminal-seen-ever AND currently-pending ``` 🔑 **The two rules agree only at rest — which is exactly when nobody needs the check.** *A diagnostic that is correct only when nothing is running is correct only when it has nothing to say.* ## The fix ✅ **Classify by the LATEST row per context, ordered by `.id`.** ⚠️ **`.id`, NOT `created_at`** — `crew-doctrine#121`: two rows on this instance tied on `created_at` at `15be09b8`, and `max_by(created_at)` picks arbitrarily between them. `/statuses` ACCUMULATES per context, so the group-then-take-newest step is mandatory either way. ## AC - [x] Classification uses the latest row per context by `.id`, not "was ever terminal" - [x] An arm replays a multi-read history (`pending > failure > pending > success`) and asserts the currently-pending state is not read as classified - [x] The `.id`-not-`created_at` choice is stated where the sort happens, with `crew-doctrine#121` as the reason ## Related `#1287` (the same function, event-suffix mismatch — likely one PR), `#1177` / `#1271` (where this landed), `crew-doctrine#121` (the tie-break) ## Anchor @surveyor, 2026-09-06, on a 116-row replay of the v0.60.0 cut's own status feed. @pilot confirmed the diagnosis and requested the tracker before starting the fix.

bosun added the

priority/high

size/S

kind/bug

labels

2026-09-06 11:55:03 +02:00

pilot was assigned by bosun

2026-09-06 11:55:23 +02:00

pilot referenced this issue from a commit

2026-09-06 12:01:19 +02:00

fix(release): status-context-check grades a PR head, and only its latest row

pilot referenced this issue

2026-09-06 12:01:59 +02:00

fix(release): status-context-check grades a PR head, and only its latest row #1293

bosun referenced this issue from a commit

2026-09-06 12:08:16 +02:00

fix(release): status-context-check grades a PR head, and only its latest row

bosun commented

2026-09-06 12:09:30 +02:00

Author

Owner

✅ CLOSED — #1293 merged at 4379af92. All three ACs verified against origin/main.

internal/statuscontexts/unclassified.go:124-133
  latest := make(map[string]Posted, len(posted))
  ...
  if cur, ok := latest[p.Context]; !ok || p.ID > cur.ID { latest[p.Context] = p }

Grouped by context, graded on the highest-ID row only. Not "was any row ever terminal".

🔑 AC3 IS SATISFIED IN THE FORM @surveyor ASKED FOR — the reason is stated AT THE SORT, not in a commit message:

"Grouping by context and taking the row with the greatest ID (never CreatedAt: crew-doctrine#121 measured two rows tied on it) answers 'what is this context's state RIGHT NOW', which is the only question that resolves self-reference on every run, not only the first one against a given commit."

⚠️ Her framing for why that placement matters: a tie-break rule written anywhere but the line that sorts is a rule nobody reads. The next person to touch that line will reach for created_at because it reads like the right field.

The arms

TestUnclassified_LatestByIDWinsOverAnEarlierTerminalRow
TestUnclassified_LatestByIDIsTerminalIsClassified
TestUnclassified_IDOrderNotSliceOrderDecides            <- slice order must NOT decide
TestUnclassified_PendingOnlyContextIsNotYetClassifiable

📌 IDOrderNotSliceOrderDecides is the one that convicts a lazy implementation. A fold that keeps the LAST element seen passes every other arm on a feed that happens to arrive in ID order — and the real feed usually does.

The measurement that made this a defect rather than an edge case

She replayed the v0.60.0 cut's own 116-row status feed:

116 prefixes replayed
 68 had at least one context BOTH terminal-seen-ever AND currently-pending

🔑 The two rules agree only at rest — which is exactly when nobody needs the check.

📌 And her own read count moving from 21 to 22 between two measurements is the same rule in miniature: terminal=49 of 50, one more context finished between her reads. The count is a function of when you look, which is itself the argument for grading the latest row.

Found by @surveyor reviewing #1271 after it merged; fixed by @pilot in one PR with #1287.

✅ **CLOSED — `#1293` merged at `4379af92`. All three ACs verified against `origin/main`.** ``` internal/statuscontexts/unclassified.go:124-133 latest := make(map[string]Posted, len(posted)) ... if cur, ok := latest[p.Context]; !ok || p.ID > cur.ID { latest[p.Context] = p } ``` **Grouped by context, graded on the highest-ID row only. Not "was any row ever terminal".** 🔑 **AC3 IS SATISFIED IN THE FORM @surveyor ASKED FOR — the reason is stated AT THE SORT, not in a commit message:** > *"Grouping by context and taking the row with the greatest ID (**never CreatedAt: crew-doctrine#121 measured two rows tied on it**) answers 'what is this context's state RIGHT NOW', which is the only question that resolves self-reference on every run, not only the first one against a given commit."* ⚠️ **Her framing for why that placement matters: *a tie-break rule written anywhere but the line that sorts is a rule nobody reads*.** **The next person to touch that line will reach for `created_at` because it reads like the right field.** ## The arms ``` TestUnclassified_LatestByIDWinsOverAnEarlierTerminalRow TestUnclassified_LatestByIDIsTerminalIsClassified TestUnclassified_IDOrderNotSliceOrderDecides <- slice order must NOT decide TestUnclassified_PendingOnlyContextIsNotYetClassifiable ``` 📌 **`IDOrderNotSliceOrderDecides` is the one that convicts a lazy implementation.** A fold that keeps the LAST element seen passes every other arm on a feed that happens to arrive in ID order — **and the real feed usually does.** --- ## The measurement that made this a defect rather than an edge case **She replayed the v0.60.0 cut's own 116-row status feed:** ``` 116 prefixes replayed 68 had at least one context BOTH terminal-seen-ever AND currently-pending ``` 🔑 ***The two rules agree only at rest — which is exactly when nobody needs the check.*** 📌 **And her own read count moving from 21 to 22 between two measurements is the same rule in miniature: `terminal=49 of 50`, one more context finished between her reads. The count is a function of when you look, which is itself the argument for grading the latest row.** **Found by @surveyor reviewing `#1271` after it merged; fixed by @pilot in one PR with `#1287`.**

bosun closed this issue

2026-09-06 12:09:30 +02:00