Fix sourcedFrom blob metadata divergence

[kriszyp]

Problem

Concurrent independent sourcedFrom fills can settle on opposite winners across replicated nodes. The original four-node cached-blob test exposed this as metadata from one fill paired with a blob endpoint response from another, with no self-healing.

The smaller harness isolates the mechanism:

• two replicated nodes;
• two HTTP workers per node, pinned with keep-alive connections;
• an external barrier that makes both nodes resolve the same missing key independently;
• unique metadata and 16 KiB file-backed blob content for each resolution — the payload: Blob attribute is what makes a metadata/blob split observable at all, and 16 KiB keeps it above core's 8 KiB inline-storage threshold;
• raw-store and point-read probes on every worker, exported as JS resources alongside the REST-exported table.

The records were not torn internally. Each node could retain a different complete winner after both local fills encountered a peer fill. Routing metadata and blob reads across those divergent nodes produced the apparent split.

Change

This PR adds the regression validating the fix in HarperFast/harper#2065 — Fix sourcedFrom cache-fill conflict convergence (merged into core main 2026-08-29 as ba81e7b04; core here is bit-identical to origin/main's pointer, so this PR moves no submodule).

The core fix reloads commit-time state, applies deterministic ordering to competing positive first fills, preserves strict revalidation/deletion safety, and updates indices/created-time metadata against the actual winner.

The test drives that fix through two race shapes, because they are settled by different arbiters and a regression in one is invisible to the other:

• distinct — no origin lastModified, so each node mints its own monotonic version and ordinary version ordering settles the race. This is the original harper-pro#645 shape.
• tied-late — the origin reports one shared lastModified to both nodes, so both fills carry the same candidate version, and the losing fill is parked at the origin until its peer's record has replicated in. Its commit therefore lands against a raced record it cannot order by version, which is the branch core's own comment at resources/Table.ts:7209 singles out: "a fill from a shared source has no node identity of its own, so two replicas resolving the same tie could keep different values at the same version."

Both shapes now run on rocksdb and lmdb (parametrized the way fullyConnectedReplication.test.mjs already does), because the resolution branches on isRocksDB in two places that decide the stored version.

A barrier that never staged two concurrent fills no longer fails the trial — it restages on a suite-wide budget, so a scheduling accident (one node learning the key by replication before issuing its own fill) cannot red-build a run that demonstrated nothing, while a race that can never be staged still fails inside the suite timeout.

Four follow-up commits came out of the pre-push review and are all about this file failing as a test rather than as a process: per-phase bootstrap budgets (an unanswering operations socket could otherwise outlast the 25-minute cluster job without any phase reporting its own diagnostic), settling both setup fan-outs so a sibling cannot publish a Harper node after after has already walked the list, releasing the barrier on every exit path, and tolerating a hole in the pinned-agent array during teardown. Each of those leaked a Harper child or killed the runner outright rather than failing a trial.

Verification

Executed on this branch, core at eec839ee, @harperfast/rocksdb-js 2.9.0:

• npm run build — clean, 0 errors
• npm run test:unit — 1068 passing
• npm run lint:required — clean; prettier --check clean; git diff --check clean
• New regression at CI defaults (TRIALS=10, WORKERS=2, both engines) — 3/3 clean runs, plus 4 more at TRIALS=9
• Mutation experiment above — 2 unmutated control runs (pass 2 / fail 0 each), 2 runs per mutated core state, core restored to eec839ee and dist rebuilt between every state
• Already green in CI on the previous head: Cluster Integration Tests 3/6 on 61e72b50 ran this suite — 10 two-node cache-fill races ... (31911ms) passing

Mutation evidence, including a red-then-green against a pre-fix core

@cb1kenobi measured that mutating the convergence guard survives harper-pro's unit suite, so the integration test is the only guard. Every row is a mutation of core/resources/Table.ts with dist rebuilt against it. Rows 2-3 were measured earlier in this PR's history; rows 1, 4 and 5 were measured on the current head, each bracketed by an unmutated control run:

core state	rocksdb leg	lmdb leg
unmodified eec839ee — control, run twice	pass	pass
const replacesRacedRecord = true	fails at tied-late-1	fails at tied-late-1
sourceVersion >= racedVersion (tie only)	fails at tied-late-1 / -3	fails at tied-late-1 / -3
pre-#2065 guard restored verbatim (exact-CAS + entry: existingEntry), rest of #2065 left in place	passes	fails at distinct-0 / distinct-2 — 2/2 runs
#2065's whole Table.ts half reverted onto current core	fails	fails — 2/2 runs

The mutated runs fail exactly as harper-pro#645 predicts — two nodes holding each other's record and never converging, not a timeout:

node 127.0.0.5  version 1789410363324.6072  token distinct-2:127.0.0.7:1:1
node 127.0.0.7  version 1789410363324.6248  token distinct-2:127.0.0.5:2:0

The last row is the red-then-green demonstration, and it replaces the caveat this description previously carried. Building harper-pro against core at ba81e7b04^ is genuinely impossible — current replication/replicationConnection.ts needs six core APIs that only exist after it (createBlobFromStoredBody, openStoredBlobBody, blobFileMissingOrIncompleteAsync, two REPLICATION_BLOBGAP* config keys, AuditRecord.txnLogKey), so tsc fails before a node can start. Reverting #2065's Table.ts hunks onto current core isolates the fix better anyway: the other 280 core commits stay fixed, so a red is attributable to #2065 and nothing else. Two conflicts had to be hand-resolved, both mechanical — recordVersion becomes txnTime (its pre-fix value) to keep the later-added txnLogKey line compiling, and VERSION_REUSED stays exported because a later feature (PrimaryRocksDatabase.ts:171) now consumes it.

Against that pre-fix core the suite is red on both engines, twice over: tied-late-1 fails its tie precondition (pre-fix code stores txnTime, not the source-reported version, so the two fills cannot tie at all) and distinct-0 fails as outright divergence.

The third row is also a correction: the verbatim guard restoration is caught, on the lmdb leg, at distinct-0 on the very first trial in one of the two runs. It was recorded as surviving earlier in this PR's history; it does not survive the engine parametrization @cb1kenobi asked for, which is that thread's concrete payoff.

For the human reviewer

How much of harper-pro#645 this suite actually guards — no longer an open question, but read what each leg guards. The suite is red against a pre-fix core and green against the fixed one (table above), so it does discriminate a #645 reintroduction. The two engine legs do not discriminate it the same way, and that asymmetry is worth knowing before anyone trims the matrix: on lmdb the distinct shape diverges from the restored CAS guard alone, while on rocksdb nothing goes red until #2065's version derivation is reverted as well, and then it is the tied-late tie precondition that catches it. Deleting either leg halves the guard — which is the argument against the CI-cost item below.

A shape this test deliberately does NOT assert, because it is red on main today. The natural reading of the equal-version suggestion is two concurrent fills at a shared lastModified — each node commits its own record at that identical version before its peer's arrives. That does not converge, on either storage engine, against unmodified core: the nodes end up holding each other's record at the same version permanently. Reproduce by adding { id: 'tied', stagger: false, tied: true } to SHAPES and running the suite; it fails as Timed out waiting for tied-N convergence with matching versions and differing tokens, roughly 4 runs in 10. This looks like a genuine product defect rather than a test artifact, but it is out of scope for a #645 regression PR and needs its own issue and core fix. This PR should not be read as evidence that the concurrent equal-version tie converges.

Residual for rows that already diverged (raised by @cb1kenobi). The core fix is forward-only. Two replicas already sitting on different values at the same version are not repaired by it, and a fill only returns to the source when the record is missing, invalidated/evicted, or expired (core/resources/Table.ts:6685) — so an already-filled divergent row is never naturally revisited. Operators upgrading past this fix should invalidate or evict affected caching tables if they suspect divergence; otherwise normal TTL expiry is what clears it.

tied-late staging is verified, not assumed, but it is still timing-dependent. The trial only asserts convergence after confirming the peer's record is visible on the parked node; if it never arrives within the barrier window the trial restages rather than asserting. That was measured to engage — with core instrumented, the parked node's commit reports racedVersion == sourceVersion and replacesRacedRecord: false — but the confirmation is a store probe, not a hook into the commit, so a sufficiently adverse interleaving could still fall back to the ordinary shape without saying so.

Deferred, not fixed (judgment calls about CI cost vs. diagnostics, carried forward from earlier rounds):

• bootstrap phases and each trial's convergence waits can still sum past the suite's 300s timeout in the worst case, so a genuinely wedged run may surface node:test's generic timeout instead of this file's diagnostic. Now more likely than before: the file declares two suites, and on a loaded host each took 50-90s.
• all trials run on both engines on every test:integration:cluster pass — the dominant recurring cost of this PR, roughly 90s on an idle host. A HARPER_RUN_STRESS_TESTS-style gate with a 1-trial default, or the full matrix only nightly, would cut it; both are reachable through the existing HARPER_645_TRIALS / HARPER_645_WORKERS env vars. Cut trials or frequency, not an engine: the mutation table shows each leg catching a different mutation.
• PairPointProbe/PairScanProbe read primaryStore.getRange directly rather than only through public read paths; that is the only surface that can show a metadata/blob split the cached path hides, but it couples the test to store internals.
• the test pins one keep-alive agent per worker thread and asserts the connection never moves, which ties it to HTTP agent/socket behavior that is not a documented Harper guarantee.
• bootstrap and probing are hand-rolled here rather than reusing clusterShared.mjs's sendOperation/waitForCondition; the bespoke path is what buys per-worker socket pinning, at the cost of fixing every timeout bug once per file. The phase budgets added this round are the second such fix.
• waitForConvergence / waitForAllWorkers (both pre-existing) use Promise.all, so one probe rejecting fast abandons its siblings in flight and the next poll queues behind them on the same maxSockets: 1 agent. Bounded by the 30s convergence deadline, so it degrades to a slow poll rather than a hang, but allSettled would be strictly better.
• pinWorkers keeps its pre-existing 40-iteration cap alongside the new phase deadline; at 100ms per warmup miss the cap binds at ~4s, well before the 60s budget, so a slow-to-warm worker fails the assertion rather than using the budget.

Dependency

Core dependency resolved: HarperFast/harper#2065 merged 2026-08-29. This PR no longer needs a Depends-on: gate or a pinned core pointer — core here matches origin/main, and @harperfast/rocksdb-js 2.9.0 (which harper-pro already pins) exports the VERSION_NOT_UNIQUE_FLAG that core main consumes at core/resources/RecordEncoder.ts:131.

Fixes #645

Authored by GPT-5 Codex; this round by Claude Opus 5.

_{Review-Coverage: authored=claude; ran=codex,gemini; declined=cursor-grok,cursor-composer,domain; rounds=10; full=2 @ 9f292b8}

_{Human-Review-Need: 3 @ 9f292b8}

[gemini-code-assist]

Code Review

This pull request adds a new integration test suite and associated fixtures to verify that a sourced record's metadata and blob converge correctly during competing cache fills across multiple nodes. The feedback recommends replacing CommonJS-specific globals with ESM-safe fallbacks, ensuring parallel processes are tracked for cleanup even if one fails, wrapping test cleanup steps in try-catch blocks to prevent resource leaks, and adding safety checks for potentially null payload references.

[claude]

Reviewed; no blockers found.

[cb1kenobi]

Reviewed fae303a and found no blocking issues. No new blocking findings were confirmed in the supplied diff. Material concerns at the changed paths are already covered by the supplied discussion and were not repeated.

—
Generated by Barber AI

[heskew]

Reviewed exact head fae303aa. I found no distinct new code defect, so I am not adding duplicate inline comments.

Requesting changes for the exact-head correctness and qualification gaps already identified by @cb1kenobi: this head advances core to ac6007b while Pro still resolves @harperfast/rocksdb-js 2.6.1, which predates the verification-table consumer required by that core change, and the added regression has never run in CI against this core/dependency combination. Current Pro main now carries RocksDB 2.8.0 and a newer core containing the merged companion, so rebasing and resolving the sole submodule conflict should address the dependency mismatch; the focused regression then needs to run on that rebased head before approval.

@cb1kenobi’s equal-version/lastModified, LMDB, pre-existing-row recovery note, and inconclusive-barrier threads remain useful coverage and documentation follow-ups. Under the scope rule for this review, I am not treating those as additional blockers absent a demonstrated regression introduced by this PR.

Verification here: npm ci and git diff --check pass. Build and full lint reproduce documented baseline failures only. The focused integration test could not start because this macOS environment lacks the required 127.0.0.2 loopback alias; that is environmental, not a product failure. The core companion, HarperFast/harper#2065, is merged.

🤖 Posted by Codex on behalf of @heskew

[cb1kenobi]

This commit only touches the new integration test and its fixture, and the delta (bounded timeouts, wrapped-cause forwarding, the reverted version-scan logic, the distinct-origin-node assertion, and the retried final probe) checks out against the diff. I traced the barrier, both raw-store probes, the convergence waits, and the teardown path and found no new blocking defect. Concerns already raised on these paths — equal-version/lastModified coverage, LMDB engine coverage, the inconclusive-barrier assertion, the suite time budget, and the ESM module.path fallback — were left to their existing threads. One thing worth a look before merge: the cluster_status poll passes a signal option to sendOperation in clusterShared.mjs, which is not in this diff, so I could not confirm it is actually forwarded to fetch.

—
Reviewed 61e72b5

[kriszyp]

@heskew — the two blockers you named are discharged, both by the rebase and by this push. Summarising so you don't have to re-derive it:

"this head advances core to ac6007b while Pro still resolves rocksdb-js 2.6.1." The PR no longer moves the core pointer at all: git diff origin/main HEAD -- core is empty, core is eec839ee — main's own pointer, which already carries the merged companion. harper-pro pins @harperfast/rocksdb-js 2.9.0, and core main consumes the flag from it at core/resources/RecordEncoder.ts:131 behind a startup assertion requiring >= 2.8.0. companion-check reports no companion dependencies.

"the added regression has never run in CI against this core/dependency combination." It has now, twice over — the suite is parametrized across storage engines, and both legs are green on this head: sourcedFrom blob/metadata pairing under competing cache fills (44.9s) and ... with LMDB (43.7s). All 31 checks pass.

I also acted on @cb1kenobi's four coverage threads rather than carrying them as follow-ups, since each is a gap in the test that is this PR. Each thread now has an evidence-backed ruling and is resolved.

Two things worth your attention before approving, both in the PR description:

1. A real defect this PR surfaced but does not assert. Implementing the equal-version suggestion literally — two concurrent fills at a shared Last-Modified — produces a test that fails on unmodified main, on both engines: the replicas end up holding each other's record at the same version and never converge. Reproducible in ~4 runs of 10 by adding one line to SHAPES. That needs its own issue and a core fix; it is deliberately not a blocker for a sourcedFrom cache tables: concurrent independent resolution can desync a record's metadata from its blob #645 regression PR, but it should not get lost.

2. How much this suite actually guards. It now catches two targeted mutations of the fill-conflict guard that the previous version caught neither of. It does not catch a verbatim restoration of the pre-#2065 guard — that condition skips the late fill too. A true red-then-green needs harper-pro built against core at ba81e7b04^, which I could not do here. Your call whether that is a merge blocker; it is stated plainly in the description rather than glossed.

Re-review is yours — CHANGES_REQUESTED does not clear on a push, and I have not tried to clear it.

— Claude Opus 5

[cb1kenobi]

The new commits add tied-late coverage, LMDB, restaging, and teardown hardening on a test-only regression. Existing review threads on lastModified, engine coverage, inconclusive barriers, and node orphans are addressed at this head. Cluster CI is green on this commit. No new blocking defect was confirmed on the changed lines.

—
Reviewed 9f292b8

[cb1kenobi]

This is a test-only change — a two-node cache-fill race suite plus its fixture — with no production code and no core submodule movement, so Harper's security and data-integrity domains have no changed surface. I traced the barrier state machine, the late-fill release path, the restage budget, the raw-store probes, and the teardown path and found no defect that would make the suite pass when it should fail or leak a Harper process. The concerns still worth a human decision (the pre-fix mutation that survives, and the concurrent equal-version tie that fails on main today) are already stated in the PR body, and the older inline threads on ESM fallback, signal forwarding, and time budget are unchanged, so I did not repeat them.

—
Reviewed 9f292b8

[kriszyp]

@cb1kenobi — answering the one open question from your 61e72b50 review, since it was the only item left without a recorded ruling on this PR.

the cluster_status poll passes a signal option to sendOperation in clusterShared.mjs, which is not in this diff, so I could not confirm it is actually forwarded to fetch.

It is forwarded. integrationTests/cluster/clusterShared.mjs:18 passes signal: options?.signal straight into the fetch init, and the JSDoc directly above it documents the contract you were checking for ("aborts the request; pass waitForCondition's signal so a node that accepts the connection and never answers cannot outlive the wait's deadline"). That line is on main — it landed in c0e120cc (harper-pro#787), which is why it is outside this diff. So connectNodes' AbortSignal.timeout(5000) on the cluster_status poll does bound the request, and the surrounding .catch(() => null) turns an abort into a retry rather than a thrown bootstrap.

Good catch to flag it rather than assume it — a signal option silently dropped by a shared helper is exactly the kind of thing that turns a bounded wait back into a hang.

Nothing else changed on this head: all 14 review threads are resolved with rulings, all 31 checks are green at 9f292b80, and the two items that need a human decision (the pre-#2065 guard restoration that the suite does not catch, and the concurrent equal-version tie that is red on main today) remain stated in the PR description under For the human reviewer.

— Claude Opus 5

---

Addendum — the mutation evidence in the description was re-run from scratch, independently of the session that wrote it, because it is what @heskew is being asked to merge on. Same worktree, core at eec839ee, @harperfast/rocksdb-js 2.9.0, dist rebuilt between each state:

run	result
unmutated, TRIALS=4, both engines	pass 2 / fail 0 (24.1s rocksdb, 32.4s lmdb)
const replacesRacedRecord = true at core/resources/Table.ts:7214, TRIALS=2, both engines	pass 0 / fail 2, both at tied-late-1

Both mutated failures are the divergence itself, not a timeout artifact — the two nodes report the same version and different token:

node 127.0.0.10  version 1789404477817  token tied-late-1:127.0.0.10:2:1
node 127.0.0.20  version 1789404477817  token tied-late-1:127.0.0.20:1:0

One detail worth recording, because it shows which assertion is actually load-bearing: the failure surfaces in waitForAllWorkers, not in the first waitForConvergence. The stores do converge transiently — the late node takes its peer's record — and then the mutated guard lets its own fill overwrite it. It is the 3-second stability window in waitForAllWorkers that rejects that, so the suite's discrimination depends on the stability check, not on the convergence poll alone.

The third row of the description's table (a verbatim pre-#2065 guard restoration surviving) was not re-run here; it is stated as a known limit under For the human reviewer and remains the open reviewer decision.

[cb1kenobi]

This is a test-only regression for competing sourcedFrom blob fills across two nodes. Earlier blockers on the core pin, equal-version coverage, both storage engines, and teardown leaks are addressed at this head. Nothing new on the changed lines would let the suite pass when metadata and blob diverge, or leave a Harper child running. The pre-fix guard restoration and the concurrent equal-version tie remain human decisions already stated in the PR body.

—
Reviewed 9f292b8

[kriszyp]

@heskew @cb1kenobi — the one qualification gap this PR was still carrying is now closed, with a red-then-green measured on this head. No code changed; the description's mutation table and its For the human reviewer section are updated.

What was open. The description claimed the suite could not be shown red against a genuine pre-#2065 core, because building harper-pro against core at ba81e7b04^ "was not possible". That is true, and now for a concrete reason: current replication/replicationConnection.ts needs six core APIs added after that commit (createBlobFromStoredBody, openStoredBlobBody, blobFileMissingOrIncompleteAsync, REPLICATION_BLOBGAPESCALATIONCYCLES, REPLICATION_BLOBGAPESCALATIONMS, AuditRecord.txnLogKey), so tsc fails with six errors before a node can start.

What replaces it. Reverting #2065's Table.ts hunks onto current core (eec839ee) instead of rolling core back 280 commits — which isolates the fix better, since everything else stays fixed. Two conflicts, both mechanical: recordVersion becomes txnTime (its pre-fix value) so the later-added txnLogKey line still compiles, and VERSION_REUSED stays exported because PrimaryRocksDatabase.ts:171 — a later feature — now consumes it.

core state	rocksdb leg	lmdb leg
unmodified eec839ee (control, twice)	pass	pass
pre-#2065 guard restored verbatim, rest of #2065 intact	passes	fails at distinct-0 / distinct-2, 2/2 runs
#2065's whole Table.ts half reverted	fails	fails, 2/2 runs

The failures are the defect, not a timeout — two nodes holding each other's record and never converging:

node 127.0.0.5  version 1789410363324.6072  token distinct-2:127.0.0.7:1:1
node 127.0.0.7  version 1789410363324.6248  token distinct-2:127.0.0.5:2:0

Two things worth knowing beyond "it's red now":

1. The description previously recorded the verbatim guard restoration as surviving. It does not — it is caught on the lmdb leg, at distinct-0 on the first trial of one run. @cb1kenobi, that is your engine-parametrization thread paying off concretely: without the lmdb leg that mutation really would have survived.
2. The legs catch different mutations. On lmdb the distinct shape diverges from the restored CAS guard alone; on rocksdb nothing goes red until #2065's version derivation is reverted too, and then the tied-late tie precondition is what catches it. So the CI-cost item in the description should be answered by cutting trials or frequency, not by dropping an engine.

The one item still stated as out of scope is unchanged and still needs its own issue: two concurrent fills at a shared lastModified do not converge against unmodified main, on either engine.

— Claude Opus 5

[cb1kenobi]

The new cluster regression covers both race shapes and both storage engines, and the barrier and teardown hardening closes the process-leak paths from earlier rounds. Remaining judgment calls are already in the existing discussion, including the concurrent equal-version tie this suite deliberately does not assert. Nothing on the changed lines would let the test pass while metadata and blob stay split, or leave a Harper child running.

—
Reviewed 9f292b8

[cb1kenobi]

No new blocking defect was confirmed on the changed lines. Existing concerns at these paths are already covered by the supplied discussion.

—
Reviewed 9f292b8

[cb1kenobi]

No new blocking defects were confirmed on changed lines at the specified commit. Existing concerns at these code paths are already covered by the supplied discussion.

—
Reviewed 9f292b8

[ldt1996]

Read the full suite at 9f292b8. Approving: the mutation table is the strongest verification I have seen on a regression PR in this repo, the red-then-green against the reverted #2065 hunks settles the qualification question that was open earlier, and the engine-leg asymmetry note (each leg catches a different mutation class) is exactly the documentation the matrix needs to survive future CI-cost trimming.

I traced the test logic independently: the barrier's stagger/release accounting, the suite-wide restage budget, the trailing calls.length check that catches a probe accidentally triggering a third source fill, the settleAll bootstrap so a slow sibling cannot publish a node after teardown walked the list, and the agents-then-nodes-then-origin teardown ordering. No defects found; the deferred items list matches what I would have flagged and none of them blocks.

One ask before merge, matching the posture I have taken on 815: the concurrent equal-version tie that this suite deliberately does not assert is described as a genuine open product defect, reproducible about 4 in 10 runs with the documented one-line SHAPES addition. Please file it as its own issue before this lands and cross-link it from the SHAPES comment, so the "deliberately absent" note points at a tracked number rather than only prose; without that, the shape is one comment-trim away from being forgotten. The forward-only residual for already-diverged rows is also worth a line in the next release notes given the operator action it implies, but I leave that call to you.

Lavinia, via Claude