[jilio]

Applications can keep ordinary writes asynchronous while waiting for selected writes to reach an independent replica. Replicator reuses Mirror's ordered copy loop and adds source-prefix barriers: Position scopes an append result, Capture resolves a fixed tail, Wait waits for destination acknowledgement plus a saved checkpoint, and Confirmed reports progress. Confirmed copying uses batched Read even for tail-capable sources so empty-chunk cursor advances remain visible; explicit waits wake idle polling without bypassing error backoff.

Positions and checkpoints are scoped to an explicit relationship ID and log generation. Restart resumes the same checkpoint namespace; replacement logs require a new generation. Unfulfilled waiters wake when Run stops, canceled waiters do not stop copying, and failed later checkpoint saves retry in place without advancing confirmation. Plain Mirror retains its existing policy. Automatic rewind reset is rejected for confirmed replication.

A successful wait inherits the destination's storage acknowledgement guarantees. It does not provide writer fencing, snapshot transfer, destination-projection application, or protection from a destination that acknowledges before durable storage. Source history must remain intact; recovery must tolerate duplicate IDs.

Durable Streams gains opt-in WithStrictDecoding() so a replication source cannot silently skip malformed envelopes. README, distributed/persistence/examples guides, a new Confirmed Replication guide, API comments, an executable example and the v0.19.0 changelog cover setup, hybrid policy, timeout/retry, generations and recovery constraints.

Validation:

• All six workspace modules: race tests and vet passed.
• Core/state: 100% statement coverage, including standalone Go 1.24.2.
• Replication/cancellation scenarios passed 10 race repetitions: whole-prefix waiting, concurrent waiters, ambiguous append acknowledgement, checkpoint outage, restart/redelivery, generation isolation, invalid history, chunk boundaries, empty-chunk advances and wakeup of an idle copier.
• A subprocess smoke test confirms 201 revisions over HTTP into SQLite, kills the source process, deletes its files, and verifies the confirmed prefix from the reopened replica. The workspace CI now runs this test on Linux.
• A deterministic async-capacity cancellation regression test exercises both live and durable Once rollback, removing the scheduler-dependent coverage gap observed during the preceding shutdown work.

Release target: v0.19.0. Submodule requirements/tags will be aligned once the root tag is published, following the repository's existing multi-module release sequence.

[claude]

Review

I read through replication.go, the mirror.go changes, the durablestream strict-decoding change, and the new tests, and traced the concurrency/state-machine logic by hand (the confirm/wait channel handshake, the checkpoint-retry loop in advance, restart/generation isolation, and the chunk-boundary/ambiguous-append edge cases). CI is green across all workspace modules, including go vet, gofmt -l, coverage, and the new process-loss smoke test, so I focused on correctness beyond what CI already checks.

Code quality / design

Overall this is a careful, well-scoped design:

• Reusing Mirror's ordered copy loop via the internal mirrorRun/cfg.progress hook (mirror.go:18, mirror.go:209) avoids duplicating the read/forward/checkpoint machinery, and plain Mirror callers are provably unaffected (the advance branch for cfg.progress == nil preserves the exact prior behavior).
• The confirm/changed-channel broadcast pattern in Replicator (replication.go) is a clean way to let multiple independent waiters block without polling, and the mutex-protected snapshot-then-select in Wait avoids the classic "missed wakeup" race.
• The docs (docs/REPLICATION.md) are unusually thorough about what a "confirmed" guarantee does not mean (no fencing, no projection-apply guarantee, no snapshot transfer) — this is exactly the kind of thing that gets over-promised in replication APIs, so I'm glad it's spelled out.

Potential bugs

I didn't find a functional bug. A few things I specifically checked and believe are correct:

• advance()'s checkpoint-retry loop blocks in place on save failure for Replicator (never returning to the copy loop), so Confirmed() can't advance past an unsaved checkpoint even though the destination append already landed — matches TestReplicationCheckpointFailureCannotConfirmOrSkip.
• Run's defer that sets stopped/runErr and closes changed executes strictly after mirrorRun returns, and confirm() only runs from within that same call, so there's no concurrent double-close of the channel.
• The WithStrictDecoding null-checks in stores/durablestream/store.go are necessary, not defensive noise: json.Unmarshal of a literal null into a non-pointer struct succeeds silently in Go, so without the explicit "null" check a poison record would decode as a zero-value event instead of erroring.

Minor nits (non-blocking)

• CHANGELOG.md:23-24 has a double blank line between the ### Added and ### Fixed sections (every other section boundary in the file uses one blank line).
• replication.go:43 has one comment line that's much longer than its neighbors ("...retain all history not yet replicated. A fresh generation starts at OffsetOldest and requires the") — looks like a merge artifact rather than intentional wrapping; gofmt won't catch this since it doesn't rewrap comments.

Test coverage

The test suite is notably thorough for a concurrency-heavy feature: whole-prefix barriers, independent concurrent waiters, ambiguous-append retry without false confirmation, checkpoint-outage retry, restart/generation isolation, chunk-boundary non-confirmation (both poll and tail paths), and history-read failures. The replication-smoke.go script adds real process-kill/file-deletion validation over HTTP+SQLite, which is a good complement to the in-process tests.

Security

Nothing concerning. WithStrictDecoding is opt-in and backward compatible; the smoke test's HTTP handler is a local httptest.Server only used for the test itself, not production code.

Nice work — this is thorough and the documentation of the guarantee boundaries is a real strength.

[claude]

Review: Confirmed replication barriers (Replicator)

I read through replication.go, the mirror.go changes it hooks into, the durablestream strict-decoding addition, and the accompanying tests/docs. This is a large, carefully-built feature with unusually thorough self-testing. Summary below.

Correctness

I traced the concurrency handshake between Wait/confirm/Run closely, since that's where a feature like this usually hides bugs:

• The changed-channel broadcast pattern (confirm() closes-and-replaces under r.mu, Run's defer closes the final one on exit) is safe: confirm() is only ever invoked synchronously from within Run's own goroutine (via mirror.advance/initialize), so it never races with the defer's close — no double-close, no lost wakeups.
• mirror.advance's split behavior for cfg.progress != nil (don't set m.offset until the checkpoint save actually succeeds, retry the save in place on failure instead of skipping forward like plain Mirror does) correctly prevents Confirmed()/Wait from ever reporting an offset that isn't durably checkpointed. I checked this against TestReplicationCheckpointFailureCannotConfirmOrSkip and TestReplicationCancelWhileCheckpointIsUnavailable and the implementation matches the documented at-least-once/no-skip guarantee, including the redelivery-after-failed-checkpoint case.
• Chunk-boundary handling (TestReplicationChunkBoundaryNeverConfirmsUnfinishedSuffix) is correct: non-last members of a chunk that share a lower "safe redelivery" offset are never confirmed via the monotonic CompareOffsets guard, only the concrete boundary is.
• Forcing Replicator onto the poll path even for EventStoreTailer sources (mirrorRun's cfg.progress == nil guard) is well-justified — Tail's iterator can't expose a cursor-only advance across an empty chunk, so batched Read is required to confirm that case at all (TestReplicationConfirmsEmptyResumeBoundaryFromTailCapableSource).
• Wait correctly supports starting before Run, independent concurrent waiters, and cancellation without affecting other waiters or in-flight copying — the wake-then-select-on-snapshot pattern avoids holding r.mu across a blocking comparer.CompareOffsets call or the channel wait.

I did not find a correctness bug in this logic. The new EventStoreOffsetComparer doc requirement ("must be safe to call concurrently") is real and necessary given Wait calls it outside the lock from arbitrary goroutines — worth double-checking any third-party/custom store implementations against that, since the bundled stores (memory/sqlite/durablestream) are pure/stateless there already.

durablestream WithStrictDecoding

Good catch in the implementation: a top-level null JSON body unmarshals into []json.RawMessage as a nil slice with no error, and a null individual envelope unmarshals into the target struct as a no-op success (zero-value, not an error) — both would silently look like "zero events, chunk complete" without the explicit strings.TrimSpace(...) == "null" checks added here. That's exactly the silent-data-loss failure mode this option exists to close for replication sources, and the tests (TestStrictDecodingDoesNotSkipPoisonRecords, TestStrictDecodingRejectsNullChunk) target it directly.

Minor / non-blocking observations

• advance()'s checkpoint-save retry loop (Replicator path) retries indefinitely at pollInterval cadence with no backoff growth. That matches the documented "retry in place" contract, but combined with a very small MirrorPollInterval (tests use 1ms) a persistently-down checkpoint store would retry very quickly. Probably fine in practice (it's still bounded by real I/O latency to the checkpoint store), but might be worth a line in REPLICATION.md's failure table about choosing MirrorPollInterval with checkpoint-store retry cost in mind.
• Small prose nit in replication.go's ReplicatorConfig doc comment: "...A fresh generation starts at OffsetOldest and requires the full source history. Source and destination must be different logs with no" wraps mid-thought onto the next line without a paragraph break — easy to miss when skimming.
• Position() re-validates offset >= OffsetOldest via the comparer on every call, including internally from Wait() on every retry iteration. Not a bug, just a small redundant comparer call in a hot retry path if Wait is polling a slow-to-confirm target — negligible given comparers here are all cheap/local.

Test coverage & CLAUDE.md checklist

I was not able to execute go fmt, go vet, go test ./..., or coverage tooling directly in this review sandbox, so I can't independently confirm the PR description's coverage/race-testing claims. That said, replication_test.go (683 lines) is unusually thorough for a first pass — it covers the wait/confirm handshake, ambiguous/lost-acknowledgement appends, checkpoint outages, restart + generation isolation, chunk boundaries, comparator failures, and idle-wake — and I didn't spot a scenario from the design doc that's missing a test. Please make sure CI actually runs the required go fmt/go vet/go test -race/coverage steps per CLAUDE.md before merge, and that the new .github/workflows/test.yml smoke-test step (go run ./scripts/replication-smoke.go) is confirmed green.

Security

No concerns. WithStrictDecoding is a security-adjacent hardening (prevents silent acceptance of malformed data in a replication/audit path). The smoke-test script only talks to an in-process httptest.Server, not a real network endpoint.

Overall: solid, well-documented feature with matching test depth. Nothing here blocks merging on correctness grounds from my read; the notes above are polish-level.

[claude]

Review

I read through replication.go, the mirror.go changes it builds on, the durablestream strict-decoding addition, and the new test suites (replication_test.go, async_capacity_test.go, strict_decode_test.go). Note: I wasn't able to execute go build/go test/go fmt in this sandbox (tool approval for go/git was unavailable), so this is a static read-through rather than a verified run — please make sure CI is green before merge.

Code quality

• The design is clean: Replicator reuses mirrorRun/advance rather than forking the copy loop, and the cfg.progress != nil branches are narrow and well-isolated from plain Mirror's existing behavior.
• The changed/wake channel pattern in replication.go (close-and-replace under r.mu for broadcast, buffered-1 non-blocking send for wakeup) is race-safe — I traced the lock ordering between confirm(), Wait(), and Run()'s deferred close and didn't find a double-close or missed-wakeup path.
• advance()'s monotonic-checkpoint guard is now applied even on the very first event for Replicator (m.offset != OffsetOldest || m.cfg.progress != nil), which is a deliberate and correct tightening — it lets a comparer failure on the first event surface as a Wait/Run error instead of silently falling through.
• Doc comments (persist.go, mirror.go, docs/REPLICATION.md) are thorough about the destination-durability caveat, which is the easiest part of this feature to misuse — good call spelling out that a successful Wait only inherits whatever Destination.Append actually guarantees.

Possible bugs / edge cases

• Nothing I'd call a correctness bug. One behavior worth double-checking against intent: in Wait, an already-canceled/expired ctx returns the context error before checking whether target is already confirmed (the ctx.Err() check happens before the ready/cmp check on every loop iteration). So Wait(alreadyCanceledCtx, alreadyConfirmedPosition) returns context.Canceled rather than nil. This matches the test in TestReplicationCaptureIsFixedAndWaitersIndependent, so it's clearly intentional, but the REPLICATION.md table doesn't call out this ordering explicitly — might be worth a one-line note so callers don't read "already confirmed" as always short-circuiting.
• The checkpoint-save retry loop in advance() for confirmed replication has no backoff cap/circuit breaker — it retries every pollInterval forever while the checkpoint store is down. That matches the documented policy ("retry in place, don't advance"), but with a very small MirrorPollInterval (as used in several tests) a real outage could retry quite fast. Not a bug, just worth confirming that's the desired production behavior vs. eventually escalating.

Performance

• Replicator always uses batched Read instead of Tail, even for tail-capable sources — correctly justified in the comments (Tail can't expose empty-chunk cursor advances), but it does mean confirmed replication gives up push-based delivery latency in favor of poll-interval latency. That's an inherent tradeoff of the feature, not a defect, but worth calling out in the PR description/README as a tradeoff versus plain Mirror for anyone choosing between the two.

Security

• No concerns. The new WithStrictDecoding() option is a good defensive addition for a security/data-integrity-adjacent property (malformed envelopes on a replication source no longer disappear silently). The smoke test script only binds an in-process httptest server and temp directories, nothing networked externally.

Test coverage

• Very thorough: whole-prefix waiting, independent concurrent waiters, ambiguous append acknowledgement, checkpoint outage (both mid-run and startup), restart/redelivery, generation isolation, chunk-boundary confirmation (poll and tail-capable source shapes), history-gap read errors, and comparer-failure propagation at each of the distinct call sites (advance, Wait, checkpoint compare) are all separately exercised. The async_capacity_test.go addition is a nice, minimal, deterministic reproduction of the cancel-during-capacity-wait rollback path (pre-filling asyncSem to force the ctx.Done() branch in select is a good trick to avoid a flaky scheduler-dependent test).
• One gap: I didn't see a test asserting the "later checkpoint save fails, then destination append also fails on retry" combination (i.e., failure stacking rather than one failure mode at a time), though this is a fairly deep edge case and the individual failure modes are each well covered.

Overall this looks solid and unusually well-documented for a feature with sharp edges (durability inheritance, generation scoping, at-least-once redelivery). Nice work.