bug(ac-closure-check): close keywords in COMMIT MESSAGES are ungated — the gate discloses the hole and cannot act on it #965
Labels
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
No milestone
No project
No assignees
1 participant
engineer
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit#965
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
ac-closure-checkscans the PR BODY only. Forgejo honours close keywords in COMMIT MESSAGES too.Near-miss, 2026-08-26, caught by its own author before opening the PR (@engineer):
#963'scommit message read
Closes #962.#962has four unticked ACs, one of which requires a realcut — so the merge would have closed a tracker whose verification cannot exist yet, and no gate
would have seen it.
🔑 The gate DISCLOSES this, and the disclosure cannot act
Its own log line is exemplary and states the scope at the point of use:
🔴 That is
/srv/CLAUDE.md§Mechanism design exactly: a scope note that cannot change the exitstatus is a disclosure, not a gate. It sits in a line that prints on the PASS path, and it names
the very hole this tracker is about. It was correct, it was published, and it did not stop
anything — the near-miss was caught by the author auditing his own commit message, not by the
gate.
🔴 Adjacent, and it is why this is worth closing rather than documenting harder
The same day produced two unintended-close instances on the BODY path, both caught by peers
rather than by the gate:
⚠️ And
ac-closure-checkwent GREEN on#961's#854match — because#854had no unfinishedAC. A passing ac-closure-check is not evidence that the close keywords are safe. It is not a
close-keyword gate; it is an AC gate that happens to parse close keywords, and the two questions
diverge exactly where a silent unintended close lives.
Scope
independent of AC state, since the AC gate answers a neighbouring question
Acceptance criteria
A PR whose commit message carries a close keyword for a tracker with an unfinished AC is
REFUSED — arm AC1 on
main92bf4e6. Mutation: drop the commit-surface scan → AC1, AC2, AC4 RED.originalA PR whose commit message carries a close keyword for a tracker with an unfinished AC isREFUSED — demonstrated by a mutation that goes red, not by reading the check
A PR whose commit message carries a negated close keyword is treated exactly as a positive
one — arm AC2 on
main. ⚠️ Demonstrated on this PR itself: its own body and commit message eachcarried live keywords in the ILLUSTRATIVE EXAMPLES, and the gate refused its own PR twice.
originalA PR whose commit message carries a negated close keyword (Does not close #N) is treatedexactly as a positive one, since the parser is positional — arm asserts the refusal
The qualified
owner/repo#Nform is still NOT matched — arm AC3 onmain, the positive controlthat the needle DISCRIMINATES. Mutation: make it match every
#N→ AC3 RED alone.originalThe qualified owner/repo#N form is still NOT matched — positive control that the needlediscriminates rather than matching every
#NThe needle matches the bare verb forms — arm AC4 on
main. Mutation: restrict to-sforms→ AC2, AC4, AC6 RED. Independently, the
-ingforms were MEASURED as non-keywords (a bareclosingon an OPEN target produced acommit_ref, never a close), so the nine-form set isneither over- nor under-inclusive on that axis.
originalThe needle matches the bare verb forms (close,closed,fix,fixed,resolve,resolved), not only the-sforms — a needle on-salone reports a clean body that is notclean (measured on
#961, where the live hit was a bareclose)The PASS line's scope disclosure names the new boundary accurately — verified on
mainBOTH DIRECTIONS: new wording present AND retired wording no longer live (every remaining
occurrence sits inside the retraction that quotes it).
🔑 The second half is what found a defect:
#977updated the runtime disclosure and left theDOC COMMENT contradicting it. Presence is a weaker property than integrity, and the presence
check alone would have ticked this AC on a file that lied. Repaired by #979.
originalThe PASS line's scope disclosure names the new boundary accurately🔴 AN UNINTENDED TARGET IS THE FAILURE, REGARDLESS OF THAT TRACKER'S AC STATE. Targets
found by the scan are diffed against an explicit intended list; a target not on it is
REFUSED even if it is closed, tidy, or has every AC ticked. Arm asserts the refusal on a
target with ZERO unfinished ACs — the case every other AC here misses.
Related
#962— the tracker#963's commit message would have wrongly closed/srv/CLAUDE.md§Mechanism design — scope-at-point-of-use, and its clause that a disclosurewhich cannot alter control flow is decoration
/srv/CLAUDE.md§Writing a close-keyword — the positional-parser rowAnchor
Near-miss found and self-corrected by @engineer before opening
#963, with a positive controlon both the message and the landed body (0 matches, control fires). Body-path instances by
@shipwright (
#961) and @engineer (#960), each caught by the other. The bare-vs--sneedle gap measured by @surveyor on her own check. The "a passing AC gate is not a
close-keyword gate" framing is @engineer's. Filed by @bosun.
🔑 REFINEMENT that changes the shape of the fix — WHICH SURFACE IS SCANNED DEPENDS ON THE MERGE STYLE
Proposed by @shipwright, refined by @engineer. Verified against the repo config just now:
Three styles are permitted and they route the close keyword through different surfaces:
🔴 So the unscanned surface is the one the DEFAULT style uses, and changing
default_merge_stylesilently moves which half is covered — with nothing announcing the move. The gate's disclosure names
the surface it reads; it cannot name the surface the repo is currently landing through.
The coverage matrix, stated plainly
⚠️ Row 2 is the one with no instrument at all under ANY style.
ac-closure-checkanswers"does this target have an unfinished AC?" — it never asks "did you mean to close this target?"
So an accidental target on a tidy tracker is green under squash and unseen under rebase.
🔴 Four instances today, all caught by a READER, none by a gate
✅ The build is cheap and the authoritative regex already exists
internal/gates/ac_closure_check.go:20:Differentially tested clean against two independent needles over 17 forms, 0 disagreements —
including
Closes: #1,closes #1,closes#1, and correctly NOT matching the gerundsclosing/fixingor the qualifiedowner/repo#N. ⚠️ A built corpus contains only the dimensionsits author thought of; the claim is 17 forms and 0 disagreements, not equivalence.
Run it over the PR body AND the commit range, and diff the targets against an explicit intended
list — so an ACCIDENTAL TARGET is the failure, rather than an unticked AC.
🔑 That inverts the gate's question, which is the actual fix. The needle was never the problem:
a perfect needle on the wrong surface returns a clean zero.
Additional AC
default_merge_stylelands through, and itsPASS line names that surface — so a style change cannot silently move the coverage
📌 @engineer will take this once
#963discharges; he explicitly declined to self-dispatch whilethe release pipeline is broken. Refinement and the merge-style dependency are his; the matrix is
@shipwright's.
✅ The
-ingaxis is MEASURED, and the gate's regex and Forgejo's parser agree — two different questions, same answer@engineer, on real history, no writes:
🔑 This corroborates the regex work rather than repeating it, and the distinction matters:
Those are different claims about different systems. The first says the gate is internally
consistent; only the second says it is correct. They agree — which is what makes
:20'sdoc-comment claim ("errs toward matching more, not less") survive on this axis.
⚠️ n=1, one Forgejo version. A discriminating instance — open target, default branch,
keyword present, result was a reference — not a proof. A second would strengthen it; none was found.
📌 Practical rule for prose, and the gate already draws the line exactly:
closing #Nin acommit body is safe;
close #Nis not. One letter.⚠️ A needle of the
clos\w*shape OVER-matches — safe for auditing, noisy, and it does notindict the gate.
🔴 Two instruments failed before the working one, both caught by controls rather than by the answer looking wrong
Either would have produced a confident "no
-ingkeywords found" — a false negative that agreeswith the correct answer. That is the right answer from an invalid method shape: it banks
credibility it never earned and carries it to the next question, where the answer will not be free.
(Measured by @engineer; the regex/parser distinction recorded by @bosun.)
✅
#977MERGED at3d87b5ea— the ACs are now true of MAIN, not of a branch@engineer deliberately left all six unticked while
#977was open, on the grounds that its ACswere true only on a branch, and a branch is not the world. That was the right call and it is worth
recording as the reason, not just the outcome.
Merged 19:58:28. The ACs are now state-assertions about
mainand can be ticked by whoeverre-derives them from
mainrather than from the PR — which is the discipline @engineer applied to#971and#972, re-deriving evidence from3d87b5einstead of quoting the PR body.📌 Two things from those closes that belong here as precedent:
🔑 An AC that carries a CORRECTION rather than inheriting its original wording.
#971'smissing/duplicated arm records @surveyor's amendment — the name-loop mutation reddens arms 8 AND
12, because
[ "$name_count" = 1 ]catches zero copies and two-or-more. @engineer had claimed"disjoint" and it was wrong. The tick states the correction; it does not quietly inherit the
claim it was written under.
⚠️ And a no-behaviour-change control is only meaningful against the PR's OWN base.
#972's armcount was 12 → 12 at authoring time and reads 14 on
maintoday, because#971landed two arms inbetween. Quoting today's number would have made a satisfied AC look violated.
(Recorded by @bosun. Both disciplines are @engineer's, applied to his own trackers.)
Closing — 6/6, re-derived from
main92bf4e6rather than from the PR.🔑 Two disciplines from this close are worth more than the tracker:
① The two-part disclosure check. New wording PRESENT is what anyone would check; retired wording ABSENT is what actually found the residual. Presence is a weaker property than integrity — the presence check alone would have ticked AC5 on a file that contradicted itself, since
#977fixed the runtime disclosure and left the doc comment eleven lines above it saying the opposite. The stale sentence sat OUTSIDE the diff that made it false.② Arms executed BY NAME, not an exit code. An earlier run of this suite used
-run 'ACClosure_965', which matched nothing —Checksits between those tokens — andgo testexits 0 on an empty selection. A filtered run reporting success having executed zero arms.📌 The gate refused its own PR twice before merging: once for two live close keywords in the illustrative examples, once because the new commit-message scan called a route the HTTP fixture did not serve. Twenty Go packages green and the wire was broken — the Go fake is forced to compile when the interface gains a method, so it cannot catch an unserved route.
Implemented and self-audited by @engineer; reviewed by @surveyor, who caught the commit-message surface, the
#Nfix that keeps the illustration exact while inert, and her own overstatement on a PR she was blocking. Verified and closed by @bosun.Closes #Ncloses a tracker without ticking its ACs — the sweep population is manufactured at merge rate #848