fix(#3477): a "never applied" NACK's re-attempt stops diffing against its own phantom by rbuergi · Pull Request #3633 · Systemorph/MeshWeaver

rbuergi · 2026-09-07T16:35:59Z

The write was lost because the re-attempt believed the mirror

#3477 is a cause, established from the code and from its own transcript — not a widened bound,
not a retry, not a catalogued flake.

Two facts that meet

1. On the owner, the echo precedes the durable flush. DataExtensions answers a
PatchDataRequest in this order:

PATCH_MERGE_STAMPED v=N refused=0     ← the merge committed IN MEMORY
PATCH_ECHO_SEEN                       ← the change went out to every mirror
IPostCommitFlush.Flush(committed)     ← persist, THEN ack

So there is a real window in which every mirror carries a value no store holds.

2. A teardown fault on that flush leg is answered OwnerDisposing. ClassifyPatchException
maps every owner-side teardown fault there, deliberately (#3499) — the alternative, Unknown, is
TERMINAL and loses the write every time. Its own remarks name the trade and even name this issue:
"Routing more faults here makes that class MORE likely, not less, and it is silent when it
happens."

Therefore the population taking the OwnerDisposing re-enqueue arm is precisely the population
whose merge is already in the mirrors while storage is untouched.

What the re-attempt did with that

UpdateRemote's re-enqueue passes refusedBaseVersion = 0, which reduces RebaseSource to
mirror.Take(1). Its comment stated the intent exactly — "re-runs the caller's update FUNCTION
against whatever the fresh activation serves" — and the code read this hub's mirror. The
mirror already carried the write, so n => n with { Name = marker } was record-equal to its base,
the patch came out empty, nothing was posted, and the caller was completed as a success
carrying a marker no store held. Every line on that path is LogDebug.

The disjunction in that comment has three cases, not two:

on the owner	mirror shows it	storage holds it	empty re-diff is
merge never ran	no	no	— a real diff is computed; the write lands
merge committed and flushed, ack lost to teardown	yes	yes	correct
merge committed, flush died with the hub	yes	no	a lost write reported as success

Rows 2 and 3 are indistinguishable from the mirror. Only the owner can separate them.

Why this is the cause, not a story

The only sighting is LateNackReenqueueTest…ReenqueuesAndLands, 1/330, Plugins shard 3 run
34059060596. Its whole transcript after the NACK is 45.000 s of nothing, and the failing wait is
the durable-storage poll — not the caller's verdict.

Every terminal the re-attempt can reach is bounded below that 45 s and logs a Warning: the
base-state wait (30 s → LATE_NACK_REENQUEUE failed), an empty base (same, at once), a second NACK
(LATE_NACK_REENQUEUE / OWNER_NACK_REENQUEUE), a non-retryable NACK (LATE_NACK_TERMINAL), a
denial or delivery failure, and silence after the post (VERDICT_TIMEOUT, 31 s). Every arm that
ACKS lands the write. Exactly one terminal is both silent and leaves storage unchanged — the
empty diff, which posts nothing at all — and it requires the mirror to already carry the marker.
That is the reading, by elimination over the code's own terminals.

🚨 It is not #3603. That fixed the upsert's CREATE leg (MeshExtensions.DispatchInnerCreate);
this is the UPDATE leg in MeshNodeStreamExtensions, a different path. #3603 merged
2026-09-07T14:20:56Z, well after the 2026-09-06T20:56Z sighting, so it could not have caused it
either.

The fix

MeshNodeStreamHandle.ReattemptBaseSource — one seam, one rule:

When the owner has just stated a write never applied, a mirror base that already carries
that write is self-contradictory. Re-read the base from the owner.

ownerSaidNeverApplied is set only by the OwnerDisposing / OwnerNotReady re-enqueues (both
arms). Conflict is excluded — it is the owner asserting it is provably PAST our base, so its
mirror rebase (ActivityTracking fails to write user activity on optimistic concurrency conflict #1910) stays.
The mirror base is used unchanged unless the caller's own lambda is a no-op against it. That
is the ambiguity, and it is the only case that pays anything.
Then one authoritative GetMeshNode — routed to the OWNER, its paced re-probe standing aside for
a still-shutting-down activation. Owner has it ⇒ the no-op is real and the success is earned;
owner does not ⇒ the diff is non-empty and the write lands.
If that read cannot answer — raises, emits null, or ends having emitted nothing — the write
raises. RequireBaseState's doctrine, restated: it never resolves back onto the phantom it
was called in to reject.

Not a band-aid. No bound moves, nothing is retried, nothing is swallowed. A first attempt, a
CONFLICT re-attempt, and any "never applied" re-attempt whose mirror yields a real diff take the
identical path they took before, with no extra read.

Verification

dotnet build -c Release -warnaserror, one project per invocation, exit read directly:
MeshWeaver.Mesh.Contract 0 Warning(s) / 0 Error(s), MeshWeaver.Graph.Test 0/0,
MeshWeaver.Documentation.Test 0/0.
ReattemptBaseIsAuthoritativeTest — 7 cases, deterministic (both bases are seams: no hub, no
cluster, no scheduler, no wall clock): phantom + owner-without ⇒ rebases on the owner; phantom +
owner-with ⇒ the no-op stands (faulting there would report failure for a durable write);
mirror behind ⇒ mirror used and the authoritative read never subscribed; ordinary write ⇒
untouched even when its lambda is a legitimate no-op; and the three ways the authoritative read
can fail to answer ⇒ all raise.
🚨 Falsified: planting the pre-fix behaviour (=> mirrorBase, unconditionally) turns it red —
"Expected value to be initial … but found post-nack-4b7556aa5983" — 2 of 5 red, and the 3
that stay green are exactly the untouched paths. Restored: 15/15 green
(ReattemptBaseIsAuthoritativeTest + WriteBaseStateTotalityTest + BaseReadBoundIsReachableTest
- LateNackReenqueueTest + UpsertAnswersWhenTheOwnerGoesAwayTest).
🚨 Deliberately not verified by re-running LateNackReenqueueTest hoping to catch the 1/330.
The race stays unreproduced; a test that can only pass by luck proves nothing.
DocumentationLinkIntegrityTest 1/1 after the doc pages.
LateNackReenqueueTest's BODY is unchanged, so the Plugins twin (TeardownTwinParityTest) is
unaffected.

The next occurrence reads itself

PHANTOM_BASE — a Warning, fired only when a write was about to be lost, carrying the corr=
from #3532 so one grep yields the parent NACK, the child's registration, the phantom and the
authoritative re-read.

Work products

Doc/Architecture/PhantomBaseAfterOwnerDisposal — the mechanism, the three-case table, the
elimination argument with the terminal table, and the falsification.
What's New (Category: Fix) — a save that said it worked during a restart really did.

Pairs-with: none — no public type or member is removed; ReattemptBaseSource is internal and the
new UpdateRemote parameter is on a private method.

Refs #3477

🤖 Generated with Claude Code

… its own phantom On the owner the ECHO PRECEDES THE DURABLE FLUSH (PATCH_MERGE_STAMPED → PATCH_ECHO_SEEN → IPostCommitFlush.Flush), and ClassifyPatchException answers OwnerDisposing for a teardown fault raised on that flush leg. So the population taking the OwnerDisposing re-enqueue arm is exactly the population whose merge is already in every mirror while storage is untouched. That re-enqueue passed refusedBaseVersion = 0, which reduces RebaseSource to mirror.Take(1). The re-attempt read this hub's mirror, found its own value already there, computed an EMPTY patch, posted nothing, and completed the caller as a SUCCESS carrying a marker no store held. Every line on that path is LogDebug. The mirror cannot tell that phantom apart from a merge that committed AND flushed — only the owner can. ReattemptBaseSource asks it: when the owner has said the write never applied and the mirror nevertheless makes the caller's lambda a no-op, the base is re-read authoritatively from the owner. Owner has it ⇒ the no-op is real; owner does not ⇒ the diff is non-empty and the write lands; the read cannot answer ⇒ the write is refused loudly, never resolved back onto the phantom. Conflict keeps its mirror rebase (#1910). A first attempt, and any re-attempt whose mirror yields a real diff, are untouched — no extra read, no extra bound. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🔵 Needs a closer look

It changes core cross-hub write re-attempt behavior in a subtle failure window (owner teardown + echoed-but-unflushed state), which warrants final human review despite good deterministic test coverage.

Pull request overview

Fixes a cross-hub write-loss edge case in the mesh node update re-attempt path: when the owner NACKs a patch as “never applied” during teardown, the writer must not re-diff against its own mirror if that mirror already contains an echoed-but-not-durably-flushed “phantom” state.

Changes:

Add ReattemptBaseSource(...) to force an authoritative owner base read only for “never applied” re-attempts whose mirror base makes the caller’s update lambda a no-op (phantom detection), and fail loudly if the authoritative read cannot answer.
Thread a new ownerSaidNeverApplied flag through UpdateRemote(...) re-enqueue paths and emit a targeted PHANTOM_BASE warning when the phantom-base swap occurs.
Add deterministic unit coverage for the phantom-base rule and document the mechanism + user-facing “What’s New” entry.

File summaries

File	Description
test/MeshWeaver.Graph.Test/ReattemptBaseIsAuthoritativeTest.cs	Adds deterministic tests covering phantom-base detection, authoritative fallback behavior, and totality on authoritative-read failure modes.
src/MeshWeaver.Mesh.Contract/MeshNodeStreamExtensions.cs	Implements the phantom-base guard (`ReattemptBaseSource`) and threads `ownerSaidNeverApplied` through `UpdateRemote` re-enqueue paths with new warning instrumentation.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-07-a-save-that-said-it-worked-during-a-restart-really-did.md	Adds a user-facing “Fix” entry describing the observable symptom and the behavioral change.
src/MeshWeaver.Documentation/Data/Architecture/PhantomBaseAfterOwnerDisposal.md	Adds an architecture write-up of the phantom-base mechanism, reasoning, and diagnostics.
src/MeshWeaver.Documentation/Data/Architecture.md	Adds the new architecture page to the documentation catalog.

Review details

Files reviewed: 5/5 changed files
Comments generated: 0
Review effort level: Lite

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

github-actions · 2026-09-07T17:34:12Z