[rafalmaciag]

Fixes #28. A 1.3.7 change, not 1.3.6 (lead's ruling): it changes what a persisted snapshot carries.

The defect

Before #26 an aggregate's version was a count of the events the build could map. A snapshot taken by such a build recorded that count as its SnapshotVersion. Over a stream that held events the build did not know, the count is lower than the stream revision the snapshot covers. After upgrading, Get reads on from SnapshotVersion + 1, so it re-folds events already in the snapshot's state (a double apply).

Why the true revision can't be recovered from the number

The build that wrote the snapshot mapped its set of types. This build may map more: the classic case is a type that was unknown before the upgrade and known after it. So "count this build's mapped events up to SnapshotVersion + 1" lands in the wrong place. That's the plausible wrong fix, and it's a mutant below.

The fix

1. New snapshots carry a marker. AppendSnapshot writes SnapshotVersionIsRevision: true beside SnapshotVersion. Since Aggregate version counts the stream revision, not delivered events (#25) #26 the version it writes is the stream revision. GetSnapshot exposes the marker as Snapshot.VersionIsRevision.
2. An unmarked (legacy) snapshot is checked before use. A snapshot is appended after the events it covers.
   • If the event at SnapshotVersion + 1 was written before the snapshot, the snapshot may cover it, so it may be low. It is not used, and Get rebuilds the aggregate from the stream, which is always right.
   • Otherwise the event after the version came later (or there is none). Then the version is the revision, and the snapshot is used.
   • The cost falls on legacy snapshots only: two single-event reads per Get, until the policy writes a new, marked snapshot.
3. The order is the $all COMMIT POSITION of both records: the aggregate's event at version + 1 and the snapshot event. The comparison is PlumberEngine.WrittenAfter, later.Position > earlier.Position. No timestamp is used (rev-reach a3f8d0c32, m1).
   • A first version compared EventRecord.Created. That's the server's wall clock: a clock step back between the two writes would make a SHORT legacy snapshot look valid, the event would be re-folded, and the result would be the very state this PR fixes.
   • The two reads are SnapshotOrderRecords, exactly what Get's snapshot branch uses. The positions come from the server on stream reads.
   • The concurrent-writer case: another writer appending between a save and its snapshot reads as "may be low". The result is a needless rebuild, never a wrong state.
   • The marker key SnapshotVersionIsRevision sits un-namespaced beside SnapshotVersion (same family; an old reader keeps finding SnapshotVersion where it is). Only JSON true is trusted; anything else counts as unmarked, so it gets checked.

Compatibility both ways

• New build, old snapshot: checked as above.
• Old build, new snapshot: unaffected. The marker is an added metadata key, and the pre-marker reader (GetSnapshot at master 875ab10, lines 1305–1315) reads only Created and SnapshotVersion. Pinned by a row that reads a marked snapshot exactly that way.

Rows (Integration/SnapshotRevisionTests)

Test aggregate: TallyAggregate, a snapshotting aggregate whose state shows a re-fold: a count plus the marks seen, so a second b reads as "abb". It has a second event type, TallyNoted, for "a type a later release added".

Legacy snapshots: written through the real AppendSnapshot, then re-appended with only the marker removed (same type, data and metadata otherwise). Each row asserts VersionIsRevision == false before loading. Loading goes through Get, via DI (AddPlumberd): a snapshot policy is what makes Get read snapshots at all.

1. ALegacySnapshotOverAnUnknownEvent_DoesNotReFold. The stream is [a, (unknown), b], with a legacy snapshot {ab, 2} at version 1. Expected: "ab", Count 2, Version 2.
2. ALegacySnapshotOverATypeThisBuildNowMaps_IsRebuilt_NotPositionedByThisBuildsCount. The stream is [a, x:TallyNoted, b], with a legacy snapshot {ab, 2} at version 1 (the old build didn't map TallyNoted). Expected: "axb", Count 3, Version 2.
3. ALegacySnapshotWhoseNextEventCameAfterIt_IsUsed. A legacy snapshot that is RIGHT is still used, so not every library user pays a replay. The snapshot holds a sentinel {S, 100}; used → "Sb"/101, rebuilt → "ab".
4. ASnapshotThisBuildWrote_IsUsed_EvenWhenTheNextEventPrecedesIt. The marker is trusted where the order check alone would doubt: the concurrent-writer shape, with the sentinel state.
5. WrittenAfter_IsDecidedByCommitPosition_NotByCreated. A real store can't plant a clock step, so the seam is fed two records whose Created order contradicts their commit order: v+1 at position 100, stamped +1 h; the snapshot at 200. The row asserts the inverted-clock precondition first, then that v+1 is NOT "written after".
6. TheRecordsTheDecisionReads_CarryServerPositions_InCommitOrder. The seam's INPUT, through the real read path, on a real store: both records have CommitPosition > 0, neither is Position.Start, and v+1's position is below the snapshot's. Were a server to send 0, every legacy snapshot would be rebuilt silently; this row makes that red.
7. AMarkedSnapshot_ReadsThroughThePreMarkerReader_Unchanged. Old build, new snapshot: the event type, the data through the serializer, then Created and SnapshotVersion (an Int64), with the marker an extra key.

Mutants (each RED, message read)

• S0, the shipped behaviour (no check): rows 1 and 2. "abb" for "ab", and "abb" for "axb".
• S1, the plausible wrong fix (position a legacy snapshot by THIS build's mapped-event count): row 2, Expected tally.State.Seen to be "axb", but "abb". Row 1 is red too (Version 1 for 2).
• S2, the over-fix (rebuild every legacy snapshot): row 3, "Sb" expected, "ab" found.
• S3, the marker ignored (every snapshot checked as legacy): row 4, "Sb" expected, "ab" found.
• S5, "written after" by Created again: row 6, Expected PlumberEngine.WrittenAfter(next, snapshot) to be False because the event at v+1 committed BEFORE the snapshot … but found True.
• S6, one side fed Position.Start: row 7, Expected next.Position.CommitPosition to be greater than 0UL, but found 0UL, plus row 3 (the correct legacy snapshot rebuilt: "Sb" expected, "ab" found). That's the silent all-rebuild direction.
• S4, the marker nested into SnapshotVersion (the alternative shape that breaks older readers): row 5, "requires an element of type 'Number', but the target element has type 'Object'". The legacy rows also red on their marker precondition.

Suite

MicroPlumberd.Tests at the tip (5abbd6d, which merges origin/master 9bb51ed; no rebase): 208/214, 18 m 30 s, exit 1, no FATAL or crash, and no refused/409 line. It was the only MP process, under --blame-hang-timeout 4m, and no hang fired. The 6 failures are exactly the long-standing set:

• GrpcApiTests.ApiInvocationWithProcessorCorrelation, ApiHandingExceptionInvocation, ApiSuccessfulInvocation;
• ProcessManagerSourceGenerationTests.CommandTypes;
• SubscriptionRunnerStateTests.ThrowsWrongExpectedVersionExceptionWhenOnDoubleChange, …WhenStreamAlreadyExists.

Earlier runs, for the record:

• At 328c0dc (base 875ab10): 199/207. The extra 2, IdentitySeedTests AT06 and AT07, failed in 1 ms with the store-port 409, removal of container d67dadda… is already in progress. That base predates Testing: a store's port is held machine-wide; its container is acted on by id, never by name; another live process's container is refused (cross-process store race) #45, which fixed that race. Merging master brought Testing: a store's port is held machine-wide; its container is acted on by id, never by name; another live process's container is refused (cross-process store race) #45 in, and the re-run above is clean.
• One hung run at 9231d6d. It was killed at 25 min with no summary, so it isn't quoted. IndexBackedMergeReviewFollowupsTests left its store up; the cause is in TestAppHost.StartAsync's unbounded wait, fixed separately (bounded host start).

🤖 Generated with Claude Code