Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions docs/kit-friction-log.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,82 @@ accounted for.

Everything above now lives in [`kit-friction-log-archive.md`](kit-friction-log-archive.md).

## 2026-07-27

- **A mutation harness that restores outside `finally` leaves the repo mutated.** My
restore line was unreachable when the script died parsing pytest output (a bare
`python3` with no pytest returned no stdout, and `splitlines()[-1]` raised). The
working tree kept a live mutant until `git status` caught it. Neither existing warning
covers it: `#50` is about stale `.pyc`, and `fallback-review-panel.md` contract item 5
warns only about **false kills** from a checksum/drift test (it does not mention
`.pyc` at all). Both frame the hazard as "the result may be wrong"; neither says the
repo may be left broken. **H** —
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.
- **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
`@coderabbitai full review`. The PR was force-pushed and then substantially rewritten,
so the only bot review covered code that no longer existed. `pr_watch`'s
`bots_behind_head` recorded it correctly — the friction is that **nothing warns at
push time**, when re-requesting is still cheap. **M** — proposed fix: have `pr-watch`
surface `covers_head: false` as a distinct, louder line right after a push that
changes the diff shape, rather than only at receipt time. Related: `#27`, `#44`.
- **The panel's worth is disjointness, and this run measured it.** Round 1: the
adversarial lens found the enclosing-repo and `GIT_DIR` regressions; the correctness
lens independently found the `~`-expansion disagreement between `init.sh` and the new
detector. Almost no overlap. Round 2's correctness lens then found the *misattributed
defect* — a false sentence in the commit message written to describe round 1's own
outcome, which could not have existed when round 1 ran. **No change proposed** —
recording it because `fallback-review-panel.md` argues disjointness from one prior
data point, and this is a second, stronger one worth citing there. Note the honest
framing of what the panel beat: the four regressions were missed by CI, by the suite
**as it then stood**, and by the mutation run **at that head** — not by the 372-test
suite quoted elsewhere, which only exists *because* those findings were fixed.
- **The archive sweep broke a cross-reference again — and this time it broke one the
PREVIOUS sweep had written.** The 2026-07-26 entry below reports the sweep orphaning
*"see the Phase 3b block above"*. Today's sweep moved Phase 3b itself into the history
file, so the surviving text — *"see the Phase 3b block, still live in
kit-handoff.md"* — became false in a second way: the target is now in the very file
the sentence sits in. Fixed by hand in this commit. **M** — the entry below proposes
warning on `(above|below)`-style references in *moved* blocks; this instance shows the
warning must also cover references **into** a moved block from anywhere in either
document, since the block that moved was the reference's *target*, not its location.
Same family as `#53`.
- **The doc-budget remedy no-op reproduced a third time** (see the 2026-07-26 entry
below). `check_doc_budget` reported 421/400; the remedy it names printed *"nothing to
move: 5 session block(s) <= --keep 6"*; `--keep 4` was needed and found by guessing.
**No new fix proposed** — recording the third occurrence. The entry below already
records two; this makes it three, two of them in *this* repo, which is a pattern
rather than a run of bad luck.
- **A review lens's isolated worktree pointed at the wrong ref again — 5 of 5 this
session, 9 of 9 across two.** (Last session's entry below records 4 of 4; this session
ran 2 lenses × 2 rounds on `#65` plus 1 on `#68`.) Every lens detected it and cloned
the target itself, because the prompt required reporting the path and diff stat.
**H** — proposed fix: this is no longer a
caveat, it is the default behaviour. Contract item 7 should stop saying "verify" and
start saying "assume the worktree is wrong; clone the target yourself first", with the
path/diff-stat report as a required field of the lens output (the existing 2026-07-26
entry proposed the report; this one is about inverting the default).

## 2026-07-26

- **The doc-budget remedy does not fire at the default `--keep`.** `check_doc_budget`
Expand Down
99 changes: 96 additions & 3 deletions docs/kit-handoff-history.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,99 @@ Archived session narratives from [`kit-handoff.md`](kit-handoff.md). Keep active
and the next step there; this file is append-only history.

## Session log
### 2026-07-25 (Phase 3b)

**Theme —** Fixed #19 + #23 together, and then spent most of the session discovering
that **the fix rounds were more dangerous than the original diff**. Seven review rounds
on #25 alone; every one found something real; five of them found a defect introduced by
the previous round's fix.

- **#25 merged — #19 and #23 closed.** `summarize_review_bots` resolves each configured
bot to *unavailable* (outage announced on a comment body **or** a status-check
description — the surface that was invisible) or *pending* (a verdict still coming).
Pending blocks the merge gate until it ages past `review.bot_pending_grace_minutes`;
unavailable never blocks and is the action signal. **Nothing reaches `converged`** —
that is the whole design, and it is what let both be fixed at once.
- **#28 merged — #10 closed.** The state_paths suite failed from inside a lane worktree
because its fixture cleared every sandbox *env* signal and not the *cwd* one. The
issue predicted a per-test audit would be needed; it wasn't — every cwd-sensitive test
already chdirs itself. A mutation pass showed the fix makes the suite *stricter*:
three tests had been passing by accidentally discovering the real repo root.
- **#29 merged — half of #27.** `review_bots.coverage` reports which commit each bot's
last review actually saw, and `--record-review` records it as `bots_behind_head`.
Verified against the real #25, where it reproduces the gap I had had to work out by
hand an hour earlier. #27 stays open for the shape-change half.
- **#26 and #27 filed** from the friction-log inbox, both with concrete sketches.

**Decided**

- The anti-wedge property lives in `converged` alone. Every new signal feeds the merge
gate, which already needs an explicit receipt — so a gate can wait, but the poll/fix/ack
loop can always finish.
- **Report, never gate.** All three new fields (`signal`, `bot_signal`/`override`,
`coverage`) make an omission legible at merge time and block nothing. The faithful
version of each risks wedging a repo whose bot is permanently unavailable.
- **Stop patching a mechanism that keeps corrupting.** The `init.sh` marker migration
produced **three** distinct config corruptions across three rounds, each while its own
post-conditions passed and it printed success (the fourth round's finding was about
the *replacement message*, not the surgery). It was deleted rather than patched again;
`init.sh` now detects the gap and prints what to add.

**Learned**

- **A fix round on gate logic is where the next bug comes from.** Session-wide:
**13 review rounds across #25, #28 and #29 — all 13 found something.** Seven of those
findings were defects introduced by the *previous round's fix* (five on #25, two on
#29), twice at HIGH. **No round on any PR came back empty**; #29's fourth pass found
no HIGH or MEDIUM but still found four LOW, including a stale comment and a dead-
argument trap. `safety-critical-changes.md` rule 3 already says to treat "the last
round found nothing" as provisional — this session never reached that state at all,
so the practical question is not when the findings stop but what blast radius
justifies stopping anyway.
- **Stopping has to be calibrated to the blast radius, not the round count.** #25 was a
merge gate — worst case, an unreviewed PR lands. #29 is a reported-never-gating display
field — worst case, a wrong warning. Same review doctrine, different stopping points,
and saying which one applies is part of the merge decision. The `never gates` property
is what made that judgment available, and it was *proved* rather than assumed — by a
review pass sweeping report shapes ad hoc, and in-repo by
`test_review_coverage_is_reported_and_never_gates` plus the 32-combination matrix in
`test_done_keeps_its_original_merge_authorization_semantics`.
- **Reading is not running.** Three defects were invisible to careful reading and obvious
on execution — most sharply, CodeRabbit's pending check reports the zero timestamp, so
the "unmeasurable age fails open" branch was not an edge case but the *only* path that
bot ever took. The #19 guard was dead code for its own target, and only polling the
live PR showed it.
- **Mutation testing found what test names asserted and test bodies didn't.** Five
properties across the session were named in tests or claimed in a PR body and pinned by
nothing: on #29, anchored author matching, newest-review-per-bot, and the `bots=`
threading; on #25, the `init.sh` list-style branch and `grep -qi`'s case-insensitivity.
Each was found by breaking the code and watching the suite still pass.
- **Every whole-file `grep '^ key:'` in a config migration is a bug in two directions**
— it misses the key at another indent *and* matches a same-named key under an unrelated
section. This change shipped one of each.
- **Removing a dangerous mechanism does not make its replacement safe.** After deleting
the list surgery, the replacement *message* still told inline-list adopters to add a
block item — walking them into the same corruption by hand.

**Open, and owned by nothing yet**

- **#27's other half** — invalidating a receipt when the diff changes *shape*, not just
when the head moves. The cheap half (visibility) shipped; the faithful half runs into
the same wedge tension as #19/#23.
- **#26** — the fallback panel. Run manually ~10 times this session (5 rounds on #25,
4 on #29, 1 on #28), two fresh-context lenses per round. CodeRabbit completed only 3
reviews across 17 pushed heads, so the panel carried most — not all — of the load; one
of its 3 was the round that caught the "ACTION NEEDED" bug. Highest-value unbuilt thing
in the tracker.

▶ Next: **#26** (make the fallback a panel spec). CodeRabbit was rate-limited on nearly
every head all day (3 completed reviews across 17 heads), so the panel carried most of
the review load — and it is still two manual subagent launches per round, ~10 rounds this
session. Two things this session learned belong in its prompt:
require the lens to **execute** the changed paths (three defects were invisible to
reading and obvious on running), and to **mutation-test** new branches (that is what
proved five separate properties were named by tests and pinned by nothing).

### 2026-07-25 (Phase 3a)

**Theme —** Made the Phase 3 sequencing decision, and it changed under scrutiny — twice.
Expand Down Expand Up @@ -88,9 +181,9 @@ Both times the correction came from asking what a *stale reader* of the mechanis
proposed fix, so they are issue-shaped — `triage-friction-log` should graduate them rather
than leaving them in the inbox.

✔ Done — shipped as PR #25 (see the Phase 3b block, still live in
[`kit-handoff.md`](kit-handoff.md)). The design constraint held:
neither surface's fix touches `converged`.
✔ Done — shipped as PR #25 (see the Phase 3b block, swept into this file by the
2026-07-27 archive run). The design constraint held: neither surface's fix
touches `converged`.

### 2026-07-25

Expand Down
Loading
Loading