fix(webhook-inbox): a stalled target read answers 503 + Retry-After, not a bare 500 (#6125)

memex/Memex.Portal.Shared/Api/InstanceAuthResponses.cs

@@ -60,7 +60,10 @@ internal static IResult Unavailable(HttpContext http, string? reason, ILogger? l
     /// <param name="http">The request — the 503 carries <c>Retry-After</c> on its response.</param>
     /// <param name="cause">The availability fault the route's read terminated with.</param>
     /// <param name="logger">Diagnostic sink, resolved while the request scope was alive.</param>
-    internal static IResult ReadUnavailable(HttpContext http, Exception cause, ILogger? logger)
+    /// <param name="error">The body's <c>error</c> text, naming what this ROUTE could not read; the
+    /// registry's catalogue wording when omitted.</param>
+    internal static IResult ReadUnavailable(
+        HttpContext http, Exception cause, ILogger? logger, string? error = null)
     {
         logger?.LogWarning(cause,
             "Mesh read behind {Path} is UNAVAILABLE — answering 503 + Retry-After rather than an "
@@ -69,7 +72,7 @@ internal static IResult ReadUnavailable(HttpContext http, Exception cause, ILogg
         http.Response.Headers.RetryAfter =
             InstanceRegistryAuthenticator.RetryAfterSeconds.ToString(CultureInfo.InvariantCulture);
         return Results.Json(
-            new { error = "The registry could not read its catalogue just now — retry shortly." },
+            new { error = error ?? "The registry could not read its catalogue just now — retry shortly." },
             statusCode: StatusCodes.Status503ServiceUnavailable);
     }
 
@@ -82,10 +85,11 @@ internal static IResult ReadUnavailable(HttpContext http, Exception cause, ILogg
     /// <param name="answer">The route's answer.</param>
     /// <param name="http">The request.</param>
     /// <param name="logger">Diagnostic sink, resolved while the request scope was alive.</param>
+    /// <param name="error">The 503 body's <c>error</c> text (see <see cref="ReadUnavailable"/>).</param>
     internal static IObservable<IResult> UnavailableOnAStalledRead(
-        this IObservable<IResult> answer, HttpContext http, ILogger? logger) =>
+        this IObservable<IResult> answer, HttpContext http, ILogger? logger, string? error = null) =>
         answer.Catch<IResult, Exception>(ex =>
             MeshWeaver.Layout.AreaErrorClassifier.IsStorageUnavailable(ex)
-                ? Observable.Return(ReadUnavailable(http, ex, logger))
+                ? Observable.Return(ReadUnavailable(http, ex, logger, error))
                 : Observable.Throw<IResult>(ex));
 }

memex/Memex.Portal.Shared/Api/WebhookInboxEndpoints.cs

@@ -105,14 +105,36 @@ private static async Task<IResult> Deliver(
         var headers = request.Headers.Select(h =>
             new KeyValuePair<string, string>(h.Key, h.Value.ToString()));
 
-        var result = (await WebhookInbox.Deliver(
+        // 🚨 A STALLED read is 503 + Retry-After, never an unhandled 500 (#6125). Before storing,
+        // Deliver confirms the target node exists with a `path:{target}` query, and when a query
+        // provider never delivers its Initial the fan-in terminates that read with
+        // QueryProviderStalledException (policy query-fanin-stall-terminal) — an availability
+        // failure that says, in its own text, that it is retryable. It used to escape this handler,
+        // so ASP.NET's exception middleware answered a bare 500 and logged the stall as a defect of
+        // the ENDPOINT. Same mapping as the plugin-bundle routes (#5345): only what
+        // AreaErrorClassifier.IsStorageUnavailable classifies becomes 503; every other fault still
+        // escapes, so a real defect keeps surfacing as one. Nothing was stored on that path, so a
+        // sender that delivers again loses nothing and duplicates nothing.
+        return (await WebhookInbox.Deliver(
                 hub, allowed, target, request.ContentType, headers, body)
+            .Select(result => Answer(result, target, logger))
+            .UnavailableOnAStalledRead(request.HttpContext, logger, StalledReadError)
             .FirstAsync()
             .ObserveCompletion(
                 ex => logger?.LogWarning(ex,
                     "Webhook delivery for target '{Target}' faulted after the response had already been sent",
                     target),
                 ct))!;
+    }
+
+    /// <summary>The 503 body's <c>error</c> when the delivery's own read could not be answered.
+    /// Nothing was stored, and the text says so: the sender's next move is to deliver again.</summary>
+    internal const string StalledReadError =
+        "The inbox could not confirm its target just now — nothing was stored; retry shortly.";
+
+    /// <summary>Maps a delivery's verdict to its HTTP answer, with the log line that goes with it.</summary>
+    private static IResult Answer(WebhookInbox.DeliveryResult result, string target, ILogger? logger)
+    {
         switch (result.Status)
         {
             case WebhookInbox.DeliveryStatus.Accepted when result.VerifyOnly:

src/MeshWeaver.Documentation/Data/Architecture/WebhookInbox.md

@@ -221,6 +221,48 @@ demanding `X-Hub-Signature-256` from Stripe would 401 every payment delivery.
    decide the verdict), and it makes core's CD verdict depend on other repositories' state, which is
    the CI-to-CI coupling `main-cd.yml` has already deleted twice.
 
+## 🚨 A stalled read answers 503, never a bare 500 (#6125)
+
+Before storing, the endpoint confirms the allowlisted target exists with a `path:{target}` query.
+When a query provider never delivers its Initial, the fan-in ends that read with
+`QueryProviderStalledException` (policy `query-fanin-stall-terminal`). That exception used to
+escape the handler, so ASP.NET answered a **bare 500** and logged it as an unhandled exception of
+`WebhookInboxEndpoints.Deliver`. Production saw this on memex on 2026-10-04 and 2026-10-05, with the
+query `path:Hosting/PlatformBuilds`.
+
+The route now maps it the way the plugin-bundle routes do (#5345), through
+`InstanceAuthResponses.UnavailableOnAStalledRead`. Only what
+`AreaErrorClassifier.IsStorageUnavailable` classifies becomes **503 + `Retry-After`**, and the
+body's `error` says nothing was stored. Every other fault still escapes, so a real defect stays
+visible. Delivering again is safe: nothing was written on that path.
+
+Pinned by `test/Memex.Portal.Shared.Test/WebhookInboxStalledReadTest.cs`. It has three cases: the
+stall answers 503, an answered read answers 200, and an ordinary fault still escapes. Without the
+mapping, the stall case fails with the production exception.
+
+## What a slow answer after "Node created" is — and is not (#6039)
+
+One GitHub delivery was logged `Node created at Hosting/PlatformBuilds/_Inbox/…` about 200 ms after
+GitHub sent it. GitHub still recorded no response within its 10 s limit. Investigated, without a
+code change:
+
+- **Not post-creation handlers.** No `INodePostCreationHandler` in core or MeshWeaver.Plugins
+  matches a `WebhookEvent`. So `RunPostCreationHandlersObs` is empty for an inbox node, and
+  `CreateNodeResponse.Ok` is posted synchronously right after the "Node created" line.
+- **What is left is the reply's dispatch.** It needs the `portal/nodeops-{meshId}` action block, and
+  it needs it however the create was issued. The endpoint holds the root hub, so its create runs on
+  that hub. A reply that hub posts is first RECEIVED and ENQUEUED on the hub's *own* block before it
+  is routed (request-fate trail: `RESPONSE_POSTED target=portal/reads-… ↩ reply#1: RECEIVED
+  @portal/nodeops-… → ENQUEUED → QUEUED queue=main`). Issuing the create from
+  `portal/reads-{meshId}` therefore does **not** separate the answer from that block. This was
+  tried and measured: with the block held after the commit, the delivery was still not answered,
+  because the reply sat on the node-CRUD block on its way out. It is unlike the content route's read
+  (#2901), whose responder is a per-node hub.
+- **Not established:** which turn held the node-CRUD block at 18:10:31Z on 2026-09-26. That is the
+  open question of #2543. Cutting the answer's dependence on that block would need one of two
+  changes. Either the commit itself becomes the signal the endpoint waits on, or a responder's
+  outgoing reply stops queueing on its own block. Both are framework changes outside this endpoint.
+
 ## Where the code lives
 
 - `memex/Memex.Portal.Shared/Api/WebhookInboxEndpoints.cs` — the anonymous `POST /api/hooks/{target}`

test/Memex.Portal.Shared.Test/WebhookInboxStalledReadTest.cs

@@ -0,0 +1,244 @@
+#pragma warning disable CS1591
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Net;
+using System.Net.Http;
+using System.Reactive.Linq;
+using System.Text;
+using System.Text.Json;
+using System.Threading;
+using System.Threading.Tasks;
+using Memex.Portal.Shared.Api;
+using MeshWeaver.Fixture;
+using MeshWeaver.Graph.Configuration;
+using MeshWeaver.Hosting.Monolith.TestBase;
+using MeshWeaver.Markdown;
+using MeshWeaver.Mesh;
+using MeshWeaver.Mesh.Security;
+using MeshWeaver.Mesh.Services;
+using MeshWeaver.Messaging;
+using MeshWeaver.PluginCatalog;
+using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.TestHost;
+using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Xunit;
+
+namespace Memex.Portal.Shared.Test;
+
+/// <summary>
+/// 🚨 A webhook delivery whose target-existence read STALLS answers 503 + <c>Retry-After</c> —
+/// never an unhandled 500 (MeshWeaver#6125).
+///
+/// <para><c>WebhookInbox.Deliver</c> confirms the allowlisted target exists with a
+/// <c>path:{target}</c> query before storing. When a query provider never delivers its Initial,
+/// the fan-in terminates that read with <see cref="QueryProviderStalledException"/> (policy
+/// <c>query-fanin-stall-terminal</c>) — retryable by its own statement. The route let it escape, so
+/// ASP.NET answered a bare 500 and logged an unhandled exception of
+/// <c>WebhookInboxEndpoints.Deliver</c> (measured on memex 2026-10-04 19:25:46Z and 2026-10-05
+/// 07:44:45Z, query <c>path:Hosting/PlatformBuilds</c>). The sibling plugin-bundle routes already
+/// answer this 503 (#5345); this pins the inbox to the same convention.</para>
+///
+/// <para>Same harness as <c>PluginBundleStalledReadTest</c>: a test <see cref="IMeshQueryProvider"/>
+/// — the extension point every backend plugs into, not a mock of a core service — that claims every
+/// query, answers each with an empty Initial, and leaves ONLY the target-existence read unanswered
+/// (or faults it) while armed. The stall fires at the fan-in's own production rung.</para>
+///
+/// <para><b>Negative control.</b> With the endpoint's <c>UnavailableOnAStalledRead</c> line removed,
+/// <see cref="AStalledTargetRead_Answers503WithRetryAfter"/> goes red: the exception escapes and the
+/// TestServer rethrows it / answers 500.</para>
+/// </summary>
+public class WebhookInboxStalledReadTest(ITestOutputHelper output) : MonolithMeshTestBase(output)
+{
+    private const string Target = "Hosting/StalledInbox";
+    private const string Route = "/api/hooks/" + Target;
+    private const string DefectMessage = "target read defect (test)";
+
+    private readonly StallingTargetProvider provider = new($"path:{Target}");
+
+    protected override MeshBuilder ConfigureMesh(MeshBuilder builder) => base.ConfigureMesh(builder)
+        .AddWebhookInbox()
+        .ConfigureServices(services => services.AddSingleton<IMeshQueryProvider>(provider));
+
+    private async Task<WebApplication> StartHost(CancellationToken ct)
+    {
+        var builder = WebApplication.CreateBuilder();
+        builder.WebHost.UseTestServer();
+        builder.Logging.ClearProviders();
+        builder.Configuration.AddInMemoryCollection(new Dictionary<string, string?>
+        {
+            [$"{WebhookInbox.TargetsConfigSection}:0"] = Target,
+        });
+        builder.Services.AddSingleton<IMessageHub>(Mesh);
+        var app = builder.Build();
+        app.MapWebhookInbox();
+        await app.StartAsync(ct);
+        return app;
+    }
+
+    private static Task<HttpResponseMessage> Post(WebApplication app, CancellationToken ct) =>
+        app.GetTestClient().PostAsync(Route,
+            new StringContent("{\"action\":\"edited\"}", Encoding.UTF8, "application/json"), ct);
+
+    /// <summary>
+    /// THE CHANGE: the existence read stalls, and the wire says "ask again later" with the shared
+    /// retry schedule. Unfixed, the exception escapes the handler.
+    /// </summary>
+    [Fact(Timeout = 300_000)]
+    public async Task AStalledTargetRead_Answers503WithRetryAfter()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        await using var app = await StartHost(ct);
+
+        provider.Stalling = true;
+        using var response = await Post(app, ct);
+
+        response.StatusCode.Should().Be(HttpStatusCode.ServiceUnavailable,
+            "the target read produced no answer — an availability failure that says it is "
+            + "retryable — so the wire must say 'ask again later', not report an unhandled 500");
+        response.Headers.RetryAfter.Should().NotBeNull("a 503 without Retry-After gives the sender no schedule");
+        response.Headers.RetryAfter!.Delta.Should().Be(
+            TimeSpan.FromSeconds(InstanceRegistryAuthenticator.RetryAfterSeconds),
+            "the inbox uses the same retry convention as the plugin-bundle routes");
+        using var json = JsonDocument.Parse(await response.Content.ReadAsStringAsync(ct));
+        json.RootElement.GetProperty("error").GetString().Should().Be(
+            WebhookInboxEndpoints.StalledReadError,
+            "the body must name what THIS route could not read, not the registry's catalogue");
+        provider.StalledReads.Should().BeGreaterThan(0,
+            "the stall must actually have been the target-existence read — otherwise this test "
+            + "measured something else");
+    }
+
+    /// <summary>
+    /// THE CONTROL: the same host, the provider answering, the target present — 200 accepted. A route
+    /// that answered 503 unconditionally would pass the case above and fail this one.
+    /// </summary>
+    [Fact(Timeout = 300_000)]
+    public async Task AnAnsweredTargetRead_StillAccepts()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        var access = Mesh.ServiceProvider.GetRequiredService<AccessService>();
+        // RunAsSystem, never Observable.Using(ImpersonateAsSystem, …) (#1790): Using opens the
+        // AsyncLocal scope on the subscribing thread and disposes it wherever the create's reply
+        // lands, which can leave this test's thread latched as System.
+        await access.RunAsSystem(
+                () => Mesh.ServiceProvider.GetRequiredService<IMeshService>().CreateOrUpdateNode(
+                    new MeshNode(Target)
+                    {
+                        Name = Target,
+                        NodeType = "Markdown",
+                        Content = new MarkdownContent { Content = "# Inbox target\n" },
+                    }))
+            .FirstAsync().Timeout(TestTimeouts.Convergence).Await(ct);
+        await using var app = await StartHost(ct);
+
+        provider.Stalling = false;
+        using var response = await Post(app, ct);
+
+        response.StatusCode.Should().Be(HttpStatusCode.OK);
+        using var json = JsonDocument.Parse(await response.Content.ReadAsStringAsync(ct));
+        json.RootElement.GetProperty("status").GetString().Should().Be("accepted");
+    }
+
+    /// <summary>
+    /// THE OTHER SIDE of the classification: an ordinary DEFECT on the same read still escapes, so a
+    /// real defect stays visible and only an availability fault becomes "retry later".
+    /// </summary>
+    [Fact(Timeout = 300_000)]
+    public async Task AnOrdinaryFault_OnTheTargetRead_IsNotConvertedTo503()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        await using var app = await StartHost(ct);
+
+        provider.Faulting = true;
+        HttpStatusCode? status = null;
+        Exception? escaped = null;
+        try
+        {
+            using var response = await Post(app, ct);
+            status = response.StatusCode;
+        }
+        catch (Exception ex)
+        {
+            // TestServer rethrows an unhandled pipeline exception to the client — that IS the escape.
+            escaped = ex;
+        }
+
+        (escaped is not null || status == HttpStatusCode.InternalServerError).Should().BeTrue(
+            $"a defect must still escape as one (status {status?.ToString() ?? "none"}, "
+            + $"exception {escaped?.GetType().Name ?? "none"})");
+        status.Should().NotBe(HttpStatusCode.ServiceUnavailable,
+            "only an availability fault may be answered 'retry later'");
+        if (escaped is not null)
+            escaped.ToString().Should().Contain(DefectMessage,
+                "the exception that escaped must be the target read's own defect, not something else");
+    }
+
+    /// <summary>
+    /// Claims every query and answers each with an empty Initial — except the target-existence query
+    /// while armed, which it leaves unanswered (<see cref="Stalling"/>: no Initial, no completion, no
+    /// error — the shape the fan-in's stall terminal exists for) or faults with an ordinary defect
+    /// (<see cref="Faulting"/>).
+    /// </summary>
+    private sealed class StallingTargetProvider(string targetQuery) : IMeshQueryProvider
+    {
+        private int stalling;
+        private int faulting;
+        private int stalledReads;
+
+        public string Name => nameof(StallingTargetProvider);
+
+        public bool Stalling
+        {
+            get => Volatile.Read(ref stalling) == 1;
+            set => Interlocked.Exchange(ref stalling, value ? 1 : 0);
+        }
+
+        public bool Faulting
+        {
+            get => Volatile.Read(ref faulting) == 1;
+            set => Interlocked.Exchange(ref faulting, value ? 1 : 0);
+        }
+
+        /// <summary>How many target reads were left unanswered.</summary>
+        public int StalledReads => Volatile.Read(ref stalledReads);
+
+        public bool Matches(IReadOnlyList<string> queryNamespaces) => true;
+
+        public IObservable<QueryResultChange<T>> Query<T>(
+            MeshQueryRequest request, JsonSerializerOptions options) =>
+            Observable.Defer(() =>
+            {
+                var isTargetRead = request.EffectiveQueries.Any(q =>
+                    string.Equals(q.Trim(), targetQuery, StringComparison.Ordinal));
+                if (isTargetRead && Faulting)
+                    return Observable.Throw<QueryResultChange<T>>(
+                        new InvalidOperationException(DefectMessage));
+                if (!isTargetRead || !Stalling)
+                    return Observable.Return(new QueryResultChange<T>
+                    {
+                        ChangeType = QueryChangeType.Initial,
+                        Items = Array.Empty<T>(),
+                        Timestamp = DateTimeOffset.UtcNow,
+                    });
+                Interlocked.Increment(ref stalledReads);
+                return Observable.Never<QueryResultChange<T>>();
+            });
+
+        public IObservable<IReadOnlyCollection<QueryResult>> Query(
+            MeshQueryRequest request, JsonSerializerOptions options)
+            => Observable.Return((IReadOnlyCollection<QueryResult>)Array.Empty<QueryResult>());
+
+        public IObservable<IReadOnlyCollection<QueryResult>> Autocomplete(
+            string basePath, string prefix, JsonSerializerOptions options,
+            AutocompleteMode mode = AutocompleteMode.RelevanceFirst, int limit = 10,
+            string? contextPath = null, string? context = null)
+            => Observable.Return((IReadOnlyCollection<QueryResult>)Array.Empty<QueryResult>());
+
+        public IObservable<T?> Select<T>(string path, string property, JsonSerializerOptions options)
+            => Observable.Return<T?>(default);
+    }
+}