Recognize a re-delivered write from an origin with no log here, instead of applying and logging it again

[kriszyp]

A replicated write that is delivered again is recognized as a duplicate again, even when its origin has no transaction log on this node (#1162).

• RocksTransactionLogStore.put files such a write in the relaying peer's log (logById(nodeId) ?? logById(viaNodeId) ?? local).
• The keyed lookup that detects a re-delivered write read only the origin's log (log: nodeId). So it missed, and every re-delivery was applied and logged again: five identical relayed puts left five log entries.
• The retention floor that decides whether that lookup runs also read the origin's log. When the log was empty it fell back to the time-based floor, which could skip the lookup.

Now the write, the keyed lookup and the retention floor all select their log through one function, RocksTransactionLogStore.logFor(nodeId, viaNodeId). The lookup also matches the entry's origin, since a relay log holds several origins' entries and (txnLogKey, nodeId) identifies one. It never creates a log.

For the human reviewer

1. Requirement: fix this on the release lines, not only on main. On main, Keep a replicated transaction to one connection and one origin log, and hold a failed one for replay #2800 files every origin's entries in its own log, which removes this mismatch structurally. That is too large for a patch line. This PR is milestoned v5.1 so it also lands on v5.1 and v5.2. Stop re-logging a replicated delete that is re-delivered onto its own tombstone #2761 (re-delivered deletes) is being backported to both first; this PR's delete test depends on it there.

2. Other lookups still read the origin's log (known remaining gap). About eight other lookup sites know only a stored entry's origin, not the relay it came through. They can still miss an entry filed in a relay log:

   • the out-of-order walk;
   • resolveAuditHead;
   • the replacing-entry lookup in RecordEncoder;
   • crdt.ts.

   So can a lookup made after the origin's own log has been created, or after its writes switch to another relay. Fixing those needs the relay recorded with each entry, or per-origin logs (Keep a replicated transaction to one connection and one origin log, and hold a failed one for replay #2800). This PR fixes the two call sites that detect re-delivered writes and the floor that gates them. When Keep a replicated transaction to one connection and one origin log, and hold a failed one for replay #2800 lands, logFor takes over its per-origin routing.

3. When viaNodeId is omitted, the lookup now resolves to local, as the write does. It used to name the origin's numeric log, which read nothing when that log didn't exist. A lookup with no nodeId still reads across all logs, as before.

Changes

• resources/RocksTransactionLogStore.ts:
  • logFor(nodeId, viaNodeId) is shared by put and getSync;
  • getSync resolves the log through it by name, so it never creates one, and matches the entry's origin.
• resources/Table.ts: pass viaNodeId and read through logFor in:
  • the up-front keyed dedup;
  • the re-delivered-duplicate check;
  • the retention floor.

Verification

• New unitTests/resources/relayedDuplicateLookup.test.js. It delivers the same relayed write five times, from a named origin that has no log here, through a relay that has one:

  • a put is logged once (5 entries on unmodified main);
  • a delete is logged once;
  • a put is logged once when the time-based retention floor is past its log key (fails without the floor change);
  • the lookup creates no log for the origin.

  The put cases also fail on unmodified v5.1 and v5.2.

• unitTests/resources/auditWalkRetentionFloor.test.js: the head lookup now names the local log as 'local' instead of 0, and still asserts it doesn't aggregate.

• npm run test:unit:resources: 3048 passing. Three sourceApplyConflictRetry cases fail identically on unmodified main on the development machine.

• Lint and Prettier are clean on changed files.

Complexity: medium

🤖 Generated with Claude Code

Origin — the dispatch brief this PR was written from

Recognize a re-delivered write from an origin with no log here, instead of applying and logging it again

Maintain #2811 (Recognize a re-delivered write from an origin with no log here, instead of applying and logging it again) on branch kris/replicated-duplicate-apply. Read the current PR head, mergeability, and latest check runs before acting; the dispatch observation may be stale. First read the remote head for refs/heads/kris/replicated-duplicate-apply without updating local refs and require it to equal this task's observed head c1df5a7; if it differs, stop with needs-input because remote history changed after dispatch. Then fetch only refs/heads/main from origin into refs/remotes/origin/main. If Git reports a paused rebase for this task's exact generation branch, require that rebase's recorded original head to equal c1df5a7 and its recorded onto commit to equal the fetched origin/main tip; stop with needs-input on either mismatch. Only then resume it without repeating the local-HEAD ancestry check or starting another rebase. Otherwise record the remote SHA, verify it is an ancestor of local HEAD, and rebase onto the fetched base. If Context carries companion instructions, follow them exactly for every named gitlink; they override ordinary conflict handling. Preserve both sides' intent for every other conflict. Stop with needs-input on semantic conflicts. Force-with-lease is authorized only for this rebase. Immediately before pushing, re-read the PR's base ref and body plus every companion PR head/state named in Context. The base must still be main, and the declarations and submodule bindings must still match Context. If an open companion advanced, update its named gitlink to the new exact head and rerun relevant tests. If it merged, point the named gitlink at the companion repository's current default-branch tip after verifying that tip contains the merge, then rerun relevant tests. If it closed without merging, stop with needs-input and do not publish its abandoned head. If the base, declaration, binding or any other companion state changed ambiguously, stop with needs-input. Then push the rebased head with git push --force-with-lease=refs/heads/kris/replicated-duplicate-apply:EXPECTED_HEAD_SHA origin HEAD:refs/heads/kris/replicated-duplicate-apply, substituting the recorded SHA. If the remote head changes, stop with needs-input; never overwrite intervening remote work. Do this before evaluating CI. Then inspect CI for the resulting current PR head, not old-head failures. Wait for relevant pending checks, diagnose remaining failures, fix them, run relevant tests, and push in THIS task. Do not create a separate CI-fix task. Stop with needs-input for judgment calls or an unavailable CI result; report exactly what was verified. No merge is authorized.

_{Dispatch: task pr-maint-8fa822a1c27808fe71f38e3e52ae32b3 · queued by automation · ran by claude/sonnet/high · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=gemini,codex,cursor-kimi; adjudicated=domain; declined=cursor-grok,cursor-composer,cursor-muse; rounds=4; full=2 @ 3bb3522}

_{Human-Review-Need: 4 (decisions: lookup-fallback-for-mapped-origin, legacy-relay-entries, placement-rule-duplication, local-fallback-for-logless-origin, legacy-relay-log-dedup, undefined-nodeid-floor-log) @ 3bb3522}

[gemini-code-assist]

Code Review

This pull request updates the transaction log store to correctly handle relayed writes from origins without their own logs by introducing a logFor helper and passing viaNodeId to log lookups. It also adds a test suite to verify relayed duplicate lookups. The feedback suggests using loose equality (nodeId == null) instead of strict equality (nodeId === undefined) for null-or-undefined checks, adhering to the repository's style guide.

[gemini-code-assist]

According to the repository style guide, loose equality (== null / != null) is the intentional idiom for null-or-undefined checks in this codebase. Using strict equality with undefined (=== undefined) violates this convention and may fail to correctly handle null values. Please update this check to use loose equality against null.

Suggested change

	const log = nodeId === undefined ? undefined : this.logFor(nodeId, viaNodeId ?? nodeId).name;
	const log = nodeId == null ? undefined : this.logFor(nodeId, viaNodeId ?? nodeId).name;

References

1. Loose equality (== null / != null) is the intentional idiom for null-or-undefined checks in this repository. ^(link)
2. Use loose equality checks ('!= null' or '== null') for null-or-undefined checks. This complies with the style guide and prevents runtime errors (such as 'TypeError') when a value can be 'null' rather than 'undefined' at runtime.

[github-actions]

Release cherry-pick v5.1: cancelled

Cherry-pick branch cherry-pick/v5.1/pr-2811 was deleted — this PR was closed without merging.

[github-actions]

Release cherry-pick v5.2: cancelled

Cherry-pick branch cherry-pick/v5.2/pr-2811 was deleted — this PR was closed without merging.

[claude]

Reviewed; no blockers found.

[kriszyp]

Closing: on main, #2800 already files every origin's entries in its own transaction log, so the re-delivery lookup finds them without this change. The bug this fixed exists on the release lines, where it's going in directly (v5.1 and v5.2 PRs, with a cross-log fallback for re-deliveries that arrive through a different relay).

— Claude Opus 5.5