fix(ac-closure): grade closing references in PR comments, inline review comments and the title (#1506) #1560

Merged

alex merged 2 commits from claude/1506-ac-closure-forge-xrefs into main 2026-10-06 11:06:48 +02:00

Conversation 6 Commits 2 Files changed 14 +1179 -42

claude commented 2026-10-06 01:31:21 +02:00

Member

• Copy link
• Copy Markdown

Refs #1506

rt ac-closure-check graded closing keywords in the PR body and the commit messages only. Forgejo also closes issues on merge from PR comments, inline review comments, the PR title, and a qualified owner/repo#N that names the same repository. #1506 has the full measurement. The short version:

Surface	Forgejo records	Closed on merge	Main's gate	This PR
PR comment, plain keyword	comment_ref closes	yes	missed	refused
PR comment, keyword in inline code / fenced block	no row	no	—	not a target
inline review comment	comment_ref closes	yes	missed	refused
review body	no row	no	—	not a target
PR comment deleted / edited to Refs	null row / neutered	no	—	not a target
PR title fix #N	pull_ref closes	yes	missed	refused
body or comment, owner/this-repo#N	closes	yes	missed (and the remedy recommended it)	refused
comment, owner/other-repo#N	closes on the other repo	yes, there	not graded	not graded, disclosed

Replayed live, read-only. main's binary on PR #1489 exits 0. This head exits 1 with FAIL #1485 … comment 113711. On the merged probe PR, the refused set is {1, 4, 8, 9, 11}, exactly the issues that closed. The untouched set is {2, 3, 5, 6, 7, 10}, exactly the issues that stayed open.

How it works

The keyword scans of the body and commits are unchanged. A new forge arm runs after them and can only add targets:

1. Every same-repo number mentioned in the title, the body, the PR comments and the inline review comments becomes a candidate. This needs no keyword list, so it does not depend on an instance's CLOSE_KEYWORDS.
2. For each candidate, the gate reads that issue's timeline from the PR's created_at onward. It is a target when its latest closes/reopens row from this PR, in this repo is closes. That is ResolveCrossReferences (v15.0.7), which the merge applies.
3. A forge-found target goes through the same Intended-targets: and unticked-box checks. The refusal names the comment by id and URL, and the remedy fits the surface:
   • a comment: edit or delete it, or wrap the keyword in inline code;
   • the title or body: edit it. Inline code is not offered there, because the title is parsed as plain text.

PR comments are read through ListIssueComments, which #1496 fixed. Cost on the three most-commented recent PRs was 22, 27 and 34 API calls, each under 1 s.

Decisions

Input surface

• PR comments API plus the keyword regex is right when one parser across all surfaces matters more than false refusals.
  • Here it would refuse a keyword quoted in inline code. That is the form this crew uses to quote one safely, and Forgejo records nothing for it.
  • It would also miss inline review comments, which that route does not return.
• The target issues' timelines are right when the question is "what will this merge close", because that is the record the merge reads.
  • The price is candidate discovery plus one read per candidate.
  • There is one documented hazard, covered next. Chosen.

Timeline termination

• Measured with limit=1: Forgejo pages over its stored rows and only then drops the ones it hides (inline code comments, references from repos the token cannot read). A page holding only hidden rows renders null, exactly like the end of the list, and rows can still follow it.
• No total count is exposed: X-Total-Count is the size of the page.
• So the PR's comments are not read from its timeline. They come from the comments route instead.
• Candidate timelines are bounded by since=<PR created_at>. Measured: since keys on updated_at, and the body's own reference row has the PR's creation second.
• On an issue, the only rows that can be hidden are cross-repo references. Ending a walk early therefore needs a full page of unreadable cross-repo references inside the PR's lifetime. This residue is documented at the reader, not papered over.

null rows

• Could-not-grade would be right if a null row could hide a live closes row from this PR.
  • The one measured producer is a deleted comment's reference. Forgejo neuters it on delete (source), and probe issue 6 stayed open (measured).
  • Treating it as fatal would make every later PR that mentions that issue ungradeable, forever.
• Skipped and counted in the output. Chosen.

Unpaginated per-review comments route

/pulls/{n}/reviews/{id}/comments ignores page and limit, as /issues/{n}/comments does (#1495). It is read in one request, the shape #1496 gave ListIssueComments.

Trigger: none added

• pull_request only (this PR) is right when a pull-request event follows every closing comment. The review loop does that under HOLD: a comment, then a fix commit, then synchronize. That case is now caught.
• issue_comment is right only if its verdict reaches the PR's check. On v15.0.7, services/actions/commit_status.go creates no status for comment-event runs, so a red run would be invisible to the merge. This is a source read at the tag, not a live probe: no runner serves the throwaway owner. Not added.
• A merge-time re-grade is right when the merge runs through a tool we own. No rt verb merges ordinary PRs, and Forgejo has no pre-merge hook.
  • So the gate now says on every run: run rt ac-closure-check again immediately before merging.
  • That is exactly the #1489 timing: comment 11:07:47, last PR event 11:07:02, merge 11:11:59.
  • Real enforcement is a follow-up with two shapes:
    • (a) a comment-triggered run that writes a commit status on the PR head under this check's context;
    • (b) a push:main audit that reopens an issue closed with open boxes, which is repair rather than prevention.

Also in this PR

• Toolkit-self fixture. tests/fixtures/ac_closure_check_fixture.py now serves the routes the gate reads, the way Forgejo does: comments routes ignore paging, and the timeline ends in null. Without that, controls 7 and 8 would go could-not-grade. A third control, PR 9, is the #1489 shape: body Refs #44, a plain-text comment closing it, ticked ACs.
  • main's rt passes PR 9 (exit 0); this rt refuses it (exit 1).
  • tests/workflows.bats pins the new control.
• AGENTS.md section 15a: the Graded column says yes for the title, PR comments and inline review comments, and review bodies stay at no. The section also names the after-the-last-event timing gap and drops "the gate's current refusal text suggests otherwise".
• Every exit-2 path states its reason. The verb's SilenceErrors used to hide it, so CI showed a bare exit 2.
• The unfinished-AC remedy said to close the issue by hand using the keyword-plus-number form. It no longer contains a closing reference of its own.
• Scope text is updated in docs/integration.md (the ac_closure gate bullet), the closeKeywordRE comment and the verb's doc comment.

Section 2 re-pin

No file in gates.DefaultComposeScripts changed: reusable-pr-gates.yml, pr-gates.sh and their peers are untouched. Only Go code, the toolkit-self workflow, the fixture and docs changed, so check-self-bootstrap should stay green. There is no CLI surface change (section 13): no new verb or flag, and help.txt is untouched.

Tests

• New and updated tests in cmd/rt, internal/gates and internal/forgejo. All 30 packages pass in golang:1.25 on an LF clone of 11aba7f.
• gofmt, go vet, gitea-twin --check, the fragment density check, and the two ac-closure tests in tests/workflows.bats all pass.
• The CI fixture flow was run locally against the built binary.
• Mutation-verified: 12 of 12 killed.

Mutation	Killed by
forge arm removed	the #1489 replay, inline review comment, title/qualified
inline review comments not used as candidates	inline review comment
repo not compared / first row wins	TestForgeCloseRef
per-review route walked by the paginator	the read-once test
null row decoded / null page treated as malformed	timeline reader
remedy reverts to the old keyword-plus-number wording	remedy-is-not-a-closing-reference
404 candidate made fatal	read failures
title/body not used as candidates	title/qualified
since dropped	the #1489 replay
comment control's success line removed from the workflow	tests/workflows.bats

Acceptance criteria → evidence

Each #1506 box is listed here so the reviewer can tick it:

#	AC	Where
1	forge arm, union with the keyword arms	readForgeArm, gates.ForgeCloseRef, TestForgeCloseRef
2	inline code / fenced block in a comment not a target	…InlineCodeInACommentIsNotATarget (regex precondition asserted)
3	#1489 shape refused, names comment + remedy	…CommentReferenceIsATarget; live replay; fixture PR 9
4	null rows skipped and counted	…NullRowsAreDisclosedNotFatal, TestListIssueTimeline_NullPage…
5	inline review comments, one GET per review	TestListPRReviewComments_UnpaginatedRouteIsReadOnce…
6	disclosure: new surfaces and what is not graded	TestRunACClosureCheck_ScopeDisclosed
7	refusal no longer offers a qualified ref as non-closing	…RemedyTextIsNotItselfAClosingReference
8	tests per surface; #1489 arm mutation-verified	mutation table
9	reusable workflow headers; no trigger change; decision tree	superseded: 0de3876 removed reusable-ac-closure-check.yml. The scope text moved to docs/integration.md's ac_closure bullet; reusable-pr-gates.yml's one-line description stays true. No trigger change; decision tree above
10	changelog.d fragment	changelog.d/1506-ac-closure-forge-xrefs.fixed.md
11	AGENTS.md 15a Graded column	flipped in this PR (#1498 is on main)

Verify at source (section 6)

@alex, please verify these at source rather than taking them from this body:

1. Build this head and run rt ac-closure-check --owner frankenbit --repo release-toolkit --pr 1489. Expect exit 1 and FAIL #1485 … comment 113711. Then run main's binary and expect exit 0.
2. Read GET /repos/frankenbit/release-toolkit/issues/1485/timeline yourself: the comment_ref closes row for 113711, and the none row for 113708, whose keyword is inside inline code.
3. Spot-check the v15.0.7 source citations: models/issues/issue_xref.go (AddCrossReferences, ResolveCrossReferences) and services/actions/commit_status.go.

Not established here:

• A non-writer's comment downgraded to none (source only).
• Another user's pending review comments, which the token may not see.
• The issue_comment status claim, which is a source read rather than a live probe.

🤖 Generated with Claude Code

Refs #1506 `rt ac-closure-check` graded closing keywords in the PR body and the commit messages only. Forgejo also closes issues on merge from **PR comments**, **inline review comments**, the **PR title**, and a **qualified `owner/repo#N` that names the same repository**. #1506 has the full measurement. The short version: | Surface | Forgejo records | Closed on merge | Main's gate | This PR | |---|---|---|---|---| | PR comment, plain keyword | `comment_ref` `closes` | yes | missed | **refused** | | PR comment, keyword in inline code / fenced block | no row | no | — | not a target | | inline review comment | `comment_ref` `closes` | yes | missed | **refused** | | review body | no row | no | — | not a target | | PR comment deleted / edited to `Refs` | `null` row / `neutered` | no | — | not a target | | PR title `fix #N` | `pull_ref` `closes` | yes | missed | **refused** | | body or comment, `owner/this-repo#N` | `closes` | yes | missed (and the remedy *recommended* it) | **refused** | | comment, `owner/other-repo#N` | `closes` on the other repo | yes, there | not graded | not graded, disclosed | **Replayed live, read-only.** `main`'s binary on PR #1489 exits **0**. This head exits **1** with `FAIL #1485 … comment 113711`. On the merged probe PR, the refused set is `{1, 4, 8, 9, 11}`, exactly the issues that closed. The untouched set is `{2, 3, 5, 6, 7, 10}`, exactly the issues that stayed open. ## How it works The keyword scans of the body and commits are unchanged. A new **forge arm** runs after them and can only add targets: 1. Every same-repo number mentioned in the title, the body, the PR comments and the inline review comments becomes a candidate. This needs no keyword list, so it does not depend on an instance's `CLOSE_KEYWORDS`. 2. For each candidate, the gate reads that issue's timeline from the PR's `created_at` onward. It is a target when its latest `closes`/`reopens` row **from this PR, in this repo** is `closes`. That is `ResolveCrossReferences` (v15.0.7), which the merge applies. 3. A forge-found target goes through the same `Intended-targets:` and unticked-box checks. The refusal names the comment by id and URL, and the remedy fits the surface: - a comment: edit or delete it, or wrap the keyword in inline code; - the title or body: edit it. Inline code is not offered there, because the title is parsed as plain text. PR comments are read through `ListIssueComments`, which #1496 fixed. Cost on the three most-commented recent PRs was 22, 27 and 34 API calls, each under 1 s. ## Decisions ### Input surface - **PR comments API plus the keyword regex** is right when one parser across all surfaces matters more than false refusals. - Here it would refuse a keyword quoted in inline code. That is the form this crew uses to quote one safely, and Forgejo records nothing for it. - It would also miss inline review comments, which that route does not return. - **The target issues' timelines** are right when the question is "what will this merge close", because that is the record the merge reads. - The price is candidate discovery plus one read per candidate. - There is one documented hazard, covered next. **Chosen.** ### Timeline termination - **Measured with `limit=1`**: Forgejo pages over its stored rows and only then drops the ones it hides (inline code comments, references from repos the token cannot read). A page holding only hidden rows renders `null`, exactly like the end of the list, and rows can still follow it. - No total count is exposed: `X-Total-Count` is the size of the page. - So the PR's comments are **not** read from its timeline. They come from the comments route instead. - Candidate timelines are bounded by `since=<PR created_at>`. Measured: `since` keys on `updated_at`, and the body's own reference row has the PR's creation second. - On an issue, the only rows that can be hidden are cross-repo references. Ending a walk early therefore needs a full page of unreadable cross-repo references inside the PR's lifetime. This residue is documented at the reader, not papered over. ### `null` rows - **Could-not-grade** would be right if a `null` row could hide a live `closes` row from this PR. - The one measured producer is a deleted comment's reference. Forgejo neuters it on delete (source), and probe issue 6 stayed open (measured). - Treating it as fatal would make every later PR that mentions that issue ungradeable, forever. - **Skipped and counted in the output.** Chosen. ### Unpaginated per-review comments route `/pulls/{n}/reviews/{id}/comments` ignores `page` and `limit`, as `/issues/{n}/comments` does (#1495). It is read in one request, the shape #1496 gave `ListIssueComments`. ### Trigger: none added - **`pull_request` only** (this PR) is right when a pull-request event follows every closing comment. The review loop does that under HOLD: a comment, then a fix commit, then `synchronize`. **That case is now caught.** - **`issue_comment`** is right only if its verdict reaches the PR's check. On v15.0.7, `services/actions/commit_status.go` creates no status for comment-event runs, so a red run would be invisible to the merge. This is a source read at the tag, not a live probe: no runner serves the throwaway owner. **Not added.** - **A merge-time re-grade** is right when the merge runs through a tool we own. No `rt` verb merges ordinary PRs, and Forgejo has no pre-merge hook. - So the gate now says on every run: *run `rt ac-closure-check` again immediately before merging.* - That is exactly the #1489 timing: comment 11:07:47, last PR event 11:07:02, merge 11:11:59. - Real enforcement is a follow-up with two shapes: - (a) a comment-triggered run that writes a commit status on the PR head under this check's context; - (b) a `push:main` audit that reopens an issue closed with open boxes, which is repair rather than prevention. ## Also in this PR - **Toolkit-self fixture.** `tests/fixtures/ac_closure_check_fixture.py` now serves the routes the gate reads, the way Forgejo does: comments routes ignore paging, and the timeline ends in `null`. Without that, controls 7 and 8 would go could-not-grade. A third control, PR 9, is the #1489 shape: body `Refs #44`, a plain-text comment closing it, ticked ACs. - `main`'s `rt` passes PR 9 (exit 0); this `rt` refuses it (exit 1). - `tests/workflows.bats` pins the new control. - **AGENTS.md section 15a**: the Graded column says yes for the title, PR comments and inline review comments, and review bodies stay at no. The section also names the after-the-last-event timing gap and drops "the gate's current refusal text suggests otherwise". - Every exit-2 path states its reason. The verb's `SilenceErrors` used to hide it, so CI showed a bare exit 2. - The unfinished-AC remedy said to close the issue by hand using the keyword-plus-number form. It no longer contains a closing reference of its own. - Scope text is updated in `docs/integration.md` (the `ac_closure` gate bullet), the `closeKeywordRE` comment and the verb's doc comment. ## Section 2 re-pin No file in `gates.DefaultComposeScripts` changed: `reusable-pr-gates.yml`, `pr-gates.sh` and their peers are untouched. Only Go code, the toolkit-self workflow, the fixture and docs changed, so `check-self-bootstrap` should stay green. There is no CLI surface change (section 13): no new verb or flag, and `help.txt` is untouched. ## Tests - New and updated tests in `cmd/rt`, `internal/gates` and `internal/forgejo`. All 30 packages pass in `golang:1.25` on an LF clone of `11aba7f`. - `gofmt`, `go vet`, `gitea-twin --check`, the fragment density check, and the two ac-closure tests in `tests/workflows.bats` all pass. - The CI fixture flow was run locally against the built binary. - **Mutation-verified: 12 of 12 killed.** | Mutation | Killed by | |---|---| | forge arm removed | the #1489 replay, inline review comment, title/qualified | | inline review comments not used as candidates | inline review comment | | repo not compared / first row wins | `TestForgeCloseRef` | | per-review route walked by the paginator | the read-once test | | `null` row decoded / `null` page treated as malformed | timeline reader | | remedy reverts to the old keyword-plus-number wording | remedy-is-not-a-closing-reference | | 404 candidate made fatal | read failures | | title/body not used as candidates | title/qualified | | `since` dropped | the #1489 replay | | comment control's success line removed from the workflow | `tests/workflows.bats` | ## Acceptance criteria → evidence Each #1506 box is listed here so the reviewer can tick it: | # | AC | Where | |---|---|---| | 1 | forge arm, union with the keyword arms | `readForgeArm`, `gates.ForgeCloseRef`, `TestForgeCloseRef` | | 2 | inline code / fenced block in a comment not a target | `…InlineCodeInACommentIsNotATarget` (regex precondition asserted) | | 3 | #1489 shape refused, names comment + remedy | `…CommentReferenceIsATarget`; live replay; fixture PR 9 | | 4 | `null` rows skipped and counted | `…NullRowsAreDisclosedNotFatal`, `TestListIssueTimeline_NullPage…` | | 5 | inline review comments, one GET per review | `TestListPRReviewComments_UnpaginatedRouteIsReadOnce…` | | 6 | disclosure: new surfaces and what is not graded | `TestRunACClosureCheck_ScopeDisclosed` | | 7 | refusal no longer offers a qualified ref as non-closing | `…RemedyTextIsNotItselfAClosingReference` | | 8 | tests per surface; #1489 arm mutation-verified | mutation table | | 9 | reusable workflow headers; no trigger change; decision tree | **superseded**: `0de3876` removed `reusable-ac-closure-check.yml`. The scope text moved to `docs/integration.md`'s `ac_closure` bullet; `reusable-pr-gates.yml`'s one-line description stays true. No trigger change; decision tree above | | 10 | `changelog.d` fragment | `changelog.d/1506-ac-closure-forge-xrefs.fixed.md` | | 11 | AGENTS.md 15a Graded column | flipped in this PR (#1498 is on `main`) | ## Verify at source (section 6) @alex, please verify these at source rather than taking them from this body: 1. Build this head and run `rt ac-closure-check --owner frankenbit --repo release-toolkit --pr 1489`. Expect exit 1 and `FAIL #1485 … comment 113711`. Then run `main`'s binary and expect exit 0. 2. Read `GET /repos/frankenbit/release-toolkit/issues/1485/timeline` yourself: the `comment_ref` `closes` row for 113711, and the `none` row for 113708, whose keyword is inside inline code. 3. Spot-check the v15.0.7 source citations: `models/issues/issue_xref.go` (`AddCrossReferences`, `ResolveCrossReferences`) and `services/actions/commit_status.go`. Not established here: - A non-writer's comment downgraded to `none` (source only). - Another user's pending review comments, which the token may not see. - The `issue_comment` status claim, which is a source read rather than a live probe. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude added 1 commit 2026-10-06 01:31:21 +02:00

fix(ac-closure): grade the forge's own closing references from comments, review comments and the title (#1506) ... 9c7601f15f

Forgejo closes an issue on merge from a PR comment, an inline review comment,
the PR title, and a qualified owner/repo#N that names the same repository.
The gate scanned only the body and the commit messages. Measured on throwaway
repos and replayed on PR 1489, where a plain-text comment closed issue 1485
behind a green check.

The new forge arm collects every same-repo number mentioned on the PR and
reads that issue's timeline, bounded to the PR's lifetime. A target is an
issue whose latest closes/reopens row from this PR is closes. This mirrors
ResolveCrossReferences, so a keyword quoted in inline code (Forgejo skips it)
is not a target, and an edited or deleted comment lifts the close. The keyword
scans are unchanged, and the result is the union of the arms.

New readers:
- ListIssueTimeline(since). Its empty page is the literal null; a row
  Forgejo cannot load is null inside the array, and those rows are counted,
  not decoded. An all-hidden page reads as the end (measured with limit=1),
  which is documented at the reader.
- ListPRReviewComments reads the unpaginated per-review route once, the same
  shape as ListIssueComments (#1495). It decodes only review ids, so a team
  review request (user: null) does not refuse.

The toolkit-self fixture serves the new routes the way Forgejo does: the
comments routes ignore paging, and the timeline ends in null. It also adds a
real-binary control for the #1489 shape (PR 9), which main's rt passes and
this rt refuses.

AGENTS.md section 15a's Graded column now says yes for the title, PR comments
and inline review comments.

Also:
- The disclosure states what is still not graded: other repos, and comments
  posted after the run.
- The refusal stops calling a qualified reference non-closing.
- The unfinished-AC remedy no longer contains a closing reference of its own.
- Every exit-2 path now states its reason; the verb's SilenceErrors used to
  hide it.

No trigger change. A run triggered by issue_comment posts no commit status on
15.0.7 (services/actions/commit_status.go), so it could not refresh this check.

Refs #1506

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude requested review from alex 2026-10-06 01:31:34 +02:00

claude reviewed 2026-10-06 09:16:27 +02:00

claude left a comment

• Copy link
• Copy Markdown

Verdict: APPROVE at 9c7601f15f2ea8fd7dffff139f5fb0c461a197ea. One AC6 gap needs a disposition before AC6 is ticked (below). This is posted as COMMENT because the forge identity is claude, the PR's author, so @alex has to click Approve.

Refs #1506

Verified at source

1. The #1489 replay, read-only, with both binaries built natively:
   • main (11aba7f) exits 0.
   • This head exits 1 with FAIL #1485 ... comment 113711 and the comment remedy.
2. The #1485 timeline, read directly:
   • 113712 comment_ref closes, from PR 1489, comment 113711.
   • 113709 comment_ref none, from comment 113708 (the inline-code one).
   • 113617 pull_ref neutered.
   • Every other row is none.
3. The v15.0.7 source, read from Codeberg at the tag:
   • ResolveCrossReferences (issue_xref.go:337) selects closes/reopens rows for this PR's repo and issue id, ordered by id, and keeps the latest per target. ForgeCloseRef matches that.
   • createCommitStatus (commit_status.go) switches on push, the pull_request family and release, and its default: returns nil. So an issue_comment run posts no status, as the body says.

Regression sweep

I ran both binaries on the 50 most recently updated PRs in this repo:

• Only #1489 changes verdict. PR 754 is exit 1 on both.
• None goes could-not-grade. Each run took 1 s or less.
• This PR passes its own gate.

The Actions token can read the new routes

In run 19953 (job 1, step ac_closure), this head's binary ran with the workflow token. It logged asked the FORGE's own timeline of 7 same-repo number(s) and finished with no target. Adopters run this gate with the same kind of token, so this was the risk I most wanted to rule out.

Tests and mutants

• cmd/rt, internal/gates and internal/forgejo pass in golang:1.25 on an LF clone.
• I wrote four mutants of my own. All are killed:

Mutant	Killed by
addAll(arm.order) removed	...CommentReferenceIsATarget, ...InlineReviewCommentIsRead, ...TitleAndQualifiedBodyAreTargets
repository not compared	TestForgeCloseRef
earliest row wins	TestForgeCloseRef
since dropped	...CommentReferenceIsATarget

• CI: all 18 statuses on the head are success.

AC6 gap (needs a disposition)

AC6 asks every run to disclose that review bodies are not graded (measured inert). The run output does not say this. The NOT-checked line names other repositories and later comments only, and TestRunACClosureCheck_ScopeDisclosed does not ask for it. docs/integration.md does say it ("Review bodies do not close anything"). There are two ways to close the gap:

• add one clause to the NOT-checked line, plus the matching want string;
• or record AC6's review-body clause as retired, with the reason (inert, so there is nothing to grade).

Nit, non-blocking

decodeTimelineEvent strictly requires ref_action and ref_issue.number, but not ref_issue.repository.full_name. If that field were ever missing, ForgeCloseRef's EqualFold would read the row as "from another PR", which is the silent direction. The reader's own comment rules that direction out. Requiring the field there would close it.

Not reproduced by me

• The {1, 4, 8, 9, 11} probe-PR replay: claude/ac-xref-probe-a-20261002 and -b- are gone (404, as section 14 asks), so I graded the live replay above instead.
• A non-writer's downgrade to none, and other users' pending review comments. The body also lists both as not established.

Verdict: APPROVE at `9c7601f15f2ea8fd7dffff139f5fb0c461a197ea`. One AC6 gap needs a disposition before AC6 is ticked (below). This is posted as COMMENT because the forge identity is `claude`, the PR's author, so @alex has to click Approve. Refs #1506 ## Verified at source 1. **The #1489 replay**, read-only, with both binaries built natively: - `main` (`11aba7f`) exits **0**. - This head exits **1** with `FAIL #1485 ... comment 113711` and the comment remedy. 2. **The #1485 timeline**, read directly: - `113712 comment_ref closes`, from PR 1489, comment 113711. - `113709 comment_ref none`, from comment 113708 (the inline-code one). - `113617 pull_ref neutered`. - Every other row is `none`. 3. **The v15.0.7 source**, read from Codeberg at the tag: - `ResolveCrossReferences` (issue_xref.go:337) selects `closes`/`reopens` rows for this PR's repo and issue id, ordered by id, and keeps the latest per target. `ForgeCloseRef` matches that. - `createCommitStatus` (commit_status.go) switches on push, the pull_request family and release, and its `default:` returns nil. So an `issue_comment` run posts no status, as the body says. ## Regression sweep I ran both binaries on the **50 most recently updated PRs** in this repo: - **Only #1489 changes verdict.** PR 754 is exit 1 on both. - **None goes could-not-grade.** Each run took 1 s or less. - This PR passes its own gate. ## The Actions token can read the new routes In run 19953 (job 1, step `ac_closure`), this head's binary ran with the workflow token. It logged `asked the FORGE's own timeline of 7 same-repo number(s)` and finished with no target. Adopters run this gate with the same kind of token, so this was the risk I most wanted to rule out. ## Tests and mutants - `cmd/rt`, `internal/gates` and `internal/forgejo` pass in `golang:1.25` on an LF clone. - I wrote four mutants of my own. All are killed: | Mutant | Killed by | |---|---| | `addAll(arm.order)` removed | `...CommentReferenceIsATarget`, `...InlineReviewCommentIsRead`, `...TitleAndQualifiedBodyAreTargets` | | repository not compared | `TestForgeCloseRef` | | earliest row wins | `TestForgeCloseRef` | | `since` dropped | `...CommentReferenceIsATarget` | - CI: all 18 statuses on the head are success. ## AC6 gap (needs a disposition) AC6 asks every run to disclose that **review bodies** are not graded (measured inert). The run output does not say this. The NOT-checked line names other repositories and later comments only, and `TestRunACClosureCheck_ScopeDisclosed` does not ask for it. `docs/integration.md` does say it ("Review bodies do not close anything"). There are two ways to close the gap: - add one clause to the NOT-checked line, plus the matching `want` string; - or record AC6's review-body clause as retired, with the reason (inert, so there is nothing to grade). ## Nit, non-blocking `decodeTimelineEvent` strictly requires `ref_action` and `ref_issue.number`, but not `ref_issue.repository.full_name`. If that field were ever missing, `ForgeCloseRef`'s `EqualFold` would read the row as "from another PR", which is the silent direction. The reader's own comment rules that direction out. Requiring the field there would close it. ## Not reproduced by me - The `{1, 4, 8, 9, 11}` probe-PR replay: `claude/ac-xref-probe-a-20261002` and `-b-` are gone (404, as section 14 asks), so I graded the live replay above instead. - A non-writer's downgrade to `none`, and other users' pending review comments. The body also lists both as not established.

claude commented 2026-10-06 09:17:09 +02:00

Author

Member

• Copy link
• Copy Markdown

HOLD: I am pushing one review-fix commit for review 7272 on top of 9c7601f. It covers the two points raised there:

• AC6: every run will also disclose that review bodies are not graded, pinned by a want string in TestRunACClosureCheck_ScopeDisclosed.
• Nit: a reference row with no ref_issue.repository.full_name will be refused as a malformed response instead of being treated as another PR's row.

Please hold the approval read until I post the new full head SHA here.

HOLD: I am pushing one review-fix commit for review 7272 on top of `9c7601f`. It covers the two points raised there: - AC6: every run will also disclose that review bodies are not graded, pinned by a want string in `TestRunACClosureCheck_ScopeDisclosed`. - Nit: a reference row with no `ref_issue.repository.full_name` will be refused as a malformed response instead of being treated as another PR's row. Please hold the approval read until I post the new full head SHA here.

claude added 1 commit 2026-10-06 09:20:24 +02:00

fix(ac-closure): disclose review bodies and refuse a reference row without its repository (#1506) ... 6a360903dc

Review 7272 on PR 1560 found two problems.

- AC6 requires every run to disclose that review bodies are not graded. Only
  docs/integration.md said so. The NOT-checked line now names them, and
  TestRunACClosureCheck_ScopeDisclosed pins it.
- decodeTimelineEvent did not require ref_issue.repository.full_name. Without
  it, ForgeCloseRef would read this PR's closing row as a same-numbered PR in
  another repo, which fails open. It is now ErrUnexpectedResponse.

Each fix was mutation-checked: removing it turns its test red. A live sweep
of 22 PRs found no exit 2, and only PR 1489 is refused.

Refs #1506

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude commented 2026-10-06 09:20:33 +02:00

Author

Member

• Copy link
• Copy Markdown

HOLD lifted. The new head is 6a360903dc559026d5aa9a28282dfc513778c71f: one commit on top of 9c7601f, addressing review 7272.

• AC6: the NOT-checked line now also says that review bodies are not read, because Forgejo records no closing reference from them. TestRunACClosureCheck_ScopeDisclosed pins it.
• Nit: a reference row without ref_issue.repository.full_name is now ErrUnexpectedResponse, so it can no longer be read as a same-numbered PR in another repo. There are two new cases in TestListIssueTimeline_MalformedRefusesRatherThanReadingAsNoReference.

Verification:

• Each fix was mutation-checked: removing it turns its test red.
• All 30 packages pass, and the toolkit-self fixture controls are unchanged: 7 exits 1, 8 exits 0, 9 exits 1.
• A live sweep of 22 PRs gave no exit 2, and only #1489 is refused.

Before submitting, pin the review to this head with rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1560 --reviewed-head 6a360903dc559026d5aa9a28282dfc513778c71f (AGENTS.md section 6.2).

HOLD lifted. The new head is `6a360903dc559026d5aa9a28282dfc513778c71f`: one commit on top of `9c7601f`, addressing review 7272. - **AC6**: the NOT-checked line now also says that review bodies are not read, because Forgejo records no closing reference from them. `TestRunACClosureCheck_ScopeDisclosed` pins it. - **Nit**: a reference row without `ref_issue.repository.full_name` is now `ErrUnexpectedResponse`, so it can no longer be read as a same-numbered PR in another repo. There are two new cases in `TestListIssueTimeline_MalformedRefusesRatherThanReadingAsNoReference`. Verification: - Each fix was mutation-checked: removing it turns its test red. - All 30 packages pass, and the toolkit-self fixture controls are unchanged: 7 exits 1, 8 exits 0, 9 exits 1. - A live sweep of 22 PRs gave no exit 2, and only #1489 is refused. Before submitting, pin the review to this head with `rt review-submit-check --owner frankenbit --repo release-toolkit --pr 1560 --reviewed-head 6a360903dc559026d5aa9a28282dfc513778c71f` (AGENTS.md section 6.2).

claude reviewed 2026-10-06 09:24:00 +02:00

claude left a comment

• Copy link
• Copy Markdown

Verdict: APPROVE at 6a360903dc559026d5aa9a28282dfc513778c71f. This replaces review 7272, which was at 9c7601f. It is posted as COMMENT because the forge identity is claude, the PR's author, so @alex has to click Approve.

Refs #1506

Delta 9c7601f..6a36090, verified at source

• History is intact: 9c7601f is an ancestor of the new head, so nothing was rewritten. Four files changed: the two fixes and their tests.
• AC6: the NOT-checked line now names review bodies ("not read because Forgejo records no closing reference from them"). TestRunACClosureCheck_ScopeDisclosed pins it. AC6 is met as written.
• Nit from 7272: decodeTimelineEvent now refuses a reference row whose ref_issue.repository.full_name is missing or empty, with ErrUnexpectedResponse.
• My own mutants, run on the touched packages in golang:1.25, both killed:

Mutant	Killed by
the full_name check disabled	TestListIssueTimeline_MalformedRefusesRatherThanReadingAsNoReference, cases no repository and empty full_name
the review-body clause removed	TestRunACClosureCheck_ScopeDisclosed

• Baseline: cmd/rt, internal/gates and internal/forgejo pass.
• The stricter decoder against live data: I re-swept the 50 most recently updated PRs with this head's binary. None reaches exit 2, so no live reference row lacks full_name. Only #1489 is newly refused; #754 is refused on main too.
• CI: all 18 statuses are success. In the ac_closure step of run 19963, job 1, the gate read 7 timelines with the Actions token and finished with no target, and the decoder refused nothing.

Everything verified in review 7272 still holds for the unchanged code. AC6 and the nit are both closed.

Verdict: APPROVE at `6a360903dc559026d5aa9a28282dfc513778c71f`. This replaces review 7272, which was at `9c7601f`. It is posted as COMMENT because the forge identity is `claude`, the PR's author, so @alex has to click Approve. Refs #1506 ## Delta `9c7601f..6a36090`, verified at source - **History is intact:** `9c7601f` is an ancestor of the new head, so nothing was rewritten. Four files changed: the two fixes and their tests. - **AC6:** the NOT-checked line now names review bodies ("not read because Forgejo records no closing reference from them"). `TestRunACClosureCheck_ScopeDisclosed` pins it. AC6 is met as written. - **Nit from 7272:** `decodeTimelineEvent` now refuses a reference row whose `ref_issue.repository.full_name` is missing or empty, with `ErrUnexpectedResponse`. - **My own mutants**, run on the touched packages in `golang:1.25`, both killed: | Mutant | Killed by | |---|---| | the `full_name` check disabled | `TestListIssueTimeline_MalformedRefusesRatherThanReadingAsNoReference`, cases `no repository` and `empty full_name` | | the review-body clause removed | `TestRunACClosureCheck_ScopeDisclosed` | - **Baseline:** `cmd/rt`, `internal/gates` and `internal/forgejo` pass. - **The stricter decoder against live data:** I re-swept the 50 most recently updated PRs with this head's binary. **None reaches exit 2**, so no live reference row lacks `full_name`. Only #1489 is newly refused; #754 is refused on `main` too. - **CI:** all 18 statuses are success. In the `ac_closure` step of run 19963, job 1, the gate read 7 timelines with the Actions token and finished with no target, and the decoder refused nothing. Everything verified in review 7272 still holds for the unchanged code. AC6 and the nit are both closed.

alex approved these changes 2026-10-06 11:06:42 +02:00

alex merged commit 6a360903dc into main 2026-10-06 11:06:48 +02:00

alex deleted branch claude/1506-ac-closure-forge-xrefs 2026-10-06 11:06:48 +02:00

forgejo-actions commented 2026-10-06 11:07:40 +02:00

• Copy link
• Copy Markdown

Landing identity record

• PR: #1560
• landed commit (server merge_commit_sha): 6a360903dc559026d5aa9a28282dfc513778c71f
• effective official approval(s):
  • @alex, review #7275, stamped commit: 6a360903dc559026d5aa9a28282dfc513778c71f
• replay comparison: no identity change (stamped SHA equals landed SHA)

This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

<!-- release-toolkit:landing-review-record-v1 landed=6a360903dc559026d5aa9a28282dfc513778c71f --> ## Landing identity record - PR: #1560 - landed commit (server merge_commit_sha): `6a360903dc559026d5aa9a28282dfc513778c71f` - effective official approval(s): - @alex, review #7275, stamped commit: `6a360903dc559026d5aa9a28282dfc513778c71f` - replay comparison: no identity change (stamped SHA equals landed SHA) This is a post-merge identity record. It does not retroactively review the landed object; it records whether the server landed the object that an official approval named.

claude referenced this pull request 2026-10-06 11:08:00 +02:00

bug(gates): ac-closure-check does not see PR comments, inline review comments or the PR title, and Forgejo closes from all three #1506

claude referenced this pull request 2026-10-06 11:08:12 +02:00

bug(gates): ac-closure-check does not see PR comments, inline review comments or the PR title, and Forgejo closes from all three #1506

Sign in to join this conversation.

Reviewers

No reviewers

alex

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

epic

Tracker: its acceptance criteria are sub-issues; closed by hand when all are ticked

  

kind

feature

New feature or capability

  

loop/paused

rt-bugfix-loop kill switch: while this is on the loop log issue, no round claims or merges

  

mirror-drift

Automated: a release here has no asset-bearing counterpart on the mirror. One tracker per drift episode; the check never reopens a closed one (#1586)

  

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+

  

status

blocked

Cannot start until a named dependency resolves

No labels

bump

major

bump

minor

bump

patch

kind

bug

kind

chore

kind

docs

kind

epic

kind

feature

loop/paused

mirror-drift

priority

critical

priority

high

priority

low

priority

medium

size

L

size

M

size

S

size

XL

status

blocked

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 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!1560

No description provided.