Extract the catch-up readiness proof model out of catchup-runner.ts

[Jurij89]

Context

Deferred from PR #2007 (issue #2006). Raised there across three review rounds and declined on scope, not on merit — the reasoning is on-thread; this issue exists so it is tracked rather than lost in a closed PR's review history.

packages/cli/src/catchup-runner.ts now holds four unrelated concerns:

1. Route-level durable catch-up helpers (classifyDurableCatchupRequest, runDurableCatchupLeg)
2. The WorkerCatchupRunner class and its parent-side RPC bridge
3. Per-peer accounting (catchupPeerResponded, catchupPeerSucceeded)
4. The readiness proof/evidence model — CatchupPlaneCompletionEvidence, CatchupPlaneRoundDiagnostics, catchupPeerPlaneEvidence, addCatchupPlaneEvidence, catchupPlaneProvenByData, catchupPlaneProvenByUnanimousEmpty, catchupPlaneReady

A reader tracing readiness from context-graph-readiness.ts lands in a module whose name promises a runner, and adding a new evidence signal reads as a runner change when it is domain policy.

Proposed change

Extract (4) into packages/cli/src/catchup-proof.ts and have the worker impl and the readiness classifier import from there. The unit tests for those predicates already live together in catchup-runner.test.ts and would move as a block.

Two adjacent cleanups worth doing in the same change, since both are about how this file decomposes:

• Collapse the parallel per-plane accumulators in catchup-runner-worker-impl.ts. Note they are not identical: durable carries metaOnlyResponses, verifiedPrivateOnlyResponses, dataRejectedMissingMeta, rejectedKcs and a complete flag that gates its evidence; shared memory carries droppedDataTriples and has no complete concept (SharedMemorySyncResult has no such field). A naive collapse trades duplication for per-plane branching.
• Type the worker RPC protocol. The bridge currently uses raw method-name strings and positional unknown[], so an argument reordering compiles on both sides and surfaces as mislabelled scheduler diagnostics rather than a type error. Payloads are typed (invoke<DurableSyncResult>, CatchupPlaneContext, SyncPeerResolution); the method/arity contract is not.

Why it is cheap now

PR #2007 deliberately left it in a good position:

• The proof model is cohesive and has no dependency on the runner class or the route helpers.
• CatchupJobResult['cleanPlaneCompletions'] is now the shared CatchupPlaneCompletionEvidence rather than a structurally duplicated inline literal, so there is no second definition to move with it.
• cleanCompletionHasResponse in context-graph-readiness.ts takes that same type, so a new evidence carrier cannot be added to the model and omitted from the pre-readiness gate without a type error — the bug class that produced one of fix(sync): walk catch-up peers progressively and fail closed on empty rounds (#2006) #2007's blockers.

Constraint to respect

catchup-runner.ts is imported by the readiness classifier, the worker impl, the subscribe route and seven test files. Land this on its own so the import churn is legible as import churn, not mixed into a behavioural change.

Related

• PR fix(sync): walk catch-up peers progressively and fail closed on empty rounds (#2006) #2007 — the change that added the proof model
• Issue Catch-up fan-out overloads sync-global queue and can report incomplete graphs as done #2006 — the bug it fixes

[Jurij89]

Adding a fourth item from PR #2007 review round 13, since it belongs to the same cluster:

Model skipped planes explicitly instead of absence-as-signal. PeerRound.durable / .shared are null when the walk deliberately skipped a plane the authority already settled. The convention predates #2007 (shared was already null when includeSharedMemory was false, and both accounting helpers already had nullable signatures), and #2007 documented the meaning on the type — but absence-as-signal still reads worse than a CatchupRoundPlan / selected-plane-set model, with runCatchupPlanesWithPolicy taking the set of planes to run.

Natural companion to collapsing the per-plane accumulators above: both change how the worker's round is modelled.

[Jurij89]

Fifth item from PR #2007 review (raised three times there):

Split packages/cli/test/catchup-runner-worker-impl.test.ts — 1400 lines, 28 tests, one describe. Requires extracting the shared harness first (mocked parentPort, the runWorkerCatchup driver, durableResult() / sharedResult() fixtures), otherwise the halves duplicate ~145 lines of setup and drift.

The split must carry the process.env scoping from 8cd3cef79. That file's vi.hoisted DKG_CATCHUP_* overrides mutated the real process env and leaked into Bura: cli [3/4], which also runs daemon-http-behavior-extra.test.ts — a suite that spawns a real daemon inheriting the parent environment. Symptom was "Daemon did not become ready within 45s", nothing to do with the daemon. Any new file entering the shard rotation needs the same capture/restore in afterAll.

Use node scripts/ci/plan-vitest-shard.mjs cli <n> to see what shares a shard before splitting.

[Jurij89]

Sixth item from PR #2007 review round 16 — and this one comes with evidence that it is a deletion, not a migration:

The legacy readiness evidence model is dead in production. catchupPlaneReadyThisRun in packages/cli/src/context-graph-readiness.ts branches on cleanPlaneCompletions being absent, documented as compatibility for "an older in-process runner during a rolling upgrade". That runner is createInlineCatchupRunner, and its only callers in the tree are two benchmark scripts:

rg 'createInlineCatchupRunner|InlineCatchupRunner' packages --glob '!**/dist/**'
packages/cli/src/catchup-runner.ts:818,819,1021          # definition
packages/cli/scripts/daemon-responsiveness-benchmark.cjs:166,168
packages/cli/scripts/catchup-runner-benchmark.cjs:143,150

WorkerCatchupRunner — the only production runner — always populates cleanPlaneCompletions. So the aggregate-diagnostics path can be removed rather than migrated: make cleanPlaneCompletions required on CatchupJobResult (or split LegacyCatchupJobResult), delete the branch, and drop the test that deletes the field to exercise it.

Until that happens the compat comment in context-graph-readiness.ts is misleading and should be corrected in place.

[Jurij89]

Seventh item, from PR #2007 review round 17 — a test-harness gap rather than a refactor, but it belongs with the rest:

No subscribe-route harness, so the job-status contract is only covered at the runner. #2007 fixed the worker-exit hang (a terminated Worker emits exit, never error, so a pending run was never settled and every LATER subscribe hung too). The runner-level tests cover the rejection and the latch. The route path that turns that rejection into a user-visible job state is not covered:

} catch (err) { job.error = …; job.status = "failed"; } finally { job.finishedAt = Date.now(); }
// and dedupe only reuses in-flight jobs:
if (existingJob && (existingJob.status === "queued" || existingJob.status === "running")) { … }

Behaviour is correct today; there is simply no harness. context-graph-reconcile-route.test.ts shows the pattern (drive handleContextGraphRoutes over a real createServer), but the subscribe branch additionally needs dashDb readiness reads, the catch-up tracker, subscription state and metadata confirmation.

Worth building once — it would also cover the deferred/denied/unreachable job-status branches, which are equally uncovered. Assert: terminal status, finishedAt present, retryable error surfaced, and a later subscribe starting a fresh job rather than being deduped to the dead one.

[Jurij89]

Two further items from PR #2007 review round 25, both design-quality rather than correctness:

1. The catch-up clock seam models an invalid state. CatchupPlanePolicyClock exposes wait and now as independent optional members, but they are one seam — injecting wait alone removes the retry loop's bound entirely (measured: 6,873,671 attempts in a 2-second budget, macrotask queue starved). #2007 added a runtime guard that throws, which is correct but patches a state the type permits. A paired clock?: { now(): number; wait(ms: number): Promise<void> } would make it unrepresentable and let the guard go.

2. No regression test for a resolved curator filtered out before the walk. authorityUnanswered is set when a resolvable curator never cleanly answers. One route into that state is the curator resolving but failing the waitForSyncProtocol probe, so it is never contacted at all. #2007 covers the transport-failure route but not the filtered-out-before-the-walk route; the behaviour is believed correct (never contacted ⇒ never answered ⇒ flag set) but is unpinned.

[Jurij89]

Additional structural item from PR #2007 round 26 (review thread 3696897272):

Make the curator-sync-peer resolver's own-definition reader an explicit typed dependency.

resolveCuratorSyncPeer takes a full DKGAgent but probes its most important new dependency at runtime:

if (typeof agent.getOwnCgDefinitionFacts !== 'function') return false;

Under the type system that branch is unreachable — it exists so a casted partial receiver fails closed to ranking rather than being silently upgraded to authority. The reviewer's point stands that the requirement should be explicit at the boundary instead: a narrow dependency interface (getCgMeta, getOwnCgDefinitionFacts, discovery) that callers must satisfy, with any genuine legacy fallback pushed to the bridge that actually talks to older agents.

Deferred from #2007 as non-blocking; the fail-closed probe stays as a documented backstop in the meantime.