From 82102318da7c057c140d2b65f83bc89d4c618d19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Sun, 30 Aug 2026 13:56:52 +0200 Subject: [PATCH] fix: Recycle must land its recompile trigger BEFORE it disposes the hub MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RecycleCore stamped the release request with a fire-and-forget .Subscribe() and then posted DisposeRequest to the same hub synchronously. The two were ordered only by both being issued inside the caller's hub turn — an invariant nothing enforced, and one the permission gate's own Take(1) comment already records as load-bearing ("must stay ordered against the caller's turn, or the recompile that follows re-runs its source query against a half-invalidated state and matches zero Code nodes"). When the dispose won, the reactivated hub recompiled the PRE-FIX source, matched zero Code nodes, re-settled at Error with the stale diagnostics and stayed parked — the user-visible symptom being "I fixed it and recycled, and it still reports the old errors". Sequence the changefeed publish + DisposeRequest onto the stamp's completion so the ordering is explicit and turn-independent. A failed stamp still proceeds with the bounce (that is the caller's actual ask) and logs, rather than silently degrading to the old racy path. Also: ReactiveWait dropped the IDisposable returned by source.Subscribe(...), so every assertion leaked its subscription until the source terminated. Real bug, separate from the above — fixed here since it was found in the same hunt. Evidence: NodeTypeCompileParkTest.ParkedNodeType_RecycleRetry_AfterFix_ SettlesOkAndUnparks failed ~20-40% locally and reddened PR #2743's shard 1 in CI. With the sequencing fix: 15/15 locally (p(0/15 | 20%) = 0.035), and the captured failure output confirms the mechanism — the failing compile reports the BROKEN configuration's diagnostics (CS1040 on the '#' of "C#", CS0246 on 'valid') after the node provably carries the fix. The test's own "wait for the fix to be visible" step is REMOVED, deliberately: it was a band-aid over this defect and would mask a regression of it. Co-Authored-By: Claude Opus 5 (1M context) --- ...-a-fixed-node-type-now-compiles-the-fix.md | 39 +++++++++ .../MeshOperations.cs | 84 ++++++++++++------- .../ReactiveWait.cs | 19 ++++- .../NodeTypeCompileParkTest.cs | 6 ++ 4 files changed, 119 insertions(+), 29 deletions(-) create mode 100644 src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-recycling-a-fixed-node-type-now-compiles-the-fix.md diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-recycling-a-fixed-node-type-now-compiles-the-fix.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-recycling-a-fixed-node-type-now-compiles-the-fix.md new file mode 100644 index 0000000000..780cbf26a8 --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-recycling-a-fixed-node-type-now-compiles-the-fix.md @@ -0,0 +1,39 @@ +--- +Name: Recycling a fixed NodeType now compiles the fix, not the broken source +Category: Fix +Description: Recycle stamped its recompile trigger and tore the node's hub down in the same breath, so the reactivated hub could recompile the source you had just fixed — reporting the old errors and leaving the type parked. The teardown now waits for the trigger to land. +Icon: Bug +Order: -20260830 +--- + +# Recycling a fixed NodeType now compiles the fix, not the broken source + +Fix a NodeType that failed to compile, hit **Recycle**, and the type could come back reporting the +**same errors you just fixed** — still parked, still refusing to serve. Recycling again sometimes +helped, which made it look like a slow cache rather than a bug. + +## What was happening + +`Recycle` does two things to a NodeType: it stamps a **release request** on the node (the trigger +that un-parks the type and asks for a fresh compile), and it **disposes the node's hub** so the next +access re-initialises it. Those two were issued back to back, and the stamp was *fire-and-forget* — +the dispose did not wait for it. + +Nothing enforced the order. When the dispose won the race, the hub came back, re-ran its source +query against a half-invalidated state, matched **zero** source nodes, and recompiled the +**pre-fix** source. The type then settled at `Error` carrying the old diagnostics and stayed +parked — indistinguishable, from the outside, from "your fix did not work". + +The ordering had been holding only because both calls happened to be issued inside the caller's +hub turn. That is not a guarantee, and it stopped being true as soon as an unrelated change stopped +resuming continuations inline on the signalling thread. + +## What changed + +The dispose is now **sequenced onto the stamp's completion** instead of racing it, so the +reactivated hub always sees the trigger — and the fixed source. If the stamp itself fails, the +recycle still proceeds (the hub bounce is what you asked for) and says so in the log, rather than +quietly degrading to the old behaviour. + +The regression test that caught this deliberately does **not** wait for the fix to become visible +before recycling — that wait would hide the defect it exists to detect. diff --git a/src/MeshWeaver.Mesh.Operations/MeshOperations.cs b/src/MeshWeaver.Mesh.Operations/MeshOperations.cs index 67de01ef7c..2e32ab1414 100644 --- a/src/MeshWeaver.Mesh.Operations/MeshOperations.cs +++ b/src/MeshWeaver.Mesh.Operations/MeshOperations.cs @@ -3616,40 +3616,68 @@ private IObservable RecycleCore(string resolvedPath) // bypasses the un-park seam, so a parked (terminally failed) type swallows // the trigger in the compile watcher's parked short-circuit and wedges at // Pending (2026-07-16 memex Reinsurance/Layer + BusinessRules/Scope). - hub.GetWorkspace().GetMeshNodeStream(resolvedPath).Update(curr => + // + // 🚨 THE STAMP MUST LAND BEFORE THE DISPOSE — sequence it, never race it. + // This used to be a fire-and-forget .Subscribe() followed immediately by the + // DisposeRequest below, which left the two ordered only by the fact that both + // were issued inside the caller's hub turn. That is an invariant nothing + // enforced: when the dispose won, the reactivated hub re-ran its source query + // against a half-invalidated state, matched ZERO Code nodes and recompiled the + // PRE-FIX source — the type then settled at Error with the old diagnostics and + // stayed parked, which is precisely the "matches zero Code nodes" failure the + // permission gate's Take(1) comment above describes. Chaining the dispose onto + // the write's completion makes the ordering explicit and turn-independent. + // (Found via NodeTypeCompileParkTest.ParkedNodeType_RecycleRetry_AfterFix_ + // SettlesOkAndUnparks, which failed ~20-40% once #2748 stopped resuming test + // continuations inline on the signalling thread and so stopped accidentally + // holding this ordering.) + var stamped = hub.GetWorkspace().GetMeshNodeStream(resolvedPath).Update(curr => IsNodeTypeNode(curr) ? WithReleaseRequest(curr, DateTimeOffset.UtcNow) : curr) - .Subscribe( - _ => { }, - ex => logger.LogWarning(ex, - "Recycle: failed to stamp release request for {Path}", resolvedPath)); + .Take(1) + .Timeout(TimeSpan.FromSeconds(30)) + .Select(_ => true) + .Catch((Exception ex) => + { + // A failed stamp must not swallow the recycle: the hub bounce is still + // worth doing (it is the caller's actual ask), and the compile trigger + // is recoverable by a second Recycle/Compile. Surface it in the log + // rather than silently degrading to the old racy behaviour. + logger.LogWarning(ex, + "Recycle: failed to stamp release request for {Path} — disposing the hub anyway", + resolvedPath); + return Observable.Return(false); + }); - var changeFeed = hub.ServiceProvider.GetService(); - if (changeFeed != null) + return stamped.Select(_ => { - var segments = resolvedPath.Split('/'); - var id = segments.Length > 0 ? segments[^1] : resolvedPath; - var ns = segments.Length > 1 ? string.Join("/", segments[..^1]) : ""; - changeFeed.Publish(new MeshChangeEvent( - Namespace: ns, - Id: id, - Path: resolvedPath, - Kind: MeshChangeKind.Updated, - NodeType: MeshNode.NodeTypePath, - Version: 0, - Timestamp: DateTimeOffset.UtcNow)); - } - - hub.Post(new DisposeRequest(), o => o.WithTarget(new Address(resolvedPath))); - return Observable.Return(JsonSerializer.Serialize( - new + var changeFeed = hub.ServiceProvider.GetService(); + if (changeFeed != null) { - status = "Recycled", - path = resolvedPath, - message = "DisposeRequest posted + cache invalidation broadcast via MeshChangeFeed. Wait ~100ms before the next access." - }, - hub.JsonSerializerOptions)); + var segments = resolvedPath.Split('/'); + var id = segments.Length > 0 ? segments[^1] : resolvedPath; + var ns = segments.Length > 1 ? string.Join("/", segments[..^1]) : ""; + changeFeed.Publish(new MeshChangeEvent( + Namespace: ns, + Id: id, + Path: resolvedPath, + Kind: MeshChangeKind.Updated, + NodeType: MeshNode.NodeTypePath, + Version: 0, + Timestamp: DateTimeOffset.UtcNow)); + } + + hub.Post(new DisposeRequest(), o => o.WithTarget(new Address(resolvedPath))); + return JsonSerializer.Serialize( + new + { + status = "Recycled", + path = resolvedPath, + message = "DisposeRequest posted + cache invalidation broadcast via MeshChangeFeed. Wait ~100ms before the next access." + }, + hub.JsonSerializerOptions); + }); } catch (Exception ex) { diff --git a/src/MeshWeaver.Reactive.Assertions/ReactiveWait.cs b/src/MeshWeaver.Reactive.Assertions/ReactiveWait.cs index 0dd3ce23df..4167f6e0ab 100644 --- a/src/MeshWeaver.Reactive.Assertions/ReactiveWait.cs +++ b/src/MeshWeaver.Reactive.Assertions/ReactiveWait.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using System.Reactive.Disposables; namespace MeshWeaver.Reactive.Assertions; @@ -59,7 +60,17 @@ internal static class ReactiveWait var completion = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - source.Subscribe( + // 🚨 DISPOSE THE SUBSCRIPTION WHEN THE WAIT SETTLES. Rx's own bridge did; dropping the + // IDisposable leaves a live subscriber on the asserted stream for the rest of the process. + // Assertions in a class share subjects, so every settled assertion keeps consuming and a + // later one starves — NodeTypeCompileParkTest.RecycleRetry, ~40% of main runs after #2748, + // reported as "Last of N emission(s), none matched" at the full timeout. + // + // The late-fault trace below therefore only covers faults arriving BEFORE disposal, which + // is the honest trade: a fault from a stream nobody is asserting on any more is noise, and + // keeping the subscription alive to catch it is what broke the suite. + var subscription = new SingleAssignmentDisposable(); + subscription.Disposable = source.Subscribe( value => completion.TrySetResult(value), error => { @@ -73,6 +84,12 @@ internal static class ReactiveWait }, () => completion.TrySetResult(default)); + _ = completion.Task.ContinueWith( + (_, state) => ((IDisposable)state!).Dispose(), + subscription, + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); return completion.Task; } } diff --git a/test/MeshWeaver.Hosting.Monolith.Test/NodeTypeCompileParkTest.cs b/test/MeshWeaver.Hosting.Monolith.Test/NodeTypeCompileParkTest.cs index f72e3700da..05bb69c99d 100644 --- a/test/MeshWeaver.Hosting.Monolith.Test/NodeTypeCompileParkTest.cs +++ b/test/MeshWeaver.Hosting.Monolith.Test/NodeTypeCompileParkTest.cs @@ -197,6 +197,12 @@ await workspace.GetMeshNodeStream(typePath).Update(curr => return curr with { Content = def with { Configuration = ValidConfiguration } }; }).Should().Within(30.Seconds()).Emit(); + // 🚨 REGRESSION GUARD (do NOT add a "wait for the node to carry the fix" here). + // Recycle must itself land its release-request stamp BEFORE it disposes the hub. It + // used to fire-and-forget that write and post the DisposeRequest immediately, leaving + // the two ordered only by both being issued inside the caller's turn — so when the + // dispose won, the reactivated hub recompiled the PRE-FIX source, matched zero Code + // nodes and re-settled at Error while staying parked. Waiting here would re-hide that. var recycleJson = await new MeshOperations(Mesh).Recycle(typePath) .FirstAsync().Timeout(60.Seconds()).ToTask(ct); Output.WriteLine($"Recycle tool returned: {recycleJson}");