Fix memory journal returns the last remote sequence instead of the next

[fubhy]

Summary

After importing remote sequence n, the memory journal returns n as the next synchronization start instead of n + 1. Sequence zero is especially indistinguishable from the initial empty state.

Important

This PR includes focused regression coverage and the memory-journal implementation fix.

Memory journal returns the last remote sequence instead of the next

Module: effect/unstable/eventlog/EventJournal
Audit ID: effect-2f6a0458baf5d1fe
Severity / confidence: high / high

What happens

After importing remote sequence n, the memory journal returns n as the next synchronization start instead of n + 1. Sequence zero is especially indistinguishable from the initial empty state.

Why it happens

Each memory remote stores a sequence initialized to zero; imports update it to the greatest observed remoteSequence, and nextRemoteSequence returns that field unchanged. The SQL implementation computes MAX(sequence) + 1 and the IndexedDB implementation reads the greatest sequence and adds one, confirming that this state is a cursor for the next request rather than the last value itself.

Expected behavior

nextRemoteSequence(remoteId) must return the first unused sequence for that remote: zero when no remote entries are recorded, otherwise one greater than the maximum recorded sequence.

Relevant implementation

These links and excerpts are pinned to audit base 17f0b91a243ccfe4a38d27debdc983adf434e738.

• packages/effect/src/unstable/eventlog/EventJournal.ts:417-481

View problematic code at packages/effect/src/unstable/eventlog/EventJournal.ts:417-466

      for (const remoteEntry of options.entries) {
        if (byId.has(remoteEntry.entry.idString)) {
          duplicateEntries.push(remoteEntry.entry)
          if (remoteEntry.remoteSequence > remote.sequence) {
            remote.sequence = remoteEntry.remoteSequence
          }
          continue
        }
        uncommittedRemotes.push(remoteEntry)
        uncommitted.push(remoteEntry.entry)
      }

      const compacted = options.compact
        ? yield* options.compact(uncommittedRemotes)
        : uncommitted

      for (const originEntry of compacted) {
        const entryMillis = entryIdMillis(originEntry.id)
        const conflicts: Array<Entry> = []
        for (let i = journal.length - 1; i >= -1; i--) {
          const entry = journal[i]
          if (entry !== undefined && entry.createdAtMillis > entryMillis) {
            continue
          }
          for (let j = i + 2; j < journal.length; j++) {
            const scannedEntry = journal[j]!
            if (scannedEntry.event === originEntry.event && scannedEntry.primaryKey === originEntry.primaryKey) {
              conflicts.push(scannedEntry)
            }
          }
          yield* options.effect({ entry: originEntry, conflicts })
          break
        }
      }
      for (const remoteEntry of uncommittedRemotes) {
        journal.push(remoteEntry.entry)
        byId.set(remoteEntry.entry.idString, remoteEntry.entry)
        if (remoteEntry.remoteSequence > remote.sequence) {
          remote.sequence = remoteEntry.remoteSequence
        }
      }
      journal.sort((a, b) => a.createdAtMillis - b.createdAtMillis)
      return {
        duplicateEntries
      }
    }),
    withRemoteUncommited: (remoteId, f) =>
      Effect.acquireUseRelease(
        Effect.sync(() => ensureRemote(remoteId).missing.slice()),
        f,

View exact lines on GitHub

Excerpt truncated. Open the complete packages/effect/src/unstable/eventlog/EventJournal.ts:417-481 range.

Reproduction

pnpm test --run packages/effect/test/unstable/eventlog/EventJournal.test.ts

Validation: The focused contract assertion fails against 17f0b91 and passes with this fix.

Implementation

The memory journal now stores the first unused remote sequence after both new and duplicate remote imports. The regression test covers the empty state, sequence zero, and a non-zero duplicate sequence.

Validated with:

pnpm test --run packages/effect/test/unstable/eventlog/EventJournal.test.ts

Audit provenance

• Audit base: 17f0b91a243ccfe4a38d27debdc983adf434e738
• Reproduction base: 17f0b91a243ccfe4a38d27debdc983adf434e738
• Findings: effect-2f6a0458baf5d1fe
• Initial patch: focused reproduction tests
• Final patch: implementation fix, expanded coverage, and changeset

Closes EFF-516

[changeset-bot]

🦋 Changeset detected

Latest commit: e65b8e3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages

Name	Type
effect	Patch
@effect/ai-anthropic	Patch
@effect/ai-openai	Patch
@effect/ai-openai-compat	Patch
@effect/ai-openrouter	Patch
@effect/atom-react	Patch
@effect/atom-solid	Patch
@effect/atom-vue	Patch
@effect/docgen	Patch
@effect/doctest	Patch
@effect/openapi-generator	Patch
@effect/opentelemetry	Patch
@effect/platform-browser	Patch
@effect/platform-bun	Patch
@effect/platform-deno	Patch
@effect/platform-node	Patch
@effect/platform-node-shared	Patch
@effect/sql-clickhouse	Patch
@effect/sql-d1	Patch
@effect/sql-libsql	Patch
@effect/sql-mssql	Patch
@effect/sql-mysql2	Patch
@effect/sql-pg	Patch
@effect/sql-pglite	Patch
@effect/sql-sqlite-bun	Patch
@effect/sql-sqlite-do	Patch
@effect/sql-sqlite-node	Patch
@effect/sql-sqlite-react-native	Patch
@effect/sql-sqlite-wasm	Patch
@effect/vitest	Patch

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

[pullfrog]

Important

This PR adds a focused regression test but does not yet include the memory-journal implementation fix. Please add the fix before merging.

Reviewed changes

• Added a regression test in packages/effect/test/unstable/eventlog/EventJournal.test.ts asserting that nextRemoteSequence returns 1 after importing a remote entry with remoteSequence: 0.

The test correctly fails against the current memory implementation (packages/effect/src/unstable/eventlog/EventJournal.ts:481 returns remote.sequence unchanged, while the SQL and IndexedDB implementations both add one). I ran the focused reproduction command and confirmed the failure: AssertionError: expected +0 to equal 1.

Before this can land:

1. Apply the memory-journal fix: maintain remote.sequence as the next unused sequence by storing remoteEntry.remoteSequence + 1 in writeFromRemote (both the duplicate and uncommitted branches), so nextRemoteSequence can return it unchanged.
2. Update the nextRemoteSequence JSDoc (packages/effect/src/unstable/eventlog/EventJournal.ts:89) from "last known sequence number" to "first unused sequence number".
3. Expand the test to cover the initial 0 state and a non-zero sequence.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

✅ No new issues found.

Reviewed changes

• Applied the memory-journal implementation fix: writeFromRemote now stores remoteEntry.remoteSequence + 1 in both the duplicate and uncommitted branches, so remote.sequence always tracks the first unused remote sequence.
• Updated the nextRemoteSequence JSDoc to describe the returned value as the first unused sequence.
• Expanded the regression test to assert the initial empty state returns 0, a remoteSequence: 0 import returns 1, and a non-zero import (5) returns 6.
• Added a patch changeset for effect.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[github-actions]

Bundle Size Analysis

Generated from PR build output; treat the content below as untrusted.

File Name	Current Size	Previous Size	Difference
basic.ts	7.13 KB	7.13 KB	0.00 KB (0.00%)
batching.ts	9.92 KB	9.92 KB	0.00 KB (0.00%)
brand.ts	6.40 KB	6.40 KB	0.00 KB (0.00%)
cache.ts	10.78 KB	10.78 KB	0.00 KB (0.00%)
config.ts	20.79 KB	20.79 KB	0.00 KB (0.00%)
differ.ts	20.37 KB	20.37 KB	0.00 KB (0.00%)
http-client.ts	21.59 KB	21.59 KB	0.00 KB (0.00%)
logger.ts	10.90 KB	10.90 KB	0.00 KB (0.00%)
metric.ts	9.05 KB	9.05 KB	0.00 KB (0.00%)
optic.ts	7.25 KB	7.25 KB	0.00 KB (0.00%)
pubsub.ts	15.05 KB	15.05 KB	0.00 KB (0.00%)
queue.ts	11.72 KB	11.72 KB	0.00 KB (0.00%)
schedule.ts	10.89 KB	10.89 KB	0.00 KB (0.00%)
schema-class.ts	19.33 KB	19.33 KB	0.00 KB (0.00%)
schema-fromJsonSchemaDocument.ts	29.15 KB	29.15 KB	0.00 KB (0.00%)
schema-representation-roundtrip.ts	25.47 KB	25.47 KB	0.00 KB (0.00%)
schema-string-transformation.ts	13.48 KB	13.48 KB	0.00 KB (0.00%)
schema-string.ts	11.01 KB	11.01 KB	0.00 KB (0.00%)
schema-template-literal.ts	15.27 KB	15.27 KB	0.00 KB (0.00%)
schema-toArbitraryLazy.ts	22.11 KB	22.11 KB	0.00 KB (0.00%)
schema-toCodeDocument.ts	24.50 KB	24.50 KB	0.00 KB (0.00%)
schema-toCodecJson.ts	19.34 KB	19.34 KB	0.00 KB (0.00%)
schema-toEquivalence.ts	19.17 KB	19.17 KB	0.00 KB (0.00%)
schema-toFormatter.ts	19.03 KB	19.03 KB	0.00 KB (0.00%)
schema-toJsonSchemaDocument.ts	22.75 KB	22.75 KB	0.00 KB (0.00%)
schema-toRepresentation.ts	19.66 KB	19.66 KB	0.00 KB (0.00%)
schema.ts	18.57 KB	18.57 KB	0.00 KB (0.00%)
stm.ts	12.69 KB	12.69 KB	0.00 KB (0.00%)
stream.ts	9.86 KB	9.86 KB	0.00 KB (0.00%)

[pullfrog]

✅ No new issues found.

Reviewed changes

Since the prior pullfrog review at d7602af, the branch was rebased onto a newer main and the regression coverage was restructured.

• Extracted the sequence assertions into a dedicated it.effect("returns the next unused remote sequence") that asserts the empty state returns 0, a remoteSequence: 0 import returns 1, and a duplicate remoteSequence: 5 import returns 6.
• Verified the memory-journal implementation fix (remoteEntry.remoteSequence + 1 in both duplicate and uncommitted branches), the nextRemoteSequence JSDoc update, and the patch changeset remain unchanged after the rebase.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}