GetMeshNode: a timed-out read is not "node not found" by rbuergi · Pull Request #661 · Systemorph/MeshWeaver

rbuergi · 2026-07-27T06:16:30Z

The defect

A read that gives up knows nothing about the node — but GetMeshNode emitted the same null the not-found path emits, with nothing louder than a Debug log. At all 45 call sites a 60 s mesh stall was indistinguishable from an absent node: callers substituted defaults, rendered empty areas, and compiled against missing NodeType definitions.

That is exactly how ThreadAgentIntegrationTest.FullFlow_CreateThread_SendMessage_NonStreamingResponse failed on CI (main 89332e347, and again on #659). From the trx:

	target test	its 2 siblings, same class, same node	project total
Failing run	60.27 s ❌	1.15 s / 0.69 s ✅	332.7 s
Passing run	0.51 s ✅	2.09 s / 0.83 s ✅	280.8 s

ReadNode burned its full 60 s budget, returned null, the test never asserted the node, every assertion passed — and only the class watchdog failed it, with "this test almost certainly ignored its CancellationToken", which the test had threaded correctly. The runner wasn't slow; one read stalled in an otherwise healthy process.

The change

GetMeshNode(path, timeout, onTimeout) — ReadTimeoutBehavior.Throw by default. The timeout surfaces a TimeoutException naming the path, elapsed time, budget and the hub's in-flight snapshot:
```
GetMeshNode('ACME/ProductLaunch') timed out after 60.0s (budget 60s) — the owning
per-node hub never answered the GetDataRequest. This is NOT 'node not found'.
Hub mesh/xyz RunLevel=Started Queue(buffer=0,deferred=0,exec=1) …
PendingCallbacks=1[…=GetDataRequest@ACME/ProductLaunch(59998ms)]
```
Strict is the right default: the API already surfaced OnError for Unauthorized, so callers already needed error discipline — 8 src sites had added .Catch/error lambdas for that reason.
EmitNull is the explicit opt-in for callers whose documented contract is "indeterminate ⇒ absent". It still logs at Warning — opting in suppresses the exception, never the evidence.
IMessageHub.GetPendingRequestDiagnostics() — focused and deliberately non-recursive, unlike GetDisposalDiagnostics(), which walks the whole hosted-hub tree and is far too heavy to attach to a live timeout on a portal mesh hub.
TestHardDeadline 60 s → 90 s. It was equal to ReadNodeTimeout, so a read that burned its budget tripped the watchdog at the same instant and the specific error could never win the race. ReadNode also stops mapping TimeoutException → null; NotFound → null is unchanged, so "absent" assertions are unaffected.

Caller audit — all 45 src sites

13 already had an error sink → unchanged.
8 fire-and-forget click actions → explicit EmitNull (PinLayoutArea ×2, UserActivityLayoutAreas ×2, GroupsLayoutArea ×2, MeshDocumentSink.DocumentExists, BrandingResolver). With no error sink they would have rethrown on the CTS timer thread — an unobserved fault, precisely the failure class this removes.
Layout areas stay strict, deliberately: LayoutAreaHost.FailRendering renders a faulted area as a visible error control, so a stall now shows the timeout instead of a false empty state.
One real bug fixed: CodeNodeType.HandleExecuteScript answered a stalled self-read with "Not executable" — a lie the user cannot act on.

Deliberately not changed

MeshNodeCompilationService's three NodeType-definition reads are pinned to EmitNull with a comment. There is a real defect there — on a stalled read ContentAs<NodeTypeDefinition>() yields null and the compile proceeds against a null definition — but it runs on the hub activation path where compile status is cached, so making it throw would turn a transient stall into a parked type (the UWDeepfield outage class). That needs its own retry/park semantics and tests, not a rider on this change. It is no longer silent either way. Follow-up issue recommended.

Tests

GetMeshNodeTimeoutSurfacingTest (4 cases) is deterministic, not timing-based: a hub whose GetDataRequest handler returns Processed() and never replies, read at its own address — the reply does not exist, so no machine speed can flake it in either direction. Cases: surfaces TimeoutException naming the path; the message carries the pending-callback snapshot; EmitNull still returns null; a genuine miss still returns null promptly.

Verified — full solution -c Release -p:CIRun=true -warnaserror, 0 warnings:

Suite	Result
`Hosting.Monolith.Test` (incl. 4 new)	398 passed, 2 skipped / 400
`AI.Test`	889 passed, 5 skipped / 894
`Graph.Test`	626 / 626
`Query.Test`	357 / 357
`Security.Test`	344 / 344
`Threading.Test`	128 / 128

No What's New entry: no user-visible behaviour change in normal operation — it changes what happens when the mesh is already failing.

🤖 Generated with Claude Code

A read that gives up knows nothing about the node, but GetMeshNode emitted the same null the not-found path emits, logged at Debug. So a 60s mesh stall was indistinguishable from an absent node at every one of 45 call sites: callers substituted defaults, rendered empty areas, and compiled against missing NodeType definitions, while the stall itself left nothing louder than a log line nobody reads. That is how ThreadAgentIntegrationTest.FullFlow_CreateThread_SendMessage_ NonStreamingResponse failed on CI (main 89332e3, PR #659): ReadNode burned its full 60s budget, returned null, the test never asserted the node, every assertion passed — and only the class watchdog failed it, blaming a CancellationToken the test had threaded correctly. The read took 60.27s; its two siblings on the same node ran in 1.15s and 0.69s. - GetMeshNode gains onTimeout: ReadTimeoutBehavior (Throw by default). The timeout surfaces a TimeoutException naming the path, elapsed time, budget and the reading hub's in-flight snapshot, so the next occurrence says WHY: our GetDataRequest still outstanding = the reply never came; the hub Executing something else for seconds = action-block congestion or ThreadPool starvation. Strict is the right default because the API already surfaced OnError for Unauthorized, so every caller already needed error discipline; 8 src sites had added .Catch/error lambdas for exactly that reason. - EmitNull is the explicit opt-in for callers whose documented contract is "indeterminate implies absent" — and it still logs the stall at Warning, so opting in suppresses the exception, never the evidence. - IMessageHub.GetPendingRequestDiagnostics(): a focused, NON-recursive snapshot. GetDisposalDiagnostics walks the whole hosted-hub tree, far too heavy to attach to a live timeout on a portal mesh hub. - MonolithMeshTestBase.TestHardDeadline 60s -> 90s. It was EQUAL to ReadNodeTimeout, so a read that burned its budget tripped the watchdog at the same instant and the specific error could never win the race. The watchdog is the backstop for hangs with no budget of their own, not a competitor to the ones that have one. ReadNode also stops mapping TimeoutException to null; NotFound still maps to null, so "absent" assertions are unaffected. All 45 call sites audited. 13 already had error sinks. 8 fire-and-forget click actions moved to explicit EmitNull — they had no error sink, so they would have rethrown on the CTS timer thread: an unobserved fault, exactly the failure class this change removes. Layout areas stay strict deliberately: LayoutAreaHost .FailRendering renders a faulted area as a visible error control, so a stall now shows the timeout instead of a false empty state. One real bug fixed: CodeNodeType.HandleExecuteScript answered a stalled self-read with "Not executable" — a lie the user cannot act on. MeshNodeCompilationService's three NodeType-definition reads are pinned to EmitNull with a comment, deliberately. There is a real defect there — on a stalled read ContentAs<NodeTypeDefinition>() yields null and the compile proceeds against a null definition — but it runs on the hub ACTIVATION path where compile status is cached, so making it throw would turn a transient stall into a PARKED type (the UWDeepfield outage class). That needs its own retry/park semantics and tests. It is no longer silent either way: the read logs the stall with diagnostics. Follow-up issue recommended. New test GetMeshNodeTimeoutSurfacingTest (4 cases) is deterministic, not timing-based: a hub whose GetDataRequest handler returns Processed() and never replies, read at its own address — the reply does not exist, so no machine speed can flake it either way. Verified: full solution Release/-warnaserror clean, 0 warnings. Monolith 398/400 (2 skipped, incl. the 4 new), AI 889/894 (5 skipped), Graph 626/626, Query 357/357, Security 344/344, Threading 128/128. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR changes the semantics of one-shot mesh reads (IMessageHub.GetMeshNode) so that a read timeout is no longer indistinguishable from “node not found”. Timeouts now surface (by default) as a TimeoutException with actionable hub diagnostics, with an explicit EmitNull opt-in for call sites whose contract is “indeterminate ⇒ absent”.

Changes:

Add ReadTimeoutBehavior and extend GetMeshNode(path, timeout, onTimeout) with strict timeout surfacing by default and EmitNull as an explicit opt-in.
Introduce IMessageHub.GetPendingRequestDiagnostics() to attach cheap, non-recursive hub in-flight state to timeout reporting.
Update call sites and tests: widen the test harness hard deadline, add deterministic timeout-contract tests, and add a missing assertion in ThreadAgentIntegrationTest.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/MeshWeaver.Hosting.Monolith.Test/GetMeshNodeTimeoutSurfacingTest.cs	New deterministic tests pinning timeout-vs-notfound behavior and diagnostics payload.
test/MeshWeaver.AI.Test/ThreadAgentIntegrationTest.cs	Adds an assertion to avoid silently proceeding with a null context node.
src/MeshWeaver.Messaging.Hub/MessageHub.cs	Implements `GetPendingRequestDiagnostics()` snapshot string for live timeout reporting.
src/MeshWeaver.Messaging.Hub/IMessageHub.cs	Adds the `GetPendingRequestDiagnostics()` API contract.
src/MeshWeaver.Mesh.Contract/ReadTimeoutBehavior.cs	Introduces the timeout behavior enum (Throw vs EmitNull).
src/MeshWeaver.Mesh.Contract/MeshNodeStreamExtensions.cs	Updates `GetMeshNode` to throw on timeout by default, log diagnostics, and support `EmitNull`.
src/MeshWeaver.Markdown.Export/Branding/BrandingResolver.cs	Opts branding reads into `EmitNull` for cosmetic fallback behavior.
src/MeshWeaver.Hosting.Monolith.TestBase/MonolithMeshTestBase.cs	Raises `TestHardDeadline` and stops mapping read timeouts to null in `ReadNode`.
src/MeshWeaver.Graph/UserActivityLayoutAreas.cs	Opts fire-and-forget click reads into `EmitNull`.
src/MeshWeaver.Graph/PinLayoutArea.cs	Opts pin/unpin fire-and-forget reads into `EmitNull`.
src/MeshWeaver.Graph/GroupsLayoutArea.cs	Opts best-effort existence/icon reads into `EmitNull`.
src/MeshWeaver.Graph/Configuration/MeshNodeCompilationService.cs	Pins activation-path reads to `EmitNull` with explicit rationale.
src/MeshWeaver.Graph/Configuration/CodeNodeType.cs	Adds an error sink for the self-read so a stall isn’t misreported as “Not executable”.
src/MeshWeaver.ContentCollections.Indexing.Graph/MeshDocumentSink.cs	Opts existence probe into `EmitNull` per interface semantics.

Comments suppressed due to low confidence (4)

src/MeshWeaver.Graph/UserActivityLayoutAreas.cs:405

This fire-and-forget subscription has no OnError handler. If GetMeshNode faults (e.g., Unauthorized), Rx will route it to the default unhandled exception path. Add an error sink so the click action can fail safely.

        ctx.Host.Hub.GetMeshNode(userPath, TimeSpan.FromSeconds(10), ReadTimeoutBehavior.EmitNull)
            .Subscribe(node =>
            {

src/MeshWeaver.Graph/PinLayoutArea.cs:152

This fire-and-forget subscription has no OnError handler. GetMeshNode can still fault (e.g., Unauthorized), which will surface as an unhandled Rx error. Add an error sink to avoid process-level unhandled exceptions and to capture diagnostics in logs.

This issue also appears on line 188 of the same file.

                    ctx.Host.Hub.GetMeshNode(userPath, TimeSpan.FromSeconds(10), ReadTimeoutBehavior.EmitNull)
                        .Subscribe(n =>
                        {
                            if (n?.Content is not User user) return;
                            var paths = user.PinnedPaths?.ToImmutableList() ?? ImmutableList<string>.Empty;

src/MeshWeaver.Graph/PinLayoutArea.cs:191

This fire-and-forget subscription has no OnError handler. If GetMeshNode faults (e.g., Unauthorized), Rx will route it to the default unhandled exception handler. Add an error sink so failures are contained and logged.

        host.Hub.GetMeshNode(userPath, TimeSpan.FromSeconds(10), ReadTimeoutBehavior.EmitNull)
            .Subscribe(node =>
            {
                if (node?.Content is not User user) return;

src/MeshWeaver.Graph/GroupsLayoutArea.cs:287

This member-icon lookup is described as best-effort but is subscribed without an error sink. If GetMeshNode faults (e.g., Unauthorized), it can surface as an unhandled Rx error and prevent membership creation. Catch errors and continue with a null icon.

                                    saveCtx.Hub.GetMeshNode(selectedMember, TimeSpan.FromSeconds(2), ReadTimeoutBehavior.EmitNull)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+            },
+            readError =>
+            {
+                // The self-read failed (timeout / access denial). Answer the caller with what
+                // actually happened instead of the misleading "Not executable" verdict above.
+                hub.Post(
+                    new ExecuteScriptResponse
+                    {
+                        Success = false,
+                        Error = $"Could not read the code node to execute: {readError.Message}"
+                    },
+                    o => o.ResponseFor(request));
            });


+        host.Hub.GetMeshNode(hubPath, TimeSpan.FromSeconds(10), ReadTimeoutBehavior.EmitNull)
            .Subscribe(node =>


+                            // the existence probe reads as "absent" (unchanged behaviour: the
+                            // create below is guarded by the mesh's own uniqueness handling);
+                            // the read logs the timeout + hub diagnostics at Warning.
+                            saveCtx.Hub.GetMeshNode(path, TimeSpan.FromSeconds(5), ReadTimeoutBehavior.EmitNull)


github-actions · 2026-07-27T06:31:08Z

Test Results (shard 3)

12 files + 1 12 suites +1 4m 32s ⏱️ + 1m 36s
970 tests +59 966 ✅ +59 4 💤 ±0 0 ❌ ±0
1 033 runs +59 1 029 ✅ +59 4 💤 ±0 0 ❌ ±0