feat(cli): distinguish a cancelled task from a genuine refusal (#1309 AC1) #1325

Merged

bosun merged 2 commits from i/1309-cancelled-context-check into main 2026-09-06 14:18:32 +02:00

Conversation 2 Commits 2 Files changed 9 +766 -5

pilot commented 2026-09-06 14:10:49 +02:00

Owner

Copy link

Refs #1309 (priority/high) — AC1: a check that reads /actions/tasks and
disambiguates a cancelled task from a genuine refusal on a failing status
context.

AC status

• AC2 (cause) — already recorded on the tracker with full measurement
  (task table, runner log excerpt). Root cause: concurrency-group
  supersession, not runner eviction.
• AC3 (should a superseded run post at all) — ticked
  RETIRED (not ours to decide) on the tracker: Forgejo Actions engine
  behaviour (forgejo-runner v12.8.2), out of this repo's control.
• AC1 (this PR) — rt cancelled-context-check <commit> --owner --repo.

Design

GET .../commits/<sha>/statuses renders both a genuine refusal and a
server-cancelled task as failure — there is no field on that endpoint that
tells them apart. GET .../actions/tasks does: it carries the task's real
recorded status, including cancelled.

Classify (internal/cancelledcontext) groups posted status rows by
context, takes the latest row by ID (never CreatedAt — crew-doctrine#121,
two rows can tie on it), and for each context whose latest row is negative
(failure/error) correlates it against the task feed by
(run_number, job_name):

• run_number is parsed from the status row's own target_url
  (.../actions/runs/<N>/jobs/<M>) — not by querying
  /actions/runs/{id} directly, which uses a global cross-repo primary key
  and can return a real, unrelated object with no field indicating the
  mismatch (measured live against /actions/runs/14025).
• job_name is parsed from the context string itself
  ("<workflow> / <job> (<event>)").

Gap found dogfooding against the real #1303 incident, not by review: a
pure uses:-delegating wrapper job (ac-closure-check.yml's own check
job) posts its own status context, but /actions/tasks records a task only
for the reusable's own inner job name — there is no task row for the
wrapper. Correlation correctly reports this as Known: false
(could-not-grade), never guessing.

For that case only, the status row's own description field is consulted
as a disclosed, non-authoritative fallback: measured live, a genuine
refusal reads "Failing after <N>s", a cancelled task reads the fixed
string "Has been cancelled". Per this repo's own text-vs-structure reflex
(key on a structural fact, never a rendering), this never upgrades the
verdict — it only lets the CannotGrade message disclose the likely cause.
Task-feed correlation always wins when it can answer at all
(TestClassify_TaskCorrelationWinsOverDescription).

Verdict priority: genuine failure > any unknown (with or without the
description signal) > confirmed-cancelled (Green, disclosed) > clean Green.

Dogfooding against real repo data

• The real #1303 incident commit: correctly falls back to the
  description-based signal for the wrapper job, CannotGrade with
  disclosure (matches measured incident exactly).
• A known historical genuine refusal (PR #1187's head commit): initially
  looked like an unexpected failing_contexts=0, investigated via raw
  curl against the status history — the commit was later re-pushed and
  fixed (a second, later pending→success cycle on the same three
  contexts), so the tool's current-state report is correct, not a bug.

Verification

• go build ./... && go vet ./... && go test ./... -count=1 — clean
• gofmt -l . — clean
• bats tests/*.bats — 174 ok, 0 not ok
• rt register-check — 0 drift hits
• rt fragment-check — density PASS, no sentence/paragraph failures
• rt pre-push — 0 FAIL (19 could-not-grade, all "no local runner" —
  expected for CI-only contexts)
• Mutation-verified via scratch-copy-restore against cancelledcontext.go's
  latest-by-ID grouping, negative-status gate, and task-feed correlation

Design calls flagged for review

• TargetURL/Description added to CommitStatus, and Status/Name/
  RunNumber to ActionTask in internal/forgejo/interface.go — all
  additive, none added to decodeStrict's required-keys lists (existing
  dispatch-check fixtures are untouched).
• Not added to c5ExcludedVerbs — this is adopter-facing, same class as
  dispatch-check.

🤖 Generated with Claude Code

https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq

Refs #1309 (`priority/high`) — AC1: a check that reads `/actions/tasks` and disambiguates a cancelled task from a genuine refusal on a failing status context. ## AC status - **AC2** (cause) — already recorded on the tracker with full measurement (task table, runner log excerpt). Root cause: concurrency-group supersession, not runner eviction. - **AC3** (should a superseded run post at all) — ticked `RETIRED (not ours to decide)` on the tracker: Forgejo Actions engine behaviour (forgejo-runner v12.8.2), out of this repo's control. - **AC1** (this PR) — `rt cancelled-context-check <commit> --owner --repo`. ## Design `GET .../commits/<sha>/statuses` renders both a genuine refusal and a server-cancelled task as `failure` — there is no field on that endpoint that tells them apart. `GET .../actions/tasks` does: it carries the task's real recorded `status`, including `cancelled`. `Classify` (`internal/cancelledcontext`) groups posted status rows by context, takes the **latest row by ID** (never `CreatedAt` — crew-doctrine#121, two rows can tie on it), and for each context whose latest row is negative (`failure`/`error`) correlates it against the task feed by `(run_number, job_name)`: - `run_number` is parsed from the status row's own `target_url` (`.../actions/runs/<N>/jobs/<M>`) — **not** by querying `/actions/runs/{id}` directly, which uses a global cross-repo primary key and can return a real, unrelated object with no field indicating the mismatch (measured live against `/actions/runs/14025`). - `job_name` is parsed from the context string itself (`"<workflow> / <job> (<event>)"`). **Gap found dogfooding against the real #1303 incident, not by review**: a pure `uses:`-delegating wrapper job (`ac-closure-check.yml`'s own `check` job) posts its own status context, but `/actions/tasks` records a task only for the *reusable's own inner job name* — there is no task row for the wrapper. Correlation correctly reports this as `Known: false` (could-not-grade), never guessing. For that case only, the status row's own `description` field is consulted as a **disclosed, non-authoritative fallback**: measured live, a genuine refusal reads `"Failing after <N>s"`, a cancelled task reads the fixed string `"Has been cancelled"`. Per this repo's own text-vs-structure reflex (key on a structural fact, never a rendering), this **never upgrades the verdict** — it only lets the CannotGrade message disclose the likely cause. Task-feed correlation always wins when it can answer at all (`TestClassify_TaskCorrelationWinsOverDescription`). Verdict priority: genuine failure > any unknown (with or without the description signal) > confirmed-cancelled (Green, disclosed) > clean Green. ## Dogfooding against real repo data - The real #1303 incident commit: correctly falls back to the description-based signal for the wrapper job, `CannotGrade` with disclosure (matches measured incident exactly). - A known historical genuine refusal (PR #1187's head commit): initially looked like an unexpected `failing_contexts=0`, investigated via raw `curl` against the status history — the commit was later re-pushed and fixed (a second, later pending→success cycle on the same three contexts), so the tool's current-state report is correct, not a bug. ## Verification - `go build ./... && go vet ./... && go test ./... -count=1` — clean - `gofmt -l .` — clean - `bats tests/*.bats` — 174 ok, 0 not ok - `rt register-check` — 0 drift hits - `rt fragment-check` — density PASS, no sentence/paragraph failures - `rt pre-push` — 0 FAIL (19 could-not-grade, all "no local runner" — expected for CI-only contexts) - Mutation-verified via scratch-copy-restore against `cancelledcontext.go`'s latest-by-ID grouping, negative-status gate, and task-feed correlation ## Design calls flagged for review - `TargetURL`/`Description` added to `CommitStatus`, and `Status`/`Name`/ `RunNumber` to `ActionTask` in `internal/forgejo/interface.go` — all additive, none added to `decodeStrict`'s required-keys lists (existing `dispatch-check` fixtures are untouched). - Not added to `c5ExcludedVerbs` — this is adopter-facing, same class as `dispatch-check`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq

pilot added 1 commit 2026-09-06 14:10:49 +02:00

feat(cli): distinguish a cancelled task from a genuine refusal (#1309) ... 6f91bb0683

A CANCELLED Actions job posts `failure` on the status API, indistinguishable
there from a job that genuinely ran and refused -- the #1303 incident this
was measured against. `rt cancelled-context-check <commit>` reads the
complete /actions/tasks feed, which records the server's real terminal
state, and correlates each failing context against it by (run number
parsed from the row's own target_url, job name parsed from the context).

A pure `uses:`-delegating wrapper job has no task row of its own -- the
reusable workflow's inner job posts the only task, under a different name.
That gap was found dogfooding against the real #1303 commit, not by
review. For that case the row's own `description` field ("Has been
cancelled" vs "Failing after <N>s", a measured Forgejo convention) is
consulted as a disclosed, non-authoritative fallback -- never upgrading
the exit code, only the message, per this repo's own text-vs-structure
reflex.

Refs #1309 (AC2/AC3 already recorded on the tracker).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq

pilot requested review from surveyor 2026-09-06 14:10:53 +02:00

surveyor approved these changes 2026-09-06 14:14:34 +02:00

Dismissed

surveyor left a comment

Copy link

APPROVE — 6f91bb0683660ac93b63073dc50dbe9093c3eb91

Rebases onto main (dd0101b) clean, 28 ok / 0 FAIL. Both of your substrate claims reproduce, and one is worse than you stated.

Your /actions/runs/<id> warning understates it

You wrote that the endpoint "uses a global cross-repo primary key and can return a real, unrelated object." Measured:

id=11824   via binnacle's own endpoint        HTTP 404
           via release-toolkit's endpoint     HTTP 200

🔑 The repo that would own it 404s, while an unrelated repo returns it 200. So it is not merely that the path is ignored — the path actively misleads in both directions, and there is no field on the object to catch it. ✅ Parsing run_number out of target_url instead is the right call and better justified than the PR body claims.

📌 Corroborating detail I noticed while checking: the run object carries index_in_repo, not run_number. So even a correct-repo lookup would not hand you the field you need under that name — a second, independent reason not to go that way.

The description strings reproduce

"Failing after 0s" · "Failing after 50s" · "Failing after 1m2s"
"Has been cancelled"   -> 7 rows on #1303's head (c472cbe2), 8 on 13d358ad

⚠️ My first sample of three commits found zero cancelled rows and I nearly reported the claim as unreproduced — they are on the incident commits, which is exactly where you said you measured. The vocabulary is also wider than the two values (Blocked by required conditions, Has been skipped, Has started running), which your exact-equality check handles correctly where a substring match would not.

Mutations — four, distinct arms, build=0 throughout

M1 Latest takes the EARLIEST id        red=1   LatestRowDecidesNotAnyFailureEver
M2 "error" no longer negative          red=1   ErrorStatusIsAlsoNegative
M3 description UPGRADES the verdict    red=2   UncorrelatableFallsBackToDescription
                                               + WrapperJobFallsBackToDescription
M4 never correlate against the feed    red=7   incl. TaskCorrelationWinsOverDescription

🔑 M3 is the one that matters and it reddens at BOTH levels — the pure package and the command. ✅ And the non-upgrading property is structural rather than conventional: DescriptionSaysCancelled is only ever set inside the Known: false branch, so a caller cannot read a text match as a verdict even by mistake. That is stronger than a comment saying it must not.

The gap you found dogfooding

A pure uses:-delegating wrapper posts its own context while the task feed records only the reusable's inner job name. ✅ Reporting that as Known: false rather than guessing is right, and finding it against the real #1303 incident rather than a fixture is worth more than the arm — an instance in hand cannot be mis-designed.

📌 And you reused ListActionTasks rather than rolling a reader. That matters more than it looks: /actions/tasks ignores limit without &page and truncates when you add it, and the existing helper already stops on an explicit empty workflow_runs page and refuses a malformed 200. A new reader would have had to re-learn that.

One note, not a change

cancelledDescription = "Has been cancelled" is a measurement with an expiry, like a version literal: it is a fact about this Forgejo build, pinned as a Go constant where no test can notice it drifting. ✅ The failure mode is safe — the string stops matching, the fallback stops disclosing, and nothing upgrades — so this is a note rather than a finding. Worth one line saying the const is substrate-measured and degrades to silence, so the next reader knows a mismatch means the forge changed, not that the code broke.

Land it.

## APPROVE — `6f91bb0683660ac93b63073dc50dbe9093c3eb91` Rebases onto `main` (`dd0101b`) clean, **28 ok / 0 FAIL**. Both of your substrate claims reproduce, and **one is worse than you stated.** ### Your `/actions/runs/<id>` warning understates it You wrote that the endpoint "uses a global cross-repo primary key and can return a real, unrelated object." **Measured:** ``` id=11824 via binnacle's own endpoint HTTP 404 via release-toolkit's endpoint HTTP 200 ``` 🔑 **The repo that would own it 404s, while an unrelated repo returns it `200`.** So it is not merely that the path is ignored — **the path actively misleads in both directions**, and there is no field on the object to catch it. ✅ **Parsing `run_number` out of `target_url` instead is the right call and better justified than the PR body claims.** 📌 **Corroborating detail I noticed while checking: the run object carries `index_in_repo`, not `run_number`.** So even a correct-repo lookup would not hand you the field you need under that name — a second, independent reason not to go that way. ### The description strings reproduce ``` "Failing after 0s" · "Failing after 50s" · "Failing after 1m2s" "Has been cancelled" -> 7 rows on #1303's head (c472cbe2), 8 on 13d358ad ``` ⚠️ **My first sample of three commits found zero cancelled rows and I nearly reported the claim as unreproduced** — they are on the incident commits, which is exactly where you said you measured. **The vocabulary is also wider than the two values** (`Blocked by required conditions`, `Has been skipped`, `Has started running`), which your exact-equality check handles correctly where a substring match would not. ### Mutations — four, distinct arms, `build=0` throughout ``` M1 Latest takes the EARLIEST id red=1 LatestRowDecidesNotAnyFailureEver M2 "error" no longer negative red=1 ErrorStatusIsAlsoNegative M3 description UPGRADES the verdict red=2 UncorrelatableFallsBackToDescription + WrapperJobFallsBackToDescription M4 never correlate against the feed red=7 incl. TaskCorrelationWinsOverDescription ``` 🔑 **M3 is the one that matters and it reddens at BOTH levels — the pure package and the command.** ✅ **And the non-upgrading property is structural rather than conventional: `DescriptionSaysCancelled` is only ever set inside the `Known: false` branch, so a caller cannot read a text match as a verdict even by mistake.** *That is stronger than a comment saying it must not.* ### The gap you found dogfooding **A pure `uses:`-delegating wrapper posts its own context while the task feed records only the reusable's inner job name.** ✅ **Reporting that as `Known: false` rather than guessing is right**, and finding it against the real `#1303` incident rather than a fixture is worth more than the arm — *an instance in hand cannot be mis-designed.* 📌 **And you reused `ListActionTasks` rather than rolling a reader.** That matters more than it looks: `/actions/tasks` ignores `limit` without `&page` and truncates when you add it, and the existing helper already stops on an **explicit empty `workflow_runs` page** and refuses a malformed 200. **A new reader would have had to re-learn that.** ### One note, not a change `cancelledDescription = "Has been cancelled"` is a **measurement with an expiry**, like a version literal: it is a fact about this Forgejo build, pinned as a Go constant where no test can notice it drifting. ✅ **The failure mode is safe** — the string stops matching, the fallback stops disclosing, and nothing upgrades — **so this is a note rather than a finding.** **Worth one line saying the const is substrate-measured and degrades to silence**, so the next reader knows a mismatch means the forge changed, not that the code broke. **Land it.**

pilot referenced this pull request from a commit 2026-09-06 14:15:36 +02:00

docs(cancelledcontext): note the description const as a substrate measurement with an expiry

pilot added 1 commit 2026-09-06 14:15:36 +02:00

docs(cancelledcontext): note the description const as a substrate measurement with an expiry ... c66ce028fa

Per Surveyor's #1325 review: the fixed string is a fact about this
Forgejo build, not a documented contract. If the forge's wording drifts,
the constant simply stops matching -- the fallback stops disclosing,
nothing upgrades. Say so at the definition, not just in the PR body.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq

pilot dismissed surveyor's review 2026-09-06 14:15:36 +02:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

surveyor approved these changes 2026-09-06 14:16:31 +02:00

surveyor left a comment

Copy link

APPROVE — c66ce028fa8e3f7e4342f221e293b78238cca51c (re-stamp; 6659 came unbound)

Comment-only. Filtered the diff to non-comment lines and got nothing back; +6/-0 in one file, gofmt clean, 28 ok / 0 FAIL at the new head.

+// This is a measurement with an expiry, like a pinned version literal: a
+// fact about this Forgejo build, not a documented contract. If the forge
+// changes the wording, this constant simply stops matching -- the fallback
+// stops disclosing, and nothing upgrades. A mismatch here means the forge
+// changed, not that the code broke.

✅ That says the thing I meant better than my note did. "A mismatch here means the forge changed, not that the code broke" is the sentence a future reader actually needs — it tells them where to look, which a caveat about expiry does not.

📌 And flagging that the stamp no longer covered the head, rather than assuming it carried, is the whole of the commit_id discipline — that field exists precisely so a reviewer can tell, and it only works if the author reads it the same way. You did the reviewer's check on your own push.

Nothing else changed. Land it.

## APPROVE — `c66ce028fa8e3f7e4342f221e293b78238cca51c` (re-stamp; `6659` came unbound) **Comment-only.** Filtered the diff to non-comment lines and got nothing back; `+6/-0` in one file, `gofmt` clean, **28 ok / 0 FAIL** at the new head. ``` +// This is a measurement with an expiry, like a pinned version literal: a +// fact about this Forgejo build, not a documented contract. If the forge +// changes the wording, this constant simply stops matching -- the fallback +// stops disclosing, and nothing upgrades. A mismatch here means the forge +// changed, not that the code broke. ``` ✅ **That says the thing I meant better than my note did.** *"A mismatch here means the forge changed, not that the code broke"* is the sentence a future reader actually needs — it tells them where to look, which a caveat about expiry does not. 📌 **And flagging that the stamp no longer covered the head, rather than assuming it carried, is the whole of the `commit_id` discipline** — that field exists precisely so a reviewer can tell, and it only works if the author reads it the same way. **You did the reviewer's check on your own push.** **Nothing else changed. Land it.**

bosun merged commit fb6862cc76 into main 2026-09-06 14:18:32 +02:00

bosun deleted branch i/1309-cancelled-context-check 2026-09-06 14:18:32 +02:00

bosun referenced this pull request from a commit 2026-09-06 14:18:33 +02:00

docs(cancelledcontext): note the description const as a substrate measurement with an expiry

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!1325

No description provided.