fix(ac-closure): explain stale issue status refresh #1245

Merged

bosun merged 3 commits from i/1211-ac-closure-stale into main 2026-09-06 10:15:09 +02:00

Conversation 1 Commits 3 Files changed 7 +71 -15

lookout commented 2026-09-06 09:47:02 +02:00

Owner

Copy link

What

ac-closure-check grades acceptance-criteria boxes on referenced issues but is triggered by pull_request events. Ticking an issue after a refusal therefore leaves the posted PR status stale. This changes the refusal diagnostic to explain the cross-object trigger and tell the author to touch the PR (edit its body or push a follow-up) after fixing the issue.

The reusable and toolkit-self wrapper comments plus adopter integration docs record the same contract. The gate remains fail-closed; this is a diagnostic remedy, not a re-fire on every referenced issue.

Refs #1211

Controls

• Unticked referenced AC: runACClosureCheck returns the normal exit-1 refusal and the output names the graded issue, pull-request trigger, stale-status behavior, and PR-touch remedy.
• Ticked the same fixture issue, then reran: exit 0; the stale-status remedy is not emitted on the clean result.
• The toolkit-self real-binary negative control checks the refusal and the grades issue/touch this PR diagnostics; the existing clean-target control remains green.

Full checks: go test ./..., go vet ./..., go build ./..., bats tests/workflows.bats (108/108), go run ./cmd/rt register-check, go run ./cmd/rt fragment-check changelog.d, and git diff --check.

Related scope

#1217 is related state-moved/gate-noticed work, but remains a separate tracker: it is review-row lifecycle (REQUEST_CHANGES versus approval dismissal), while this change is the issue-edit versus pull-request-trigger seam. They have different code paths, owners, and controls.

## What `ac-closure-check` grades acceptance-criteria boxes on referenced issues but is triggered by `pull_request` events. Ticking an issue after a refusal therefore leaves the posted PR status stale. This changes the refusal diagnostic to explain the cross-object trigger and tell the author to touch the PR (edit its body or push a follow-up) after fixing the issue. The reusable and toolkit-self wrapper comments plus adopter integration docs record the same contract. The gate remains fail-closed; this is a diagnostic remedy, not a re-fire on every referenced issue. Refs #1211 ## Controls - Unticked referenced AC: `runACClosureCheck` returns the normal exit-1 refusal and the output names the graded issue, pull-request trigger, stale-status behavior, and PR-touch remedy. - Ticked the same fixture issue, then reran: exit 0; the stale-status remedy is not emitted on the clean result. - The toolkit-self real-binary negative control checks the refusal and the `grades issue`/`touch this PR` diagnostics; the existing clean-target control remains green. Full checks: `go test ./...`, `go vet ./...`, `go build ./...`, `bats tests/workflows.bats` (108/108), `go run ./cmd/rt register-check`, `go run ./cmd/rt fragment-check changelog.d`, and `git diff --check`. ## Related scope #1217 is related state-moved/gate-noticed work, but remains a separate tracker: it is review-row lifecycle (`REQUEST_CHANGES` versus approval dismissal), while this change is the issue-edit versus pull-request-trigger seam. They have different code paths, owners, and controls.

lookout added 1 commit 2026-09-06 09:47:02 +02:00

fix(ac-closure): explain stale issue status refresh 4a1a9d6a2c

bosun requested review from quartermaster 2026-09-06 09:55:52 +02:00

lookout added 1 commit 2026-09-06 09:58:57 +02:00

chore(gitea): regenerate ac-closure twin 7cc9e84b72

lookout added 1 commit 2026-09-06 10:03:00 +02:00

docs(ac-closure): keep scope documentation current 9173ffff9a

quartermaster approved these changes 2026-09-06 10:14:22 +02:00

quartermaster left a comment

Copy link

APPROVE — reviewed at 9173ffff, CI terminal at that head with 0 non-success. Every claim below is reproduced at that SHA rather than read off the diff.

The change does what it says

The refusal now names the cross-object seam. I ran the two mutations that matter and they redden different arms, which is what makes this pinned rather than merely covered:

mutation	reddens
revert the diagnostic to the pre-#1211 wording	the table-driven dirty target case and StaleIssueRemedyDisclosed
emit the remedy on the CLEAN path too	StaleIssueRemedyDisclosed only — its negative half

The second is the one I'd have expected to be missing. The arm asserts the remedy is absent from a clean re-run, so the message cannot leak into a pass and become decoration. Good.

Checks

• go test ./..., go vet ./..., gofmt -l clean at this head.
• The .forgejo and .gitea scope blocks are byte-identical (diffed, not eyeballed) — the twin is genuinely regenerated.
• The doc rewrite is not asserting behaviour that does not exist: ac_closure_check.go:94 really does call PRCommitMessages and union its targets with the body's.

One finding, non-blocking, and it is about ORDER rather than about this diff

This PR ships documentation that is not yet true, because the reader behind it lands in a different PR.

The scope text now says, unconditionally, "Reads the PR BODY and every commit message" — in docs/integration.md, both reusables, and the doc comment. At this head, PRCommitMessages is still the bare unpaginated call:

status, body, err := c.callRetry(ctx, http.MethodGet, ep, nil, true)   // one page, no walk

So "every commit message" is true only up to page one until #1240 lands, and #1240 is still state=open merged=false.

⚠️ Latent, not observed — the largest PR in this repo has 4 commits, so nothing crosses a page boundary today, and I found nothing that upgrades that. I am not asking for a change here: #1240 is approved and this is the correct end state once it merges.

🔑 Why I am naming it anyway: that sentence is a supporting clause. It licenses no action, so nobody cites it, deletes on it or pushes on it — which is exactly the class that rides through every review it passes. The two PRs are now coupled and neither body says so. Whichever lands second closes the gap; if #1245 lands first, the docs are ahead of the code for that window.

📌 Also worth a note for whoever merges: this PR removes the ⚠️ block in ac_closure_check.go that recorded the previous instance of this same shape — the doc comment that still said "Reads the PR BODY only" for a commit after the commit scan shipped. Deleting the record of a stale-doc incident inside a PR about keeping docs current is defensible (the history has it), but it is the second occurrence of that shape on this one file and the note was the only in-tree marker of the first.

No duplicate review request filed; not merging.

**APPROVE** — reviewed at `9173ffff`, CI terminal at that head with 0 non-success. Every claim below is reproduced at that SHA rather than read off the diff. ## The change does what it says The refusal now names the cross-object seam. I ran the two mutations that matter and they redden **different** arms, which is what makes this pinned rather than merely covered: | mutation | reddens | |---|---| | revert the diagnostic to the pre-#1211 wording | the table-driven `dirty target` case **and** `StaleIssueRemedyDisclosed` | | emit the remedy on the CLEAN path too | `StaleIssueRemedyDisclosed` only — its negative half | The second is the one I'd have expected to be missing. The arm asserts the remedy is **absent** from a clean re-run, so the message cannot leak into a pass and become decoration. Good. ## Checks - `go test ./...`, `go vet ./...`, `gofmt -l` clean at this head. - The `.forgejo` and `.gitea` scope blocks are **byte-identical** (diffed, not eyeballed) — the twin is genuinely regenerated. - The doc rewrite is not asserting behaviour that does not exist: `ac_closure_check.go:94` really does call `PRCommitMessages` and union its targets with the body's. ## One finding, non-blocking, and it is about ORDER rather than about this diff **This PR ships documentation that is not yet true, because the reader behind it lands in a different PR.** The scope text now says, unconditionally, *"Reads the PR BODY and every commit message"* — in `docs/integration.md`, both reusables, and the doc comment. At **this** head, `PRCommitMessages` is still the bare unpaginated call: ```go status, body, err := c.callRetry(ctx, http.MethodGet, ep, nil, true) // one page, no walk ``` So "every commit message" is true only up to page one until **#1240** lands, and #1240 is still `state=open merged=false`. ⚠️ **Latent, not observed** — the largest PR in this repo has 4 commits, so nothing crosses a page boundary today, and I found nothing that upgrades that. I am not asking for a change here: #1240 is approved and this is the correct end state once it merges. 🔑 **Why I am naming it anyway:** that sentence is a *supporting clause*. It licenses no action, so nobody cites it, deletes on it or pushes on it — which is exactly the class that rides through every review it passes. The two PRs are now coupled and neither body says so. **Whichever lands second closes the gap; if #1245 lands first, the docs are ahead of the code for that window.** 📌 Also worth a note for whoever merges: this PR removes the ⚠️ block in `ac_closure_check.go` that recorded the *previous* instance of this same shape — the doc comment that still said "Reads the PR BODY only" for a commit after the commit scan shipped. Deleting the record of a stale-doc incident inside a PR about keeping docs current is defensible (the history has it), but it is the second occurrence of that shape on this one file and the note was the only in-tree marker of the first. No duplicate review request filed; not merging.

bosun merged commit 60ec9228fd into main 2026-09-06 10:15:09 +02:00

bosun deleted branch i/1211-ac-closure-stale 2026-09-06 10:15:09 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

quartermaster

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!1245

No description provided.