From 703890b00899d3cbf58620172628274072c1c8f3 Mon Sep 17 00:00:00 2001 From: Topi Jarvinen Date: Mon, 27 Jul 2026 07:58:49 +0300 Subject: [PATCH 1/2] docs: record the closing-keyword trap firing through its own safeguard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The squash-merge of the previous handoff PR closed issue 61 in error. Reopened by hand. Why the safeguard did not stop it: I measured that GitHub ignores a closing keyword inside a backticked code span, which is true OF A PR BODY, because that is rendered markdown. I generalised it to a commit message, which is plain text where a backtick is just a character, and built a pre-merge grep that strips code spans before checking. It passed the squash message, and the merge fired the keyword anyway. The measurement was right; the generalisation was not. Same string, two surfaces, opposite behaviour: PR body backticked keyword -> ignored (closingIssuesReferences: []) commit message backticked keyword -> FIRES So the proposed check has to run per surface: strip code spans for a PR body, strip nothing for a commit or squash message, and check the squash message at merge time specifically — it is composed after the PR body was reviewed and is never itself reviewed by anything. Third occurrence across three sessions, and the first to get through a safeguard built for the previous one. Claude-Session: https://claude.ai/code/session_01PWoYsKTB9fBp4knZbXqbHc --- docs/kit-friction-log.md | 50 ++++++++++++++++++++++++++-------------- docs/kit-handoff.md | 12 ++++++---- 2 files changed, 40 insertions(+), 22 deletions(-) diff --git a/docs/kit-friction-log.md b/docs/kit-friction-log.md index c3ec722..7627e19 100644 --- a/docs/kit-friction-log.md +++ b/docs/kit-friction-log.md @@ -47,23 +47,39 @@ Everything above now lives in [`kit-friction-log-archive.md`](kit-friction-log-a proposed fix: contract item 5 should say the restore belongs in a `finally` (or a git-clean check) and that any harness must verify the tree is clean *after* it exits, not only after a successful run. -- **The closing-keyword rule as written is wrong, and measuring it changed the fix.** - Last session's entry says never to write one "even negated". Acting on that, I rewrote - a PR body sentence that mentioned a backticked `` `Closes #61` ``. Then I measured: - - ``` - PR #68 body contains `Closes #61` (backticked) → closingIssuesReferences: [] - PR #63 body contains "Does NOT close #60" → closingIssuesReferences: [58, 60] - ``` - - **GitHub excludes inline code spans**; what it does not excuse is negating prose. So - the real rule is narrower than the entry below claims, and the obvious mechanical fix - — grep for `(close|fix|resolve)\w*\s+#\d+` — would fire on every legitimate backticked - reference and train people to ignore it. **M** — proposed fix: a `pr-watch` / `wrap-up` - check that strips fenced blocks and inline code **first**, then flags a closing keyword - within a few tokens of a negation (`not`, `n't`, `NOT`, "stays open"), which is the - actual failure mode. Correct the 2026-07-26 entry's rule at the same time; it is - currently stated more strongly than the evidence supports. +- **The closing-keyword trap fired a third time — through the safeguard I had just + built for it.** Sequence, because the shape of the mistake is the finding: + + 1. Last session's entry (below) says never to write a closing keyword next to an + issue number, "even negated". + 2. This session I measured that rule and found it too strong: a PR body carrying a + backticked `` `Closes #61` `` produced `closingIssuesReferences: []`, while `#63`'s + un-backticked negated mention had fired. Conclusion recorded: **GitHub excludes + inline code spans**, so the hazard is negated *prose*. + 3. I built a pre-merge grep from that conclusion — strip code spans, then look for a + closing keyword — ran it on a squash-merge message, and it passed. + 4. **The merge closed `#61` anyway.** Reopened by hand. + + The measurement was right and the generalisation was wrong. A PR body is **rendered + markdown**; a commit message is **plain text**, where a backtick is just a character. + Same string, two surfaces, opposite behaviour: + + | surface | backticked `` `Closes #N` `` | + | --- | --- | + | PR body | ignored (measured: `closingIssuesReferences: []`) | + | commit message | **fires** (measured: `#61` closed by the squash of `#68`) | + + **H** — proposed fix, now with the surface distinction that makes it correct: the + `pr-watch` / `wrap-up` check must run *per surface*. For a **PR body**, strip fenced + blocks and inline code first, then flag a keyword near a negation. For a **commit or + squash message**, strip nothing — flag every `(close|fix|resolve)\w*\s+#\d+` and make + the operator confirm each one is intended. And a squash-merge message needs checking + at merge time specifically, because it is composed *after* the PR body was reviewed and + is never itself reviewed by anything. + + Worth stating plainly for whoever hits this next: three occurrences, three sessions, + and the third got through a safeguard written for the second. A rule derived from one + surface must not be applied to another without re-measuring there. - **A review bot with an incremental model keeps a stale review across a rewrite.** CodeRabbit reviewed the pre-split head, then declined `@coderabbitai review` ("does not re-review already reviewed commits") and hit its Fair Usage limit on diff --git a/docs/kit-handoff.md b/docs/kit-handoff.md index cded8c3..01f2ef3 100644 --- a/docs/kit-handoff.md +++ b/docs/kit-handoff.md @@ -59,11 +59,13 @@ stood (355 tests), by my own mutation run at that head, or by CodeRabbit. wrap-up's own handoff block then miscited `#36`, overstated a test count, and got a GitHub rule backwards — all caught by a review lens, none by me. This is the third consecutive session where claim-vs-artifact drift is the most common finding. -- **A closing keyword inside backticks does NOT fire.** Measured: - `closingIssuesReferences` is empty for a PR whose body carries a backticked - `` `Closes #61` ``, while `#63`'s *un-backticked* "Does NOT close #60" closed it. So - the hazard is negated **prose**, not any mention — a grep that ignores code spans - would have over-fired on every legitimate reference. +- **A rule measured on one surface must be re-measured on the next.** A backticked + closing keyword is ignored in a **PR body** (rendered markdown — measured: + `closingIssuesReferences: []`) and **fires in a commit message** (plain text, where a + backtick is just a character). I measured the first, generalised to the second, built a + pre-merge grep on that generalisation, and the squash-merge closed `#61` anyway — + through the safeguard written for this exact trap. Reopened by hand; third occurrence + in three sessions. - **Two of my tests pinned nothing**, including the one whose stated thesis is "don't restate the list": it re-derived its expectation from the real `KIT_OWNED` with the prefix filter left out, so deleting that filter left the suite green. From 6085d21f130cc990c6fd8188570cdaeaa2e757b4 Mon Sep 17 00:00:00 2001 From: Topi Jarvinen Date: Mon, 27 Jul 2026 08:45:47 +0300 Subject: [PATCH 2/2] docs: stop deriving the closing-keyword mechanism; keep the conservative rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A review lens found that this PR's own correction was also wrong, which makes three wrong statements of the same rule and is rule 1's threshold. The experiment I relied on varied TWO things at once. The PR-body datum had the keyword inside a FENCED block; the commit-message datum had it INLINE. So "GitHub excludes inline code spans" was never measured, and "fenced blocks are inert everywhere, inline spans fire everywhere" fits the same evidence. Under that reading the fix proposed last round — strip inline code from a PR body before checking — reopens the exact hole that closed issue 61. The negation hypothesis is under-determined too: PR 63's body carries a plain non-negated "fix" reference alongside the negated one, so a check that flags only keywords near a negation would have passed that body clean. So: keep the standing conservative rule (never write a closing keyword next to an issue number you do not intend to close, any form, any surface). It would have prevented all three incidents; the measurement talked me out of it. The proposed check now follows the rule rather than a theory of markdown — flag every match on every surface with no stripping, cover the owner/repo, URL and colon forms the naive regex misses, and check the squash message at merge time, since nothing else does. Also corrected: three occurrences across TWO sessions, not three; the handoff line asserting the issue was "never closed", which this PR's own subject matter falsifies; and the claim that a standing safeguard existed, when it was an ad-hoc grep. Claude-Session: https://claude.ai/code/session_01PWoYsKTB9fBp4knZbXqbHc --- docs/kit-friction-log.md | 76 +++++++++++++++++++++++----------------- docs/kit-handoff.md | 18 +++++----- 2 files changed, 53 insertions(+), 41 deletions(-) diff --git a/docs/kit-friction-log.md b/docs/kit-friction-log.md index 7627e19..578843c 100644 --- a/docs/kit-friction-log.md +++ b/docs/kit-friction-log.md @@ -47,39 +47,49 @@ Everything above now lives in [`kit-friction-log-archive.md`](kit-friction-log-a proposed fix: contract item 5 should say the restore belongs in a `finally` (or a git-clean check) and that any harness must verify the tree is clean *after* it exits, not only after a successful run. -- **The closing-keyword trap fired a third time — through the safeguard I had just - built for it.** Sequence, because the shape of the mistake is the finding: - - 1. Last session's entry (below) says never to write a closing keyword next to an - issue number, "even negated". - 2. This session I measured that rule and found it too strong: a PR body carrying a - backticked `` `Closes #61` `` produced `closingIssuesReferences: []`, while `#63`'s - un-backticked negated mention had fired. Conclusion recorded: **GitHub excludes - inline code spans**, so the hazard is negated *prose*. - 3. I built a pre-merge grep from that conclusion — strip code spans, then look for a - closing keyword — ran it on a squash-merge message, and it passed. - 4. **The merge closed `#61` anyway.** Reopened by hand. - - The measurement was right and the generalisation was wrong. A PR body is **rendered - markdown**; a commit message is **plain text**, where a backtick is just a character. - Same string, two surfaces, opposite behaviour: - - | surface | backticked `` `Closes #N` `` | - | --- | --- | - | PR body | ignored (measured: `closingIssuesReferences: []`) | - | commit message | **fires** (measured: `#61` closed by the squash of `#68`) | - - **H** — proposed fix, now with the surface distinction that makes it correct: the - `pr-watch` / `wrap-up` check must run *per surface*. For a **PR body**, strip fenced - blocks and inline code first, then flag a keyword near a negation. For a **commit or - squash message**, strip nothing — flag every `(close|fix|resolve)\w*\s+#\d+` and make - the operator confirm each one is intended. And a squash-merge message needs checking - at merge time specifically, because it is composed *after* the PR body was reviewed and - is never itself reviewed by anything. - - Worth stating plainly for whoever hits this next: three occurrences, three sessions, - and the third got through a safeguard written for the second. A rule derived from one - surface must not be applied to another without re-measuring there. +- **The closing-keyword trap fired again, because I weakened a correct rule on the + strength of an experiment that did not test what I thought.** `#61` was closed by the + squash-merge of `#68` and reopened by hand. + + **What is actually measured** — three data points, with their confounds stated, + because two earlier write-ups of this stated more than the data supports: + + | artifact | form | outcome | + | --- | --- | --- | + | `#68` body | `Closes #61` inside a **fenced block** | inert (`closingIssuesReferences: []`) | + | `9c6ab3a` commit message | `Closes #61` in an **inline** span | **fired** — closed `#61` | + | `#63` body | `Does NOT close #60` **and** a plain `fix #60` | fired; which one is **not isolated** | + + **What is NOT measured, though I twice wrote as if it were:** whether an *inline* span + is inert in a PR body. The two backtick data points differ in **two** variables at once + — fenced-vs-inline *and* body-vs-commit — so "GitHub excludes inline code spans" and + "fenced blocks are inert everywhere, inline spans fire everywhere" fit the evidence + equally well. Under the second reading, the fix I proposed last round (strip inline + code from a PR body before checking) reopens the exact hole that closed `#61`. + + Likewise the *negation* hypothesis: `#63`'s body carries a plain, non-negated `fix #60` + alongside the negated mention, so a check that only flags keywords **near a negation** + would have passed that body clean. + + **H** — **stop deriving a mechanism; take the conservative rule.** Three attempts to + state this precisely have each been wrong, which is rule 1's threshold. The original + 2026-07-26 rule below — never write a closing keyword adjacent to an issue number you + do not intend to close, in any form, on any surface — would have prevented all three + incidents; the measurement talked me *out* of a correct rule. It stands as written, and + the proposed check follows it rather than any theory of markdown: + + - flag **every** match on **every** surface — PR body, every commit message, and the + squash message — with **no stripping** of code spans or fenced blocks; + - cover the forms the naive regex misses and GitHub honours: `owner/repo#61`, a full + issue URL, and `Closes: #61` (a colon, not whitespace); + - check the **squash message at merge time** specifically. It is composed *after* the + PR body was reviewed, and nothing reviews it — verified: `9c6ab3a`'s message matches + neither `#68`'s title, its body, nor any of its three branch commits, and no hook in + `scripts/` inspects commit messages at all; + - the operator confirms each match. Over-firing is the acceptable failure here. + + Occurrence count, corrected: **three occurrences across two sessions** (`#63` and `#64` + were the same session; `9c6ab3a` is this one). - **A review bot with an incremental model keeps a stale review across a rewrite.** CodeRabbit reviewed the pre-split head, then declined `@coderabbitai review` ("does not re-review already reviewed commits") and hit its Fair Usage limit on diff --git a/docs/kit-handoff.md b/docs/kit-handoff.md index 01f2ef3..c0dd0d1 100644 --- a/docs/kit-handoff.md +++ b/docs/kit-handoff.md @@ -59,13 +59,14 @@ stood (355 tests), by my own mutation run at that head, or by CodeRabbit. wrap-up's own handoff block then miscited `#36`, overstated a test count, and got a GitHub rule backwards — all caught by a review lens, none by me. This is the third consecutive session where claim-vs-artifact drift is the most common finding. -- **A rule measured on one surface must be re-measured on the next.** A backticked - closing keyword is ignored in a **PR body** (rendered markdown — measured: - `closingIssuesReferences: []`) and **fires in a commit message** (plain text, where a - backtick is just a character). I measured the first, generalised to the second, built a - pre-merge grep on that generalisation, and the squash-merge closed `#61` anyway — - through the safeguard written for this exact trap. Reopened by hand; third occurrence - in three sessions. +- **An under-determined measurement talked me out of a correct rule.** `#68`'s + squash-merge closed `#61` (reopened by hand) because I had weakened the standing + "never write a closing keyword next to an issue number, even negated" rule after + measuring one PR body as inert. That experiment varied **two** things at once — + fenced-vs-inline *and* body-vs-commit — so it never established the thing I concluded + from it. Three attempts to state the rule precisely have each been wrong; the + conservative original would have prevented all three incidents. Stop deriving the + mechanism (rule 1). - **Two of my tests pinned nothing**, including the one whose stated thesis is "don't restate the list": it re-derived its expectation from the real `KIT_OWNED` with the prefix filter left out, so deleting that filter left the suite green. @@ -81,7 +82,8 @@ stood (355 tests), by my own mutation run at that head, or by CodeRabbit. `#67` (the same hardcoded triple `#59` just fixed, where it *writes* bad config). `init.sh` has no automated test coverage and **no issue tracks that** — `#36` is the `pre-push` twin, and `#67`'s body miscites it for `init.sh`; both need correcting. -- **`#61`** — still open, never closed: the hook-detection half, with the panel's +- **`#61`** — open (closed in error by `#68`'s squash-merge, reopened by hand): the + hook-detection half, with the panel's evidence, the shape a correct fix needs, and a table of 9 `git config` value forms of which the current scan misparses 5. - **`#47`** still the highest-leverage unbuilt thing, and it subsumes `#67`.