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
28 changes: 26 additions & 2 deletions src/MeshWeaver.Graph/Configuration/MeshNodeCompilationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -502,11 +502,35 @@ private IObservable<IEnumerable<MeshNode>> SnapshotSources(
//
// So read directly and keep the synced query only as a FALLBACK for the case the direct
// read finds nothing — never the other way round. An earlier attempt (#682) kept the cache
// primary and raced a delayed probe against it; the probe never fired once in production,
// primary and raced a DELAYED probe against it; the probe never fired once in production,
// which is precisely the failure this ordering removes: the fast path is no longer
// conditional on losing a race.
//
// 🚨 MERGE, not Concat — both start NOW and the first ANSWER wins.
//
// Concat is sequential: the synced query is not even subscribed until the probe has
// COMPLETED, so every compile paid the probe's full latency up front — and the probe cannot
// answer faster than SourceProbeQuietWindow, because each leg has to watch its chunked
// Initial go quiet before it may emit. That is a floor of ~1s per compile that the previous
// code never paid (the probe was DelaySubscription'd and, on a healthy mesh, never ran).
// Measured on Hosting.Monolith.Test, same box, CompileLeafStabilityTest:
//
// before #690 (cached primary) 4s
// #690 as merged (probe, then cached) 52s ← 13×; pushed the suite past CI's 6m cap
// this change (true race) ~4s
//
// Merging keeps #690's win intact — a STALLED synced query no longer costs 45s, because the
// probe answers in about a read plus the quiet window — while a HEALTHY synced query answers
// immediately and the probe's latency never lands on the critical path. Neither source is
// privileged, so the fast path does not depend on winning a race: whichever is healthy
// answers, and the loser is simply dropped by FirstAsync.
//
// The probe's `.Where(count > 0)` is what makes this safe to merge: a probe that finds
// nothing stays SILENT and completes, so it can never beat the cached query with an empty
// set (compiling against no sources is worse than the stall this exists to dodge). That
// filter is also the likeliest reason #682's probe "never fired" — not the delay alone.
return DirectSourceProbe(ntDef, selfPath, TimeSpan.Zero)
.Concat(Observable.Defer(() => ResolveSources(ntDef, selfPath, null).Take(1)))
.Merge(Observable.Defer(() => ResolveSources(ntDef, selfPath, null).Take(1)))
.FirstAsync()
.Timeout(bound)
.Catch<IEnumerable<MeshNode>, TimeoutException>(ex =>
Expand Down
48 changes: 47 additions & 1 deletion src/MeshWeaver.Mesh.Contract/MeshExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,15 @@ private static IMessageDelivery HandleCreateNodeRequest(
// them — which is precisely why this belongs at the boundary rather than in each writer.
// Runs with the other STRUCTURAL invariants: before the validators and before their System
// bypass, because a root grant is catastrophic regardless of who writes it.
//
// 🚨 The guard MUST see the NORMALISED node, hence the call above. MeshNode.MainNode defaults
// to Path, so a satellite created the ordinary way (`MeshNode.FromPath("{p}/_Access/a1")`,
// no explicit MainNode) arrives with MainNode == its own path while the path encodes scope
// "{p}" — a mismatch the guard refuses. Normalising first is what makes the framework's own
// documented satellite shape legal; guarding the RAW node rejected every caller that relies
// on the auto-derivation this method has always performed.
node = NormalizeSatelliteMainNode(node, meshConfig);

if (AccessAssignmentGuard.IsScopeInvalid(node, out var scopeReason))
{
logger.LogError("[CreateNode] REFUSED mis-scoped AccessAssignment {Path}: {Reason}", node.Path, scopeReason);
Expand Down Expand Up @@ -402,12 +411,16 @@ private static IMessageDelivery HandleCreateNodeRequest(
// and the access-granted notification named "{scope}/_Access" instead of the node that
// was shared. A root-level satellite (`_Access/{id}`, the root-scope grant) has no
// owner: MainNode = "" is that shape's documented value (see ActivityNodeGuard).
// 🚨 Already APPLIED ABOVE, before the AccessAssignment scope guard — that guard
// compares MainNode against the scope the path encodes, so it must run on the
// normalised node. Retained here (idempotent: the condition is false once MainNode
// has been rewritten) because it is the `if` of the chain whose `else if` is 1b'.
if (!string.IsNullOrEmpty(node.NodeType)
&& !string.IsNullOrEmpty(node.Namespace)
&& meshConfig.IsSatelliteNodeType(node.NodeType)
&& node.MainNode == node.Path)
{
node = node with { MainNode = SatelliteTableMapping.OwnerOfSatellitePath(node.Namespace) };
node = NormalizeSatelliteMainNode(node, meshConfig);
}
// 1b'. Repair a STALE BARE-ID self-default MainNode on a MAIN (non-satellite) node.
// MainNode is a STORED property: unlike the computed Path/Segments it does NOT follow
Expand Down Expand Up @@ -1953,6 +1966,15 @@ private static IMessageDelivery HandleCreateOrUpdateNodeRequest(
// partition/scope its path sits under. Guarding only the create path would leave the hole
// open — an upsert could set MainNode back to empty afterwards, which silently converts a
// partition grant into a ROOT grant (All on every partition). Both write paths, or neither.
//
// 🚨 And — exactly as on the create path — the guard must see the NORMALISED node: MainNode
// defaults to Path, so an un-normalised satellite reads as "scoped to itself" and trips the
// mismatch branch. Normalising here also fixes a real asymmetry: upsert never derived a
// satellite's MainNode, so an upserted satellite pointed at itself instead of its owner.
var upsertMeshConfig = hub.ServiceProvider.GetService<MeshConfiguration>();
if (upsertMeshConfig != null)
node = NormalizeSatelliteMainNode(node, upsertMeshConfig);

if (AccessAssignmentGuard.IsScopeInvalid(node, out var upsertScopeReason))
{
logger.LogError("[UpsertNode] REFUSED mis-scoped AccessAssignment {Path}: {Reason}", node.Path, upsertScopeReason);
Expand Down Expand Up @@ -2166,6 +2188,30 @@ void PostFail(string error, NodeUpsertRejectionReason reason)
};
}

/// <summary>
/// Point a satellite's <c>MainNode</c> at its OWNING main node — the namespace with the
/// satellite tail cut off (<c>{owner}/_Access</c> → <c>{owner}</c>) — when it still carries the
/// record's self-default (<c>MainNode == Path</c>, i.e. never explicitly set).
///
/// <para>Shared by BOTH write paths so they agree. <c>CreateNode</c> has always done this;
/// <c>UpsertNode</c> did not, which left a satellite upserted without an explicit MainNode
/// pointing at ITSELF — the exact shape every MainNode consumer misreads (SatelliteAccessRule
/// delegates permissions to it, <c>rebuild_user_effective_permissions</c> projects a grant at
/// <c>COALESCE(main_node, namespace)</c>, satellite scope filters match it as the attachment
/// point). The <c>AccessAssignment</c> scope guard runs on BOTH paths, so both must normalise
/// before it or the guard rejects the framework's own default shape.</para>
///
/// <para>Idempotent: once MainNode differs from Path the node is returned unchanged, so calling
/// it twice on one write is safe.</para>
/// </summary>
private static MeshNode NormalizeSatelliteMainNode(MeshNode node, MeshConfiguration meshConfig) =>
!string.IsNullOrEmpty(node.NodeType)
&& !string.IsNullOrEmpty(node.Namespace)
&& meshConfig.IsSatelliteNodeType(node.NodeType)
&& node.MainNode == node.Path
? node with { MainNode = SatelliteTableMapping.OwnerOfSatellitePath(node.Namespace) }
: node;

/// <summary>
/// True when applying <paramref name="sourceNode"/> onto <paramref name="existing"/> via
/// <see cref="UpdateAccordingToSourceNode"/> would change nothing but the churn stamps
Expand Down
Loading