root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs: correct every number in the self-healing sweep doc — all of mine were wrong, three different ways (#2865)

Browse Source 
CodeRabbit flagged #2838's doc as saying four sweeps converted when the
PR converted more. It merged before I could answer, so this is the
fix-forward — and re-measuring found the count itself was wrong, along
with **every intermediate number I published**.

## Measured, comments stripped

Literal column queries in `self-healing.ts`: **47 before, 36 now.**
Eight sweeps converted, all eight named in the doc.

## Three distinct errors, each recorded because the next worker re-runs
this

1. **The per-commit "N remaining" counts (44, 43, 42, 41, 40) were
arithmetic on an assumed starting point.** I decremented a number
instead of measuring one — in a program whose central discipline is that
measurement beats assumption, in commit messages that also said
"measured".
2. **A raw `grep -c` counts explanatory comments that quote the old
query form** — including the ones these conversions *add*. So converting
a sweep could leave the count unchanged, which is exactly what it
appeared to do for six of the eight.
3. **The obvious comment filter (`startsWith("//") || startsWith("*")`)
misses block-comment lines beginning with ordinary prose**, which is
most of them here. That is why my first correction said 45 and was still
wrong.

The doc now carries the strip-comments-then-count command, so the number
is **reproducible rather than quoted**.

## Also corrected

The activation-risk list is **2 sweeps, not 4** —
`finalizeNoOpReviewTasks` and `recoverCompletionHandoffLimbo` were
converted in the same PR and are no longer risky. A stale list naming
specific sweeps and line numbers is worse than a stale count: it reads
as a work queue, and I nearly "fixed" a guard I had already wired from
exactly that kind of row.

## Verification

`pnpm lint`, `check:changesets`, census `--strict` — clean. Docs-only;
no code change.
This commit is contained in:
gsxdsm
2026-07-30 16:34:45 -07:00
committed by GitHub
parent b85e5f90e1
commit 6099f028e4
1 changed files with 60 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

69
docs/solutions/architecture-patterns/self-healing-sweeps-are-blind-on-a-renamed-board.md
View File

@@ -86,10 +86,35 @@ The two shapes that work:
## Converting a sweep: the four-part shape, and the part that is easy to miss
Four sweeps are converted (`reconcileDoneTaskIntegrity`, `recoverAlreadyMergedReviewTasks`,
`recoverStuckMergeDeadlocks`, `recoverInterruptedMergingTasks`). They are deliberately identical, because
the second one drifted from the first — it was written from the pre-review version and reproduced a flaw
review had already fixed one commit earlier.
**Eight sweeps are converted**, all deliberately identical because the second one drifted from the first —
it was written from the pre-review version and reproduced a flaw review had already fixed one commit
earlier:
```text
reconcileDoneTaskIntegrity recoverMergeableReviewTasks
recoverAlreadyMergedReviewTasks recoverReviewTasksWithFailedPreMergeSteps
recoverStuckMergeDeadlocks finalizeNoOpReviewTasks
recoverInterruptedMergingTasks recoverCompletionHandoffLimbo
```
**Measured, and the measurement needed three corrections — recount before quoting it.** Literal column
queries in `self-healing.ts`, comments stripped: **47 before, 36 now.**
Every intermediate number I published was wrong, and each in a different way:
- The per-commit "N remaining" counts were **arithmetic on an assumed starting point**, not measurements.
- A raw `grep -c` counts **explanatory comments that quote the old query form** — including the ones this
conversion adds, so converting a sweep could leave the count unchanged.
- The obvious comment filter (`startsWith("//") || startsWith("*")`) misses block-comment lines that begin
with ordinary prose, which is most of them here.
Count with block comments actually stripped:
```bash
node -e 'const s=require("fs").readFileSync("packages/engine/src/self-healing.ts","utf8")
.replace(/\/\*[\s\S]*?\*\//g,"").replace(/^\s*\/\/.*$/gm,"");
console.log((s.match(/listTasks\(\{ column: "/g)||[]).length)'
```
1. **Read** — `resolveProjectColumnsForRoles(store, ROLES)`, then query each column and dedupe by id. A
read happens before any task is in hand, so there is nothing to resolve a per-task lane from. The
@@ -119,14 +144,12 @@ Widen the read and they become reachable for the first time. `recoverMergeableRe
one of them. **The sweep would have found the cards and refused them** — strictly worse than not finding
them, because it looks fixed.
Measured across `self-healing.ts`: **4 sweeps hold both a literal column query and a genuinely unwired lane
guard**; 32 hold a literal query with no such guard.
Measured across `self-healing.ts`: **2 sweeps hold both a literal column query and a genuinely unwired lane
guard**; 34 hold a literal query with no such guard.
```text
finalizeNoOpReviewTasks getTaskMergeBlocker
recoverOrphanOnlyScopeViolations getTaskHardMergeBlocker
recoverPostDoneNonContinuableWedge getTaskHardMergeBlocker
recoverCompletionHandoffLimbo getTaskMergeBlocker
```
**That number was 6 in the first version of this doc, and both extra rows were my scanner lying.** Worth
@@ -150,7 +173,7 @@ renamed-board cards and declined them.
parameter at all, and every one of its callers sat behind a literal query. Nothing exercised it.
Parts 4 and 5 are both invisible to the census — one is string contents, the other is reachability — and
each of the **43 remaining queries** carries both risks.
each of the **36 remaining queries** carries both risks.
## Testing a sweep: assert what happens ONLY when the change is correct
@@ -181,6 +204,34 @@ precisely the bug being fixed. Where a guard exists, assert the end-to-end outco
And run the revert. Every one of the four above was found that way and none by reading.
### "It needs a git fixture" is usually a private method you have not stubbed
Twice on this branch I judged a sweep's lane-sensitive guard unreachable in a unit test because it sat
behind git-backed calls, and shipped a candidacy-only assertion with the limit stated in the PR. Review
pushed back on the second one, and the objection was right: the git-backed calls —
`resolveSelfHealingMergeTarget`, `findAlreadyMergedTaskCommit` — are **private instance methods**, so
stubbing them on the manager reaches the guard with no git anywhere on the path.
```ts
const manager = new SelfHealingManager(store, { rootDir: "/repo" });
Object.assign(manager, {
resolveSelfHealingMergeTarget: vi.fn(async () => ({ branch: "main", source: "settings" })),
findAlreadyMergedTaskCommit: vi.fn(async () => ({ sha: "abcdef1234567890" })),
});
```
`executor-worktree-owner-renamed-lanes.test.ts` already did exactly this for `findActiveWorktreeOwner`,
with a header explaining why. I had read that file the same week. The guard was never unreachable — it
was unreachable *the way I first tried to reach it*, and "I stated the limit honestly in the PR" made a
gap feel resolved when it was merely disclosed.
Once you are past the guard, the assertion writes itself: the sweep's own write differs by exactly the
wiring under test — blocker clear → `status: null`, blocker fires → `status: "failed"` with a
finalization-blocked error. Prefer that over candidacy whenever a lane-sensitive guard exists.
**Before writing "this cannot be tested without a fixture", check whether the thing in the way is
private.** If it is, it is a stub, not a fixture.
## Related
- `docs/solutions/test-failures/optional-flags-seam-hides-unconverted-column-guards.md` — the same lesson one level down: the census counts syntax, and a green suite that omits the new parameter carries no information about the change.