Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions src/MeshWeaver.Data/DataExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1285,6 +1285,32 @@ void AckOnce(bool success, MeshNodeError? error = null)
return;
}

// 🚨 A PARTIAL refusal is still a refusal (#2463). The check above only fires
// when NOTHING landed; a patch where some fields applied and others were
// refused fell through here and acked SUCCESS, so the caller believed its whole
// write had landed and never re-applied the refused half. That is the #648
// acked-write-loss in its partial form, and it is the harder one to see: the
// node really did change, so every "did it commit?" check says yes.
//
// It has a name in the wild. RolePlay/Scenery compiled fine, the mesh phase
// patched four compile-outcome fields, MergeGuard refused all four as
// stale/reordered, another field in the same patch landed — so this path acked
// Success, `IsDirty` never converged, and the gate read a status that was never
// written and called it a compile failure. A false RED on the required check.
//
// AckOnce LATCHES, so nacking here and letting the commit proceed is
// deliberate: the fields that were NOT in conflict are valid and keeping them
// is right, while the caller re-reads and re-applies. The re-run re-diffs
// against fresh state, so what already landed is a no-op and only the refused
// fields are retried. Conflict is one of the provably-safe retried codes and
// its budget is bounded, so this cannot spin.
if (refusedKeys > 0)
AckOnce(false, new MeshNodeError(
MeshNodeErrorCode.Conflict, hubPath,
$"cross-hub write PARTIALLY refused: {refusedKeys} field(s) changed on "
+ "the owner since the writer's base. What did not conflict was kept — "
+ "re-read and re-apply so the refused field(s) converge."));
Comment on lines +1310 to +1312

// 🚨 The OWNER mints the new Version on apply.
// Per the owned-stream contract — SynchronizationStream
// ("ONLY the owning hub sets Version", line ~255) and
Expand Down Expand Up @@ -1635,6 +1661,33 @@ as System.Text.Json.Nodes.JsonObject
AckOnce(true);
return null;
}

// 🚨 A PARTIAL refusal is still a refusal (#2463). The check above only fires
// when NOTHING landed; a patch where some fields applied and others were
// refused fell through here and acked SUCCESS, so the caller believed its whole
// write had landed and never re-applied the refused half. That is the #648
// acked-write-loss in its partial form, and it is the harder one to see: the
// node really did change, so every "did it commit?" check says yes.
//
// It has a name in the wild. RolePlay/Scenery compiled fine, the mesh phase
// patched four compile-outcome fields, MergeGuard refused all four as
// stale/reordered, another field in the same patch landed — so this path acked
// Success, `IsDirty` never converged, and the gate read a status that was never
// written and called it a compile failure. A false RED on the required check.
//
// AckOnce LATCHES, so nacking here and letting the commit proceed is
// deliberate: the fields that were NOT in conflict are valid and keeping them
// is right, while the caller re-reads and re-applies. The re-run re-diffs
// against fresh state, so what already landed is a no-op and only the refused
// fields are retried. Conflict is one of the provably-safe retried codes and
// its budget is bounded, so this cannot spin.
if (refusedKeys > 0)
AckOnce(false, new MeshNodeError(
MeshNodeErrorCode.Conflict, hubPath,
$"cross-hub write PARTIALLY refused: {refusedKeys} field(s) changed on "
+ "the owner since the writer's base. What did not conflict was kept — "
+ "re-read and re-apply so the refused field(s) converge."));

// The OWNER bumps the Version on apply (same rule as the deferred path).
// 🚨 Count from the PRE-MERGE node: the patch is client-supplied, and a
// `version` field in it has already been merged into currentNode — counting
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,82 @@ public async Task AllKeysRefusedThreeWayMerge_IsNackedConflict_NeverAckedSuccess
.Name.Should().Be("live-truth");
}

/// <summary>
/// #648 pin 3 — the PARTIAL refusal (#2463), and the one that hid the longest.
///
/// <para>Pin 1 covers a patch where EVERY key is refused, which the no-change backstop
/// catches because the node is byte-identical afterwards. A patch that refuses SOME keys and
/// applies others slips past that check: the node really did change, so
/// <c>DeepEquals(preMerge, current)</c> is false and the owner acked SUCCESS while part of the
/// caller's write was on the floor.</para>
///
/// <para>That is harder to see than pin 1, because every "did it commit?" question answers
/// yes. It is #2463: RolePlay/Scenery compiled fine, the mesh phase patched four
/// compile-outcome fields, MergeGuard refused all four as stale/reordered while another field
/// in the same patch landed — so the write was acked Success, <c>IsDirty</c> never converged,
/// and the gate read a status that had never been written and called it a compile failure. A
/// false RED on the required check, from a write that reported itself applied.</para>
///
/// <para>Fail-without: <c>Success = true</c> and the refused field silently absent.
/// Pass-with: Conflict, AND the non-conflicting field is still kept — a partial refusal must
/// not throw away the half that was valid.</para>
/// </summary>
[Fact(Timeout = 55_000)]
public async Task PartiallyRefusedThreeWayMerge_IsNackedConflict_AndKeepsWhatDidNotConflict()
{
var id = $"partial-refuse-{Guid.NewGuid():N}";
var path = $"{TestPartition}/{id}";

await NodeFactory.CreateNode(new MeshNode(id, TestPartition)
{
Name = "created", NodeType = "Markdown", State = MeshNodeState.Active
}).Should().Within(30.Seconds()).Emit();
await ReadNode(path).Should().Within(30.Seconds()).Match(n => n is { Name: "created" });

// Advance ONLY Name through the owner, so the writer's base for Name is stale while its
// base for Description is still current. That asymmetry is the whole point.
var client = GetClient(c => c.AddData());
await client.GetWorkspace().GetMeshNodeStream(path)
.Update(n => n with { Name = "live-truth" })
.Should().Within(30.Seconds()).Emit();
await ReadNode(path).Should().Within(30.Seconds()).Match(n => n is { Name: "live-truth" });

var descriptionKey = JsonOptions.PropertyNamingPolicy?.ConvertName(nameof(MeshNode.Description))
?? nameof(MeshNode.Description);
var owner = await ResolveOwner(path);
var patch = new PatchDataRequest(
new MeshNodeReference(),
new RawJson($"{{\"{NameKey}\":\"stale-mirror-write\",\"{descriptionKey}\":\"landed\"}}"))
{
// Base carries the PRE-advance Name (stale ⇒ refused) and the untouched Description
// (current ⇒ applies). One patch, two fates.
BaseValues = new RawJson($"{{\"{NameKey}\":\"created\"}}")
};

var response = await AwaitResponseAsync(patch, o => o.WithTarget(owner));
var patchResponse = response.Message;
Output.WriteLine(
$"[response] success={patchResponse.Success} code={patchResponse.NodeError?.Code} "
+ $"error={patchResponse.Error}");

patchResponse.Success.Should().BeFalse(
"part of the caller's write was refused, so the write did not fully land — acking "
+ "Success is the #648 acked-write-loss in its partial form, and the caller stops "
+ "retrying a field that never converged (#2463's IsDirty that never settles)");
patchResponse.NodeError.Should().NotBeNull();
patchResponse.NodeError!.Code.Should().Be(MeshNodeErrorCode.Conflict,
"Conflict is the retried, provably-safe code: the caller re-reads and re-applies, and "
+ "the half that already landed re-diffs to a no-op");

var final = await ReadNode(path).Should().Within(30.Seconds()).Emit();
final!.Name.Should().Be("live-truth",
"the refused key keeps the newer live value — that is the merge's verdict");
final.Description.Should().Be("landed",
"🚨 a partial refusal must NOT discard the half that did not conflict: those fields "
+ "were written against a current base and are valid. Rolling them back would turn a "
+ "reportable conflict into real data loss.");
}

/// <summary>
/// #648 pin 2 — single-writer activation invariant. The patch is posted right after the
/// owner's <c>Dispose()</c>: the DisposeRequest is queued ahead of it in the same inbox,
Expand Down
Loading