Fix conflict scanning indexes the wrong history array - #7029
Conversation
🦋 Changeset detectedLatest commit: 43e6c61 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Important
This PR currently contains only the reproduction test; the implementation fix it documents is not yet present. CI is expected to fail until toConflicts is fixed, so treat this review as feedback on the regression test and the path to the fix.
Reviewed changes
- Added
packages/effect/test/unstable/eventlog/UnencryptedEventLogConflictIndex.test.ts, a focused regression test that exercisesEventLogServerUnencrypted.processEntriesthrough theEventLog.WriteSingleRPC path.
@v0 or keep the SHA fresh with Dependabot | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Important
The incremental change moves and adapts the regression test, but the toConflicts source fix is still absent on this branch and the new test file is not formatted.
Reviewed changes
- Moved the reproduction from
UnencryptedEventLogConflictIndex.test.tsintoEventLogServerUnencrypted.test.tsand adaptedregisterHandlerUnsafeto the current{ event, handler }API shape. - Verified the test still fails against the unfixed implementation with the expected
[oldSameKey]vs[newerSameKey]mismatch.
⚠️ Source fix still missing and test file unformatted
The implementation fix for toConflicts is not present: packages/effect/src/unstable/eventlog/EventLogServerUnencrypted.ts:414 still reads history[j] instead of newHistory[j]. The new test file also fails pnpm dprint check — run pnpm dprint fmt on it before the next push.
The existing review thread on the line-106 assertion about also capturing originA's conflicts remains open and would make the regression test strictly stronger.
@v0 or keep the SHA fresh with Dependabot | Fix it ➔ | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Applied the one-line source fix in
packages/effect/src/unstable/eventlog/EventLogServerUnencrypted.tssotoConflictsscansnewHistory[j]instead ofhistory[j], matching the suffix it intended to examine. - Strengthened the regression test in
packages/effect/test/unstable/eventlog/EventLogServerUnencrypted.test.tsto assert conflicts for bothoriginA(expected[]) andoriginB(expected[newerSameKey.idString]), covering both the false-positive and false-negative sides of the bug.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Added
.changeset/tidy-cats-smile.mdwith a patch-level changeset describing the unencrypted event log conflict-scanning fix. - Removed the redundant
Effect.scopedwrapper from the regression test inpackages/effect/test/unstable/eventlog/EventLogServerUnencrypted.test.ts;it.effectalready appliesEffect.scopedinternally, so the wrapper only created an unnecessary child scope.
@v0 or keep the SHA fresh with Dependabot | View workflow run | Using Kimi K2 (free via Pullfrog for OSS) | 𝕏

Summary
The unencrypted server identifies the correct newer suffix of stored entries but scans an equally sized prefix when building conflicts. It can report entries older than the incoming event while omitting newer same-key entries from the end of storage.
Important
This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.
Conflict scanning indexes the wrong history array
Module:
effect/unstable/eventlog/EventLogServerUnencryptedAudit ID:
effect-64c8be87a9a44e92Severity / confidence: high / high
What happens
The unencrypted server identifies the correct newer suffix of stored entries but scans an equally sized prefix when building conflicts. It can report entries older than the incoming event while omitting newer same-key entries from the end of storage.
Why it happens
At insertion index
i,toConflictsassignsnewHistory = history.slice(i). The loop is bounded bynewHistory.lengthbut readshistory[j]instead ofnewHistory[j], so wheneveri > 0it examines prefix indices0..newHistory.length - 1rather than suffix indicesi..history.length - 1.Expected behavior
For each incoming entry, conflicts must contain all and only non-duplicate stored entries at or after its chronological insertion point whose event tag and primary key match the incoming entry.
Relevant implementation
These links and excerpts are pinned to audit base
17f0b91a243ccfe4a38d27debdc983adf434e738.packages/effect/src/unstable/eventlog/EventLogServerUnencrypted.ts:396-423View problematic code at
packages/effect/src/unstable/eventlog/EventLogServerUnencrypted.ts:396-423View exact lines on GitHub
Reproduction
pnpm test --run packages/effect/test/unstable/eventlog/UnencryptedEventLogConflictIndex.test.tsObserved failure: Focused contract assertion failed against 17f0b91, demonstrating: Conflict scanning indexes the wrong history array.
Implementation handoff
The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.
pnpm test --run packages/effect/test/unstable/eventlog/UnencryptedEventLogConflictIndex.test.tsAudit provenance
17f0b91a243ccfe4a38d27debdc983adf434e73817f0b91a243ccfe4a38d27debdc983adf434e738effect-64c8be87a9a44e92Closes EFF-469