fix(#1910): a Conflict retry must rebase on state the owner has not refused by rbuergi · Pull Request #1961 · Systemorph/MeshWeaver

rbuergi · 2026-08-20T22:48:08Z

Fixes #1910.

The issue's remedy was already implemented — that is the finding

#1910 suggests "adding a retry-with-refresh loop for concurrency conflicts, or switching to an append-only/event-sourced pattern". The loop exists. UpdateRemote re-enqueues on Conflict (bounded at 2), and it re-runs the update lambda rather than re-posting the precomputed patch — the #1814 defect, already fixed.

What it did not do is guarantee the re-run sees anything different.

attempt 0: read mirror.Take(1) → v10 → lambda → patch{accessCount:6, lastAccessedAt:T1, lastModified:T1}
                                        base{accessCount:5, lastAccessedAt:T0, lastModified:T0}
owner:     live is v11 (someone else's increment landed) → all 3 keys refused → nothing applied
           → NACK Conflict "3 field(s) changed … re-read and re-apply"
attempt 1: read mirror.Take(1) → still v10   ← the mirror has not received v11 yet
           → identical patch, identical base → refused identically
attempt 2: same
           → terminal MeshNodeStreamException → the activity write is dropped and logged as an error

The mirror is read at the instant the NACK arrives, which is exactly when it is most likely to still hold the version that was just refused. A retry whose input cannot change is not a retry — it is #1814 one level down: there the patch was re-sent, here the base is re-read from a source that had not moved.

(The "3 field(s)" in the incident is the shape's fingerprint: the diff of a UserActivity fold is content.accessCount, content.lastAccessedAt, and the lastModified stamp UpdateRemote adds — three leaves, all refused.)

The fix

A Conflict is the owner stating it is ahead of the writer's base: it minted a version for the write that beat us before it answered us. So the state that makes the re-attempt converge already exists and is in flight to this mirror. The re-attempt now waits for exactly that fact — a version strictly greater than the one refused — and rebases on it. That IS the "re-read and re-apply" the refusal asks for.

MeshNodeStreamHandle.RebaseSource — static, with the mirror and the scheduler as seams:

Conflict only. OwnerDisposing / OwnerNotReady mean the patch never reached a merge, so no newer version was minted; they pass 0 and are untouched, as is every first attempt (no filter, no timer, no second subscription on the ordinary write path).
Never parks. If the mirror does not advance within 5s, the re-attempt proceeds against what it has — exactly the previous behaviour — and logs STALE_MIRROR. It converges where the old shape could not and degrades to the old shape where it cannot, so it cannot be worse.
Not a backoff. It is the bound on one wait for an arrival, not an interval between repeats. The retry budget is unchanged — deliberately, since raising it would be the band-aid.

It also relieves the other way that budget gets spent: a loser no longer re-issues at the same instant as every other loser, it issues after the state that beat it, so a burst of concurrent writers serialises into a chain instead of re-stampeding.

Closed in the same change: filtering introduces a completion the unfiltered shape could not produce. A mirror that ends (its hub torn down) while holding only the refused version would complete the source empty — the write's observer never fires and the caller waits on a pipeline that has already finished. That now errors, naming the write as not landed.

Tests

6 deterministic tests in ActivityWriteSurvivesConflictTest — TestScheduler + a ReplaySubject mirror, no hub, no cluster, no sleep:

a Conflict retry waits for a version newer than the refused one, and rebases on it;
negative control: the old mirror.Take(1) shape, written out — it yields the very version the owner refused, so the regression above cannot go vacuous;
a mirror that never advances is not parked on, and the degradation is reported;
a first attempt reads immediately (the ordinary path costs nothing);
a non-staleness re-enqueue does not wait for a version that will never come;
a mirror that ends without newer state errors instead of completing empty.

Non-vacuity: with RebaseSource reverted to mirror.Take(1), 2 fail —
Expected collection to be empty because re-running the lambda against the very version the owner refused …, but found 1 item(s) and Expected collection to be empty because still inside the bound, but found 1 item(s).

What I could not reproduce, and why that matters

40 concurrent TrackActivityRequests for one path land all 40 increments in a monolith test, both with and without this change (3 runs each). In-process the cache's per-path serial queue plus sub-millisecond mirror propagation means the retry's re-read is fresh anyway. The end-to-end loss needs the production interleaving — the write queue advancing on UpdateRemote's optimistic 2s emit while a patch is still in flight, and a mirror that has not caught up — which is why the incident reads "1 occurrence on 1 pod for 1 user" rather than as a reproducible failure.

So the regression is pinned at the seam, with the old shape kept beside it as a control, rather than with a concurrency test that would be green on main by luck and flaky forever after.

🤖 Generated with Claude Code

…efused ActivityTracking dropped a user's navigation record: MeshNode Conflict at 'rbuergi/_UserActivity/rbuergi': cross-hub write refused: 3 field(s) changed on the owner since the writer's base and nothing was applied — re-read and re-apply The issue proposed "adding a retry-with-refresh loop, or switching to an append-only pattern". A retry-with-refresh loop was already there, and it already re-ran the update lambda rather than re-posting the patch — the #1814 defect, fixed. What it did NOT do is guarantee the re-run sees anything different. UpdateRemote rebuilt each attempt from `mirror.Take(1)`: whatever this hub's mirror holds at that instant. Immediately after a Conflict NACK that is very often still the version the owner just refused. The lambda then recomputes the same values from the same base, ExtractBaseValues emits the same base, and the owner refuses identically — three attempts, three identical patches, then a terminal MeshNodeStreamException and the caller's write is gone. A retry whose input cannot change is not a retry; it is #1814 one level down, with the base re-read from a source that had not moved instead of the patch being re-sent. Waiting for the newer state is sound rather than hopeful: a Conflict is the owner stating it is AHEAD of the writer's base, because it committed the write that beat us BEFORE it answered us. The state that makes the re-attempt converge already exists and is in flight to this mirror. So the re-attempt waits for exactly that fact — a version strictly greater than the one refused — bounded at 5s, and falls back to the previous behaviour (with a STALE_MIRROR warning) if it never arrives. It converges where the old shape could not and degrades to the old shape where it cannot. Only Conflict carries a stale base. OwnerDisposing / OwnerNotReady mean the patch never reached a merge, so no newer version was minted and waiting for one would burn the bound before every one of those retries; they pass 0 and are untouched, as is every first attempt. This also relieves the second failure mode without touching the retry budget: a loser no longer re-issues at the same instant as every other loser, it issues after the state that beat it, so a burst of concurrent writers serialises into a chain instead of re-stampeding. Closed in the same change: the filter introduces a completion the unfiltered shape could not produce. A mirror that ENDS while holding only the refused version would complete the source empty, the write's observer would never fire, and the caller would wait on a pipeline that had already finished. That now errors, naming the write as not landed. Measured, and worth recording: 40 concurrent TrackActivityRequests for one path land all 40 increments in a monolith test both with and without this change. The end-to-end failure needs the production interleaving (the cache queue advancing on UpdateRemote's optimistic 2s emit, slower mirror propagation), which is why the incident reads "1 occurrence on 1 pod" — so the regression is pinned deterministically at the seam, with the old shape kept beside it as a negative control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR hardens cross-hub MeshNodeStreamHandle.UpdateRemote optimistic-concurrency retries so a Conflict re-attempt rebases on mirror state the owner has not already refused, preventing “retry that can’t change its input” write drops (issue #1910).

Changes:

Add MeshNodeStreamHandle.RebaseSource(...) and thread the refused base version through UpdateRemote(...) so Conflict retries wait (bounded) for a strictly newer mirror version before re-running the update lambda.
Add deterministic tests (TestScheduler + ReplaySubject mirror seam) covering both the new rebase behavior and the pre-fix negative control.
Add a WhatsNew entry and add Microsoft.Reactive.Testing to the Query test project.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
test/MeshWeaver.Query.Test/MeshWeaver.Query.Test.csproj	Adds `Microsoft.Reactive.Testing` dependency used by the new deterministic conflict-rebase tests.
test/MeshWeaver.Query.Test/ActivityWriteSurvivesConflictTest.cs	New deterministic test suite validating conflict retry rebasing and edge cases (stale mirror bound, mirror completion).
src/MeshWeaver.Mesh.Contract/MeshNodeStreamExtensions.cs	Implements bounded rebase-on-newer-mirror logic for `Conflict` retries and threads refused base version through `UpdateRemote`.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-21-activity-survives-a-write-collision.md	Adds release note describing the user-visible effect of the conflict retry behavior change.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-08-20T23:01:22Z

Test Results (shard 3)

10 files ±0 10 suites ±0 4m 32s ⏱️ -24s
2 195 tests +1 2 194 ✅ +1 1 💤 ±0 0 ❌ ±0
2 652 runs +1 2 651 ✅ +1 1 💤 ±0 0 ❌ ±0