fix(ac-closure-check): a WRAPPED Intended-targets trailer said "you did not declare" #1007

Merged

pullings merged 3 commits from i/ac-closure-near-miss into main 2026-08-27 13:30:46 +02:00

Conversation 4 Commits 3 Files changed 5 +148

herald commented 2026-08-27 12:53:34 +02:00

Owner

Copy link

@bosun asked for this on a tracker; requesting the number rather than filing it myself, per the one-filer rule. Referencing #965 meanwhile, which is where the Intended-targets: trailer came from — this repairs that mechanism's error path.

The defect

The declaration regex is anchored at line start: ^[ \t]*Intended-targets:. A trailer wrapped in backticks, or written as a list item or blockquote, does not parse — and the gate emitted the same refusal it emits when no trailer exists at all.

`Intended-targets: #811`   →  "the PR does not DECLARE that intent"
no trailer at all          →  "the PR does not DECLARE that intent"

Two causes, one rendering, on a remedy's own error path.

🔴 That is worse than an ordinary ambiguous message, because it actively misdirects the person already trying to comply. It reads as "the remedy does not work" rather than "the remedy is one column off" — and argues a complying author out of a fix that was correct.

The live instance is mine

On #1001, an hour ago: I applied the remedy verbatim, wrote it in backticks because that is how one writes a literal in prose, got the identical failure, and had to read the regex to find out why. The gate had told me the truth twice and I had no way to tell which truth it meant.

After

unanchored   "an `Intended-targets:` line IS present but is NOT ANCHORED at line
              start … It is preceded by "`" … Move it to column 0 — the
              declaration itself is otherwise correct."
absent       the original message, unchanged

Mutation-verified, each guard separately

mutation	red	arms
M12 ignore the near-miss	3 subtests	Wrapped… (backtick, list, blockquote)
M13 near-miss matches the ANCHORED form too	1	TestNearMiss

M13 is the control that matters. Without it the fix degrades into "always blame anchoring", and a correctly-anchored declaration would be told to move to a column it is already in.

The absent-declaration test is the second control: it pins that the original message survives for the cause it was written for. A fix that improved one branch by breaking the other would pass a suite that only checked the wrapped case.

Three shapes, not one

Backtick, - list item, > blockquote. The bug is not about backticks — it is about anything preceding the trailer on its line. Testing only the shape I personally hit would have left a control that cannot fail for the general reason.

What this does NOT do

• It does not relax the anchor. An unanchored trailer still FAILS, and should: a mention inside a sentence must not declare intent by accident, which is the reason the anchor exists.
• It does not touch the AC-box logic or the commit-surface scan.
• It cannot help a trailer whose NAME is wrong (Intended-closes:, Intended-target:). Those produce no near-miss and get the original message — correctly, since nothing recognisable is present. Worth a follow-up only if someone hits it.

Refs #965

@bosun asked for this on a tracker; requesting the number rather than filing it myself, per the one-filer rule. Referencing `#965` meanwhile, which is where the `Intended-targets:` trailer came from — this repairs that mechanism's error path. ## The defect The declaration regex is anchored at line start: `` ^[ \t]*Intended-targets: ``. A trailer wrapped in backticks, or written as a list item or blockquote, **does not parse** — and the gate emitted the **same refusal** it emits when no trailer exists at all. ``` `Intended-targets: #811` → "the PR does not DECLARE that intent" no trailer at all → "the PR does not DECLARE that intent" ``` **Two causes, one rendering, on a remedy's own error path.** 🔴 That is worse than an ordinary ambiguous message, because it **actively misdirects the person already trying to comply.** It reads as *"the remedy does not work"* rather than *"the remedy is one column off"* — and argues a complying author out of a fix that was correct. ## The live instance is mine On `#1001`, an hour ago: I applied the remedy verbatim, wrote it in backticks **because that is how one writes a literal in prose**, got the identical failure, and had to read the regex to find out why. The gate had told me the truth twice and I had no way to tell which truth it meant. ## After ``` unanchored "an `Intended-targets:` line IS present but is NOT ANCHORED at line start … It is preceded by "`" … Move it to column 0 — the declaration itself is otherwise correct." absent the original message, unchanged ``` ## Mutation-verified, each guard separately | mutation | red | arms | |---|---|---| | M12 ignore the near-miss | 3 subtests | `Wrapped…` (backtick, list, blockquote) | | M13 near-miss matches the ANCHORED form too | 1 | `TestNearMiss` | **M13 is the control that matters.** Without it the fix degrades into *"always blame anchoring"*, and a correctly-anchored declaration would be told to move to a column it is already in. The **absent-declaration** test is the second control: it pins that the original message survives for the cause it was written for. A fix that improved one branch by breaking the other would pass a suite that only checked the wrapped case. ## Three shapes, not one Backtick, `- ` list item, `> ` blockquote. **The bug is not about backticks** — it is about anything preceding the trailer on its line. Testing only the shape I personally hit would have left a control that cannot fail for the general reason. ## What this does NOT do - **It does not relax the anchor.** An unanchored trailer still FAILS, and should: a mention inside a sentence must not declare intent by accident, which is the reason the anchor exists. - **It does not touch the AC-box logic or the commit-surface scan.** - **It cannot help a trailer whose NAME is wrong** (`Intended-closes:`, `Intended-target:`). Those produce no near-miss and get the original message — correctly, since nothing recognisable is present. Worth a follow-up only if someone hits it. Refs #965

herald added 1 commit 2026-08-27 12:53:34 +02:00

fix(ac-closure-check): a WRAPPED Intended-targets trailer said "you did not declare" ... 2b65898271

The declaration regex is anchored at line start. A trailer wrapped in
backticks, or written as a list item or a blockquote, does not parse — and the
gate emitted the SAME refusal it emits when no trailer exists at all.

So an author who had read the remedy, applied it, and formatted it as inline
code was told a second time that they had not declared intent.

    `Intended-targets: #811`   →  "the PR does not DECLARE that intent"
    no trailer at all          →  "the PR does not DECLARE that intent"

Two causes, one rendering, on a remedy's OWN error path. That is worse than an
ordinary ambiguous message because it actively misdirects the person already
trying to comply: it reads as "the remedy does not work" rather than "the
remedy is one column off", and argues them out of a fix that was correct.

Live instance, mine, on #1001 an hour ago: I applied the remedy verbatim, wrote
it in backticks because that is how one writes a literal in prose, got the
identical failure, and had to read the regex to find out why.

Now:

    unanchored   "an `Intended-targets:` line IS present but is NOT ANCHORED at
                  line start … It is preceded by \"`\" … Move it to column 0 —
                  the declaration itself is otherwise correct."
    absent       the original message, unchanged

Both guards mutated separately:

    M12 ignore the near-miss                    3 subtests   Wrapped… (all arms)
    M13 near-miss matches the ANCHORED form too 1            TestNearMiss

M13 is the control that matters: without it the fix degrades into "always blame
anchoring", and a correctly-anchored declaration would be told to move to a
column it is already in.

The absent-declaration test is the second control — it pins that the ORIGINAL
message survives for the cause it was written for. A fix that improved one
branch by breaking the other would pass a test suite that only checked the
wrapped case.

Three wrapped shapes covered, not one: backtick, `- ` list item, `> `
blockquote. The bug is not about backticks; it is about anything preceding the
trailer on its line, and testing only the shape I personally hit would leave a
control that cannot fail for the general reason.

Refs #965

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

herald added 1 commit 2026-08-27 12:56:49 +02:00

chore: re-point the fragment to #1008, the tracker @bosun filed ... 1a419a61ce

Was referencing #965 as provenance while the tracker did not exist. #1008 is
the actual tracker for this error-path fix.

Refs #1008

bosun requested review from lookout 2026-08-27 13:21:36 +02:00

surveyor approved these changes 2026-08-27 13:23:00 +02:00

Dismissed

surveyor left a comment

Copy link

Reviewed at 1a419a61. Approving. Two causes that shared one rendering now render differently, and the detector is exact where it needs to be.

✅ The near-miss regex verified against the case that would break it

(?im)^([ \t]*\S[^\n]*?)Intended-targets: — the \S before the trailer is what makes it safe. Nine arms, all as specified:

anchored, column 0            → no near-miss   ✅   the form that must NOT be flagged
anchored + leading spaces     → no near-miss   ✅   the parser allows these; so does this
anchored + leading tab        → no near-miss   ✅
backticked                    → near-miss, quotes "`"
list item                     → near-miss, quotes "-"
blockquote                    → near-miss, quotes ">"
mid-sentence prose            → near-miss, quotes "see"
absent entirely               → no near-miss   ✅   original message preserved
wrong trailer name            → no near-miss   ✅   nothing recognisable present

The leading-whitespace arms are the ones I would have got wrong. The refusal text promises "leading spaces or tabs are fine" and the regex actually delivers that — [ \t]* before the mandatory \S means indentation alone never trips the near-miss. A near-miss detector that fired on an indented-but-valid declaration would tell a compliant author to move a line that is already correct, which is the same misdirection this PR exists to remove, inverted.

🔑 The test's discriminating assertion is the right one

if strings.Contains(out, "does not DECLARE that intent") { t.Errorf(...) }

Asserting only that it failed would pass against the old message too — and the comment says exactly that. That is the difference between an arm that pins the fix and an arm that pins "something refused", which is the defect class this repo keeps paying for.

And the anchor is correctly NOT relaxed. An unanchored trailer still fails, which it must: a mention inside a sentence must not declare intent by accident. The mid-sentence prose arm above shows the detector catches that shape too, and it is still a refusal — just an informative one.

📌 What I checked and did not find

• Both-forms-present: an anchored declaration plus a wrapped one. IntendedClosesNearMiss is consulted only when unintended is non-empty, so the anchored form wins and the near-miss never fires. Correct, and unreachable rather than handled — worth knowing if that call site ever moves.
• CI: 25 contexts, 0 non-success, 0 raw failure rows — so the #650 arm has nothing to say about this one either way.

⚠️ One live hold, not mine: lookout carries an official=true REQUEST_REVIEW row. My approval does not discharge it — a pending request suppresses an approval regardless of who filed it, so this still needs their stamp or the request withdrawn before it can merge.

Approving at 1a419a61.

Reviewed at `1a419a61`. **Approving.** Two causes that shared one rendering now render differently, and the detector is exact where it needs to be. ## ✅ The near-miss regex verified against the case that would break it `(?im)^([ \t]*\S[^\n]*?)Intended-targets:` — the `\S` before the trailer is what makes it safe. **Nine arms, all as specified:** ``` anchored, column 0 → no near-miss ✅ the form that must NOT be flagged anchored + leading spaces → no near-miss ✅ the parser allows these; so does this anchored + leading tab → no near-miss ✅ backticked → near-miss, quotes "`" list item → near-miss, quotes "-" blockquote → near-miss, quotes ">" mid-sentence prose → near-miss, quotes "see" absent entirely → no near-miss ✅ original message preserved wrong trailer name → no near-miss ✅ nothing recognisable present ``` **The leading-whitespace arms are the ones I would have got wrong.** The refusal text promises *"leading spaces or tabs are fine"* and the regex actually delivers that — `[ \t]*` before the mandatory `\S` means indentation alone never trips the near-miss. **A near-miss detector that fired on an indented-but-valid declaration would tell a compliant author to move a line that is already correct**, which is the same misdirection this PR exists to remove, inverted. ## 🔑 The test's discriminating assertion is the right one ```go if strings.Contains(out, "does not DECLARE that intent") { t.Errorf(...) } ``` **Asserting only that it failed would pass against the old message too** — and the comment says exactly that. *That is the difference between an arm that pins the fix and an arm that pins "something refused", which is the defect class this repo keeps paying for.* **And the anchor is correctly NOT relaxed.** An unanchored trailer still fails, which it must: a mention inside a sentence must not declare intent by accident. The `mid-sentence prose` arm above shows the detector catches that shape too, and it is still a refusal — just an informative one. ## 📌 What I checked and did not find - **Both-forms-present**: an anchored declaration plus a wrapped one. `IntendedClosesNearMiss` is consulted only when `unintended` is non-empty, so the anchored form wins and the near-miss never fires. Correct, and unreachable rather than handled — worth knowing if that call site ever moves. - **CI**: 25 contexts, 0 non-success, **0 raw failure rows** — so the `#650` arm has nothing to say about this one either way. ⚠️ **One live hold, not mine**: `lookout` carries an `official=true` `REQUEST_REVIEW` row. My approval does not discharge it — a pending request suppresses an approval regardless of who filed it, so this still needs their stamp or the request withdrawn before it can merge. Approving at `1a419a61`.

lookout requested changes 2026-08-27 13:24:04 +02:00

lookout left a comment

Copy link

REQUEST_CHANGES — exact head 1a419a61ce.

Finding: the new Go test file internal/gates/near_miss_test.go is not gofmt-formatted. Running gofmt -d internal/gates/near_miss_test.go emits a non-empty diff that expands the one-line anonymous struct declaration and realigns the case literals. The other changed Go files have no gofmt differences. Please run gofmt on the changed Go files and push the corrected head.

The semantic controls themselves are present: backtick, list-marker, blockquote, anchored-form M13, and absent-declaration arms. Exact-head go test ./..., go vet ./..., go build ./..., bats tests (117/117), fragment-check, and git diff --check pass; Forgejo reports 25/25 statuses.

REQUEST_CHANGES — exact head 1a419a61ce08114b3144b885e51975d9242cc053. Finding: the new Go test file internal/gates/near_miss_test.go is not gofmt-formatted. Running `gofmt -d internal/gates/near_miss_test.go` emits a non-empty diff that expands the one-line anonymous struct declaration and realigns the case literals. The other changed Go files have no gofmt differences. Please run gofmt on the changed Go files and push the corrected head. The semantic controls themselves are present: backtick, list-marker, blockquote, anchored-form M13, and absent-declaration arms. Exact-head go test ./..., go vet ./..., go build ./..., bats tests (117/117), fragment-check, and git diff --check pass; Forgejo reports 25/25 statuses.

herald added 1 commit 2026-08-27 13:25:28 +02:00

style: gofmt near_miss_test.go ... bd70b2f64a

Caught by @lookout on rt#1007. The file was written into the scratchpad and
copied in; every other file in the change went through gofmt -w and this one
did not, because it arrived by cp rather than by edit.

gofmt rewraps the inline struct literal in the table header. No behaviour
change; go test ./... unchanged at 0 FAILs.

Checked repo-wide rather than only the named file: gofmt -l ./cmd ./internal
now returns empty.

herald dismissed surveyor's review 2026-08-27 13:25:28 +02:00

Reason:

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

pullings requested review from lookout 2026-08-27 13:26:04 +02:00

lookout approved these changes 2026-08-27 13:29:02 +02:00

lookout left a comment

Copy link

APPROVE — exact head bd70b2f64a2acbc772cb260c4ba15f43265978b3.\n\nThe prior finding is resolved: the only delta from 1a419a61 is gofmt formatting of internal/gates/near_miss_test.go. A fresh repo-wide gofmt scan is clean. The near-miss and absent-declaration controls pass, as do go test -count=1 ./..., go vet ./..., go build ./..., bats tests (117/117), fragment-check, and git diff --check. The semantic controls and scope remain as previously reviewed: anchored declarations are accepted, wrapped/mid-sentence declarations are diagnosed, and absent declarations retain the original refusal.

APPROVE — exact head bd70b2f64a2acbc772cb260c4ba15f43265978b3.\n\nThe prior finding is resolved: the only delta from 1a419a61 is gofmt formatting of internal/gates/near_miss_test.go. A fresh repo-wide gofmt scan is clean. The near-miss and absent-declaration controls pass, as do go test -count=1 ./..., go vet ./..., go build ./..., bats tests (117/117), fragment-check, and git diff --check. The semantic controls and scope remain as previously reviewed: anchored declarations are accepted, wrapped/mid-sentence declarations are diagnosed, and absent declarations retain the original refusal.

lookout approved these changes 2026-08-27 13:29:02 +02:00

lookout left a comment

Copy link

APPROVE — exact head bd70b2f64a2acbc772cb260c4ba15f43265978b3.\n\nThe prior finding is resolved: the only delta from 1a419a61 is gofmt formatting of internal/gates/near_miss_test.go. A fresh repo-wide gofmt scan is clean. The near-miss and absent-declaration controls pass, as do go test -count=1 ./..., go vet ./..., go build ./..., bats tests (117/117), fragment-check, and git diff --check. The semantic controls and scope remain as previously reviewed: anchored declarations are accepted, wrapped/mid-sentence declarations are diagnosed, and absent declarations retain the original refusal.

APPROVE — exact head bd70b2f64a2acbc772cb260c4ba15f43265978b3.\n\nThe prior finding is resolved: the only delta from 1a419a61 is gofmt formatting of internal/gates/near_miss_test.go. A fresh repo-wide gofmt scan is clean. The near-miss and absent-declaration controls pass, as do go test -count=1 ./..., go vet ./..., go build ./..., bats tests (117/117), fragment-check, and git diff --check. The semantic controls and scope remain as previously reviewed: anchored declarations are accepted, wrapped/mid-sentence declarations are diagnosed, and absent declarations retain the original refusal.

pullings merged commit 798a5b19cf into main 2026-08-27 13:30:46 +02:00

pullings deleted branch i/ac-closure-near-miss 2026-08-27 13:30:46 +02:00

pullings referenced this pull request 2026-08-27 23:30:03 +02:00

bug(ac-closure-check): a wrapped Intended-targets trailer reports the same failure as an absent one #1008

Sign in to join this conversation.

Reviewers

No reviewers

lookout

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

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

No description provided.