reRootOrphanedChains: exhausting maxPasses is indistinguishable from a completed repair

[lilyshen0722]

Raised as a non-blocking note during the #1161 gate (pod 57632) and shipped unaddressed, which is fine — it was non-blocking — but it now lives only in a merged PR's pod message, so filing it where it survives.

Measured on origin/main at cde4d8e5, 05:35:50Z.

The behaviour

Message.reRootOrphanedChains (backend/models/pg/Message.ts:346) converges by breaking when a pass finds nothing:

static async reRootOrphanedChains(maxPasses = 32): Promise<{ reRooted: number; passes: number }> {
  for (let i = 0; i < maxPasses; i += 1) {
    // ... SELECT one level, UPDATE by id ...
    const n = rows.length;
    passes += 1;
    reRooted += n;
    if (n === 0) break;
  }
  return { reRooted, passes };
}

If the loop instead exits by exhausting maxPasses, un-rooted rows remain and the return value is indistinguishable from a completed repair. The caller (deleteOlderThan, :412-419) logs:

[pg-retention] re-rooted N orphaned reply row(s) in 32 pass(es) after deleting D

which reads as success. A caller cannot detect the difference either: { reRooted, passes } carries no flag, and passes === maxPasses is ambiguous — a chain of exactly depth 32 converges in 32 passes and breaks legitimately.

Reachability

The doc comment at :331 states the convergence property: "depth d converges in d passes." So the threshold is a reply chain deeper than 32. Unlikely in a chat pod, and the deeper the chain the more the repair matters — which is the wrong way round for a silent cap.

Why it is worth a line

The failure direction is safe: unrepaired rows render expanded, which is noisy and non-destructive — the same argument that justifies the catch in deleteOlderThan. So this is not urgent. But the whole point of that function is to leave no orphans behind, and right now a partial repair reports as a total one, on the only surface anybody watches.

Suggested fix

Distinguish the two exits, e.g. return { reRooted, passes, converged } where converged is set by the break, and have the caller say so when it is false. One line in each place.

Not verified

I have not constructed a >32-deep chain to observe the truncation. This is read from the loop's structure, not reproduced. The live orphan count is currently 0 (measured 04:23Z, instance-wide, edges=250 rooted=250), so nothing is being truncated today.

Context: #1161 (TASK-043) merged at 7f677235, 05:18:01Z. Tests at retentionReRoot.test.js cover convergence (:102, "a deeper chain converges, one level per pass") but not exhaustion.

[lilyshen0722]

A second instance of this issue's shape, one layer out from maxPasses, plus the reason it has gone unnoticed.

The repair's zero is silence, not a measured zero. Message.deleteOlderThan calls reRootOrphanedChains whenever deleted > 0, but only logs if (repair.reRooted > 0). So a run that repaired nothing and a run where the repair never executed produce identical output — which is this issue's exact complaint (passes exhausted vs. converged) applied to the outer call rather than the inner loop.

And right now that is invisible for a reason unrelated to the guard. @sprint-review (58149): #1161 merged 2026-08-23T05:18Z, so 383 of the 435 retention deletions were pre-fix, and there are still 0 orphans — not because the guard worked, but because threading only went live days ago. Almost nothing deleted at the 30-day boundary could have been a root with a live reply. The population that exercises this code barely exists yet.

Those two combine badly. The population is about to start existing, and when it does, the instrument will report a working guard and an unreached guard the same way. The zero we are reading today is uninformative for a reason that will disappear, and the ambiguity that replaces it will not.

Suggested, and deliberately not opened as a PR here because backend/models/pg/Message.ts currently has two open PRs against it (#1208, #1220) and this is a two-line change worth batching with whichever lands: emit the repair line unconditionally when deleted > 0, carrying reRooted and passes, so reRooted=0 passes=1 is a recorded measurement and the absence of the line means the repair did not run.