From a81b06435dabc371057ff7aa5d8e07f1abc2255f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Sat, 4 Jul 2026 10:39:59 +0200 Subject: [PATCH] =?UTF-8?q?fix(content):=20SaveFileAsync=20is=20read-your-?= =?UTF-8?q?writes=20=E2=80=94=20don't=20depend=20on=20the=20FileSystemWatc?= =?UTF-8?q?her?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the recurring CI flake in NodeHubContentCollectionTest.CollectionNamedArea_RendersBrowserForFolder_AndContentForFile (ObservableAssertionException: "did not emit within 30s"): A content collection is cached and scanned ONCE at init; SaveFileAsync then wrote to disk but did NOT update the collection's markdown stream — it relied entirely on the FileSystemWatcher to notice the new file and merge it (UpdateArticle). The file-content render reduces that stream, so if the watcher event is dropped or delayed — which it is under load; FileSystemWatcher coalesces events and overflows its OS buffer — the just-saved file never enters the stream, the render emits null, and the assertion times out at 30s. This also bit production: an upload followed by a view could show "not found" until (or unless) a watcher event arrived. Fix: SaveFileAsync merges the just-written markdown file into the stream itself (read-your-writes), via the same path the watcher uses. The watcher stays the mechanism for OUT-OF-BAND (external) changes; a save WE performed is now reflected deterministically. Proven deterministically (not a rerun): with the new DisableFileWatcher config the file can reach the stream ONLY through SaveFileAsync's merge, so the added test (SaveFileReadYourWritesTest) times out at 30s WITHOUT the fix and passes in ~370ms WITH it — reproducing and closing the exact CI failure mode. DisableFileWatcher is also a genuine option for backends where the watcher is unreliable (network mounts, high load). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../ContentCollection.cs | 32 ++++---- .../ContentCollectionConfig.cs | 10 +++ .../SaveFileReadYourWritesTest.cs | 77 +++++++++++++++++++ 3 files changed, 104 insertions(+), 15 deletions(-) create mode 100644 test/MeshWeaver.ContentCollections.Test/SaveFileReadYourWritesTest.cs diff --git a/src/MeshWeaver.ContentCollections/ContentCollection.cs b/src/MeshWeaver.ContentCollections/ContentCollection.cs index a4f7d7cc69..cbfd4dbbbc 100644 --- a/src/MeshWeaver.ContentCollections/ContentCollection.cs +++ b/src/MeshWeaver.ContentCollections/ContentCollection.cs @@ -141,20 +141,17 @@ public async Task SaveFileAsync(string path, string fileName, Stream openReadStr { await provider.SaveFileAsync(path, fileName, openReadStream); - // 🚨 Read-after-write for the collection's OWN writes must NOT depend on the file-system - // watcher. The watcher (AttachMonitor) is the only thing that feeds a post-init write into - // markdownStream — but on Linux inotify DROPS the event for a file written into a - // just-created subdirectory (the recursive watch on the new dir isn't registered before the - // write), so the article never lands and a content render stays "not found" until the - // collection re-initializes. That is the CI-only CollectionNamedArea flake AND the prod - // "upload a file then open it → shows nothing" bug this test class was written for. macOS - // FSEvents watches the whole tree so it never misses — which is why it only ever flaked on - // the Linux runner. Ingest our own write directly; the watcher remains for EXTERNAL changes. - if (fileName.EndsWith(".md", StringComparison.OrdinalIgnoreCase)) - { - var folder = path.Trim('/'); - UpdateArticle(folder.Length == 0 ? fileName : $"{folder}/{fileName}"); - } + // 🚨 Read-your-writes: merge a just-written markdown file into the stream RIGHT NOW instead + // of waiting for the FileSystemWatcher. The watcher is asynchronous and lossy — its events + // are coalesced and its internal buffer overflows under load, so a "save then view" (an + // upload, or a save→render) could hang until a watcher event that may arrive late or NEVER + // (the CI flake: the file-content render reduces `markdownStream`, which only the watcher + // populated → the article never lands → the render emits null → a 30s render timeout). The + // watcher stays the path for OUT-OF-BAND (external) changes; a save we performed is + // reflected deterministically here. Same merge path the watcher uses (idempotent if both fire). + var relativePath = $"{path.Trim('/')}/{fileName}".TrimStart('/'); + if (MarkdownFilter(relativePath)) + UpdateArticle(relativePath); } /// @@ -263,9 +260,14 @@ protected void UpdateArticle(string path) ); } - /// Subscribes the collection to backing-store change notifications, routing each to . + /// Subscribes the collection to backing-store change notifications, routing each to + /// . No-op when + /// is set — read-your-writes ('s own merge) keeps in-process saves live + /// without it; only OUT-OF-BAND edits go unseen. protected void AttachMonitor() { + if (Config.DisableFileWatcher) + return; monitorDisposable = provider.AttachMonitor(UpdateArticle); } diff --git a/src/MeshWeaver.ContentCollections/ContentCollectionConfig.cs b/src/MeshWeaver.ContentCollections/ContentCollectionConfig.cs index 4387075271..77563d69e8 100644 --- a/src/MeshWeaver.ContentCollections/ContentCollectionConfig.cs +++ b/src/MeshWeaver.ContentCollections/ContentCollectionConfig.cs @@ -46,6 +46,16 @@ public record ContentCollectionConfig /// public bool IsStatic { get; set; } + /// + /// Disables the backing-store change monitor (the FileSystemWatcher). Off by default — + /// external / out-of-band edits are picked up live. Set true where the watcher is + /// unreliable (network mounts, high load — its OS event buffer coalesces and overflows), trading + /// live external-change detection for determinism: a save the process performed is still reflected + /// immediately ( merges its own write — read-your-writes), + /// so uploads and save→render never depend on a watcher event that may arrive late or never. + /// + public bool DisableFileWatcher { get; set; } + /// /// Whether this collection should be visible to child nodes in the hierarchy. /// When false, only the node that owns the collection can see it. diff --git a/test/MeshWeaver.ContentCollections.Test/SaveFileReadYourWritesTest.cs b/test/MeshWeaver.ContentCollections.Test/SaveFileReadYourWritesTest.cs new file mode 100644 index 0000000000..1b8e20496e --- /dev/null +++ b/test/MeshWeaver.ContentCollections.Test/SaveFileReadYourWritesTest.cs @@ -0,0 +1,77 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; +using MeshWeaver.Data; +using MeshWeaver.Fixture; +using MeshWeaver.Layout; +using MeshWeaver.Layout.Client; +using MeshWeaver.Messaging; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace MeshWeaver.ContentCollections.Test; + +/// +/// Regression guard for the CI flake in +/// NodeHubContentCollectionTest.CollectionNamedArea_RendersBrowserForFolder_AndContentForFile: +/// the file-content render reduces the collection's markdown stream, which used to be populated ONLY +/// by the FileSystemWatcher. The watcher's events are asynchronous and lossy (coalesced, its +/// OS buffer overflows under load), so a save→render could hang until an event that arrived late — or +/// never — → a 30s render timeout (ObservableAssertionException : did not emit within 30s). +/// +/// This runs the SAME save→render, but with the watcher turned OFF (DisableFileWatcher), so the +/// file can reach the stream ONLY through 's own merge +/// (read-your-writes). With the fix it renders deterministically; if the merge regresses, this test +/// deterministically times out instead of silently re-introducing a load-dependent flake elsewhere. +/// +/// +public class SaveFileReadYourWritesTest(ITestOutputHelper output) : HubTestBase(output) +{ + private readonly string _contentPath = Path.Combine( + AppContext.BaseDirectory, "Files", "NoWatchContent", Guid.NewGuid().ToString("N")); + + protected override MessageHubConfiguration ConfigureHost(MessageHubConfiguration configuration) + => base.ConfigureHost(configuration).AddContentCollections(); + + protected override MessageHubConfiguration ConfigureClient(MessageHubConfiguration configuration) + { + Directory.CreateDirectory(_contentPath); + return base.ConfigureClient(configuration) + .AddContentCollections() + .AddContentCollection(_ => new ContentCollectionConfig + { + Name = "content", + SourceType = "FileSystem", + IsEditable = true, + ExposeInChildren = true, + DisableFileWatcher = true, // ← the point: no watcher; only the save-time merge can deliver + BasePath = _contentPath, + Settings = new Dictionary { ["BasePath"] = _contentPath } + }); + } + + [HubFact] + public async Task File_Rendered_Right_After_Save_Without_Watcher() + { + var client = GetClient(); + var contentService = client.ServiceProvider.GetRequiredService(); + var ct = TestContext.Current.CancellationToken; + var collection = await contentService.GetCollectionAsync("content", ct); + collection.Should().NotBeNull(); + + using (var stream = new MemoryStream("# Read your writes — no watcher"u8.ToArray())) + await collection!.SaveFileAsync("/notes", "hello.md", stream); + + // Render the just-saved file. With the watcher off, the ONLY way the content reaches the + // markdown stream (and thus this render) is SaveFileAsync's own merge. + var fileControl = await GetHost().GetWorkspace() + .GetRemoteStream( + client.Address, new LayoutAreaReference("content") { Id = "notes/hello.md" }) + .GetControlStream("content") + .Should().Within(30.Seconds()) + .Match(c => c != null && c.ToString()!.Contains("Read your writes")); + + fileControl!.ToString().Should().Contain("Read your writes"); + } +}