From efaa35b690ffbd7258515717a6526c913fb06dfe Mon Sep 17 00:00:00 2001 From: Phil Scott Date: Fri, 29 May 2026 10:00:22 -0400 Subject: [PATCH] fix(build): stop build-time projection from caching an empty corpus on Windows On Windows, `dotnet run -- build` could ship an empty search index and header-only llms.txt while reporting success. AuditRunner.StartAsync fired its initial pass fire-and-forget; in build mode that pass self-fetched every route through the SiteProjection before the in-process TestServer had started. The self-fetch threw "server has not been started", RenderOneAsync swallowed it as a per-page skip, SeedAsync completed with an empty corpus, and AsyncLazy cached the poison that the search/llms emitters then consumed. Linux/dev won the startup race; Windows lost it deterministically. Two fixes: 1. Ordering: AuditRunner now runs its initial pass on IHostApplicationLifetime.ApplicationStarted instead of inside StartAsync, so the web server is guaranteed up before any self-fetch. Preserves build-time link-audit coverage. 2. Fail loud: add SelfFetchUnavailableException; HttpDispatcher.CreateClient throws it when the TestServer isn't started or Kestrel has no address. SiteProjection.RenderOneAsync no longer catches it, so an infrastructure failure faults SeedAsync (AsyncLazy evicts and retries) rather than caching an empty corpus as if the crawl had completed. Tests: HttpDispatcherTests pins the wrapped-exception behavior; SiteProjection faults-then-retries to a populated corpus instead of caching empty; AuditRunner tests updated for the ApplicationStarted gate. --- src/Pennington/Generation/AuditRunner.cs | 17 ++- .../Infrastructure/HttpDispatcher.cs | 24 +++- .../SelfFetchUnavailableException.cs | 25 ++++ src/Pennington/Pipeline/SiteProjection.cs | 7 +- .../Generation/AuditRunnerTests.cs | 26 ++++- .../Infrastructure/HttpDispatcherTests.cs | 43 +++++++ .../Pipeline/SiteProjectionTests.cs | 107 +++++++++++++++++- 7 files changed, 235 insertions(+), 14 deletions(-) create mode 100644 src/Pennington/Infrastructure/SelfFetchUnavailableException.cs create mode 100644 tests/Pennington.Tests/Infrastructure/HttpDispatcherTests.cs diff --git a/src/Pennington/Generation/AuditRunner.cs b/src/Pennington/Generation/AuditRunner.cs index 68d6b4d3..080d803c 100644 --- a/src/Pennington/Generation/AuditRunner.cs +++ b/src/Pennington/Generation/AuditRunner.cs @@ -19,6 +19,7 @@ public sealed class AuditRunner : IHostedService private readonly AuditCache _cache; private readonly IFileWatcher _fileWatcher; private readonly LocalizationOptions _localization; + private readonly IHostApplicationLifetime _lifetime; private readonly ILogger _logger; private readonly bool _isBuildMode; private readonly Lock _runLock = new(); @@ -30,8 +31,9 @@ public AuditRunner( AuditCache cache, IFileWatcher fileWatcher, LocalizationOptions localization, + IHostApplicationLifetime lifetime, ILogger logger) - : this(services, cache, fileWatcher, localization, logger, PenningtonBuildMode.WritesOutput) + : this(services, cache, fileWatcher, localization, lifetime, logger, PenningtonBuildMode.WritesOutput) { } @@ -41,6 +43,7 @@ internal AuditRunner( AuditCache cache, IFileWatcher fileWatcher, LocalizationOptions localization, + IHostApplicationLifetime lifetime, ILogger logger, bool isBuildMode) { @@ -48,6 +51,7 @@ internal AuditRunner( _cache = cache; _fileWatcher = fileWatcher; _localization = localization; + _lifetime = lifetime; _logger = logger; _isBuildMode = isBuildMode; } @@ -55,10 +59,13 @@ internal AuditRunner( /// public Task StartAsync(CancellationToken cancellationToken) { - // Prime the cache with an initial pass so the first request after startup - // already sees current diagnostics. Subsequent file changes invalidate via - // the IFileWatcher subscription below. - _activeRun = RunAsync(cancellationToken); + // Defer the initial pass until the application has fully started. A hosted service's + // StartAsync runs while sibling hosted services — including the web server that backs + // the in-process self-fetch — may not be up yet, so a build-mode pass that fetches + // rendered HTML through the projection would race the server start and fail (the empty + // result would then poison the projection's cache). ApplicationStarted fires only after + // every hosted service, the server included, has started. + _lifetime.ApplicationStarted.Register(RunInBackground); _fileWatcher.SubscribeToChanges(() => RunInBackground()); return Task.CompletedTask; } diff --git a/src/Pennington/Infrastructure/HttpDispatcher.cs b/src/Pennington/Infrastructure/HttpDispatcher.cs index 851fa87f..cc28a1f8 100644 --- a/src/Pennington/Infrastructure/HttpDispatcher.cs +++ b/src/Pennington/Infrastructure/HttpDispatcher.cs @@ -31,15 +31,33 @@ public HttpClient CreateClient() // Path-relative URLs ("/foo/bar") resolve against that and are dispatched // to the same RequestDelegate Kestrel would have invoked. Wrap its handler // with the cache so repeat self-fetches replay one render. - var testHandler = new CachingHttpHandler(_cache) { InnerHandler = testServer.CreateHandler() }; + HttpMessageHandler innerHandler; + try + { + innerHandler = testServer.CreateHandler(); + } + catch (InvalidOperationException ex) + { + // TestServer.Application is null until the host's IServer has started. A + // self-fetch issued before that — e.g. a startup hosted service racing the + // server start — is an infrastructure failure, not a per-page content error. + // Surface it as such so callers retry once the host is up instead of baking + // an empty corpus. + throw new SelfFetchUnavailableException( + "The in-process TestServer has not started yet; a self-fetch was issued before " + + "the host's server was ready.", + ex); + } + + var testHandler = new CachingHttpHandler(_cache) { InnerHandler = innerHandler }; return new HttpClient(testHandler) { BaseAddress = new Uri("http://localhost/") }; } var addresses = _server.Features.Get()?.Addresses; if (addresses is null || addresses.Count == 0) { - throw new InvalidOperationException( - "HttpDispatcher requires either a TestServer or a listening Kestrel host. " + + throw new SelfFetchUnavailableException( + "HttpDispatcher requires either a started TestServer or a listening Kestrel host. " + "IServerAddressesFeature has no addresses — is the app started yet?"); } diff --git a/src/Pennington/Infrastructure/SelfFetchUnavailableException.cs b/src/Pennington/Infrastructure/SelfFetchUnavailableException.cs new file mode 100644 index 00000000..22d54692 --- /dev/null +++ b/src/Pennington/Infrastructure/SelfFetchUnavailableException.cs @@ -0,0 +1,25 @@ +namespace Pennington.Infrastructure; + +/// +/// Thrown by when the in-process +/// transport is not ready — the host's +/// has not started yet (a TestServer whose application is still null, or a Kestrel host +/// that has not bound a listening address). Distinct from a per-page content failure: +/// site-crawling consumers (notably ) must let +/// this propagate so a partially-built or empty corpus is never cached as if the crawl had +/// completed. +/// +public sealed class SelfFetchUnavailableException : Exception +{ + /// Initializes the exception with a message describing why the transport is unavailable. + public SelfFetchUnavailableException(string message) + : base(message) + { + } + + /// Initializes the exception with a message and the underlying cause. + public SelfFetchUnavailableException(string message, Exception innerException) + : base(message, innerException) + { + } +} diff --git a/src/Pennington/Pipeline/SiteProjection.cs b/src/Pennington/Pipeline/SiteProjection.cs index b077d648..6a25a420 100644 --- a/src/Pennington/Pipeline/SiteProjection.cs +++ b/src/Pennington/Pipeline/SiteProjection.cs @@ -450,11 +450,16 @@ await Parallel.ForEachAsync( Content: fetched, Sections: BuildSectionsLazy(_extractor, fetched)); } - catch (Exception ex) + catch (Exception ex) when (ex is not SelfFetchUnavailableException) { _logger.LogWarning(ex, "SiteProjection: failed to project {Path}, skipping", toc.Route.CanonicalPath.Value); return null; } + // SelfFetchUnavailableException is deliberately NOT caught: the in-process server + // wasn't ready, which is an all-or-nothing infrastructure failure, not a per-page + // content error. Letting it propagate faults SeedAsync so AsyncLazy evicts the task + // and the next access retries — otherwise the whole corpus would be cached empty and + // the search index / llms.txt would silently ship with zero pages. } private static Lazy> BuildSectionsLazy( diff --git a/tests/Pennington.Tests/Generation/AuditRunnerTests.cs b/tests/Pennington.Tests/Generation/AuditRunnerTests.cs index 3d910892..d9ffd80f 100644 --- a/tests/Pennington.Tests/Generation/AuditRunnerTests.cs +++ b/tests/Pennington.Tests/Generation/AuditRunnerTests.cs @@ -1,5 +1,6 @@ using System.Runtime.CompilerServices; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging.Abstractions; using Pennington.Generation; using Pennington.Infrastructure; @@ -23,17 +24,24 @@ public async Task StartAsync_InBuildMode_RunsRenderedAuditor_AndCachesItsDiagnos using var sp = services.BuildServiceProvider(); var cache = sp.GetRequiredService(); + var lifetime = new StubLifetime(); var runner = new AuditRunner( sp, cache, sp.GetRequiredService(), sp.GetRequiredService(), + lifetime, NullLogger.Instance, isBuildMode: true); await runner.StartAsync(TestContext.Current.CancellationToken); - // RunAsync started in StartAsync; give the cache a moment to be populated. + // The initial pass is gated on ApplicationStarted so the server is up before any + // self-fetch; nothing should have run yet. + cache.Diagnostics.ShouldBeEmpty(); + lifetime.FireStarted(); + + // RunAsync started on ApplicationStarted; give the cache a moment to be populated. for (var i = 0; i < 50 && cache.Diagnostics.IsEmpty; i++) { await Task.Delay(10, TestContext.Current.CancellationToken); @@ -58,15 +66,18 @@ public async Task StartAsync_InDevMode_SkipsRenderedAuditors() using var sp = services.BuildServiceProvider(); var cache = sp.GetRequiredService(); + var lifetime = new StubLifetime(); var runner = new AuditRunner( sp, cache, sp.GetRequiredService(), sp.GetRequiredService(), + lifetime, NullLogger.Instance, isBuildMode: false); await runner.StartAsync(TestContext.Current.CancellationToken); + lifetime.FireStarted(); // Wait long enough for any background run to settle. await Task.Delay(100, TestContext.Current.CancellationToken); @@ -110,4 +121,17 @@ public void SubscribeToChanges(Action onUpdate) { } public void SubscribeToChanges(Action onUpdate) { } public void Dispose() { } } + + private sealed class StubLifetime : IHostApplicationLifetime + { + private readonly CancellationTokenSource _started = new(); + public CancellationToken ApplicationStarted => _started.Token; + public CancellationToken ApplicationStopping => CancellationToken.None; + public CancellationToken ApplicationStopped => CancellationToken.None; + public void StopApplication() { } + + // Mirrors the host firing ApplicationStarted once every hosted service (the server + // included) has started — the gate AuditRunner now waits on before its initial pass. + public void FireStarted() => _started.Cancel(); + } } \ No newline at end of file diff --git a/tests/Pennington.Tests/Infrastructure/HttpDispatcherTests.cs b/tests/Pennington.Tests/Infrastructure/HttpDispatcherTests.cs new file mode 100644 index 00000000..19aae1b6 --- /dev/null +++ b/tests/Pennington.Tests/Infrastructure/HttpDispatcherTests.cs @@ -0,0 +1,43 @@ +using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Pennington.Infrastructure; + +namespace Pennington.Tests.Infrastructure; + +public class HttpDispatcherTests +{ + [Fact] + public void CreateClient_UnstartedTestServer_ThrowsSelfFetchUnavailable() + { + // A TestServer whose host hasn't started has a null Application, so CreateHandler() + // throws InvalidOperationException. The dispatcher must surface that as the dedicated + // infrastructure failure (not a generic exception a per-page catch would swallow) so + // the projection retries instead of caching an empty corpus. This is the exact + // condition the Windows build-ordering bug hit when a startup hosted service raced + // the server start. + using var server = new TestServer(new ServiceCollection().BuildServiceProvider()); + var dispatcher = new HttpDispatcher(server, new BuildHtmlCache([])); + + Should.Throw(() => dispatcher.CreateClient()); + } + + [Fact] + public void CreateClient_NonTestServerWithoutAddresses_ThrowsSelfFetchUnavailable() + { + // The Kestrel path with no bound addresses is the same "server isn't ready" condition. + var dispatcher = new HttpDispatcher(new NoAddressServer(), new BuildHtmlCache([])); + + Should.Throw(() => dispatcher.CreateClient()); + } + + private sealed class NoAddressServer : IServer + { + public IFeatureCollection Features { get; } = new FeatureCollection(); + public void Dispose() { } + public Task StartAsync(IHttpApplication application, CancellationToken cancellationToken) + where TContext : notnull => Task.CompletedTask; + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + } +} diff --git a/tests/Pennington.Tests/Pipeline/SiteProjectionTests.cs b/tests/Pennington.Tests/Pipeline/SiteProjectionTests.cs index 48f9508a..b6f5abb3 100644 --- a/tests/Pennington.Tests/Pipeline/SiteProjectionTests.cs +++ b/tests/Pennington.Tests/Pipeline/SiteProjectionTests.cs @@ -5,6 +5,7 @@ namespace Pennington.Tests.Pipeline; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Primitives; +using Pennington.Content; using Pennington.Infrastructure; using Pennington.LlmsTxt; using Pennington.Pipeline; @@ -104,15 +105,53 @@ public async Task GetPageAsync_UnknownPath_ReturnsNull() page.ShouldBeNull(); } - private static SiteProjection CreateProjection(EndpointDataSource? endpointDataSource = null) + [Fact] + public async Task SelfFetchUnavailable_DuringSeed_FaultsRatherThanCachingEmptyCorpus() + { + // Regression for the Windows build-ordering bug: when the in-process server isn't + // started yet, the self-fetch throws SelfFetchUnavailableException. That must NOT be + // swallowed as a per-page skip (which would cache an empty corpus and silently ship a + // zero-page search index / llms.txt). It must fault the seed so AsyncLazy evicts and + // the next access — once the server is up — rebuilds the real corpus. + var dispatcher = new FlakyDispatcher(); + var projection = CreateProjection( + contentServices: [new SinglePageContentService()], + dispatcher: dispatcher); + var ct = TestContext.Current.CancellationToken; + + // First access: server not ready -> infrastructure failure propagates. + await Should.ThrowAsync(async () => + { + await foreach (var _ in projection.GetPagesAsync(ct)) + { + } + }); + + // Second access after the server is up: the faulted seed was evicted, so the + // projection rebuilds and yields the real page instead of an empty corpus. + dispatcher.ServerReady = true; + var pages = new List(); + await foreach (var page in projection.GetPagesAsync(ct)) + { + pages.Add(page); + } + + pages.Count.ShouldBe(1); + pages[0].Route.CanonicalPath.Value.ShouldBe("/page/"); + dispatcher.CreateClientCalls.ShouldBeGreaterThanOrEqualTo(2); + } + + private static SiteProjection CreateProjection( + EndpointDataSource? endpointDataSource = null, + IEnumerable? contentServices = null, + IInProcessHttpDispatcher? dispatcher = null) { - var dispatcher = new StubDispatcher(); return new SiteProjection( - contentServices: [], + contentServices: contentServices ?? [], enrichment: new MetadataEnrichmentService([]), renderer: new StubRenderer(), xrefResolver: new XrefResolvingService(new XrefResolver([])), - fetcher: new RenderedHtmlFetcher(dispatcher, NullLogger.Instance), + fetcher: new RenderedHtmlFetcher(dispatcher ?? new StubDispatcher(), NullLogger.Instance), extractor: new HeadingSectionExtractor(), options: new SiteProjectionOptions(), endpointDataSource: endpointDataSource ?? new StubEndpointDataSource(), @@ -155,4 +194,64 @@ protected override Task SendAsync(HttpRequestMessage reques => Task.FromResult(new HttpResponseMessage(System.Net.HttpStatusCode.NotFound)); } } + + // Throws the infrastructure failure until the "server" is marked ready, mirroring + // HttpDispatcher.CreateClient against a TestServer whose Application is not yet set. + private sealed class FlakyDispatcher : IInProcessHttpDispatcher + { + public bool ServerReady { get; set; } + public int CreateClientCalls { get; private set; } + + public HttpClient CreateClient() + { + CreateClientCalls++; + if (!ServerReady) + { + throw new SelfFetchUnavailableException("server not started (test)"); + } + + return new HttpClient(new OkHandler()) { BaseAddress = new Uri("http://localhost/") }; + } + + private sealed class OkHandler : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + => Task.FromResult(new HttpResponseMessage(System.Net.HttpStatusCode.OK) + { + Content = new StringContent( + "

Page

Body

", + System.Text.Encoding.UTF8, + "text/html"), + }); + } + } + + // Minimal content service yielding a single fetchable TOC entry (no LlmsOnlySource, + // so the projection takes the HTTP self-fetch path through the dispatcher). + private sealed class SinglePageContentService : IContentService + { + private static readonly ContentRoute Route = new() + { + CanonicalPath = new UrlPath("/page/"), + OutputFile = new FilePath("page/index.html"), + }; + + public IAsyncEnumerable DiscoverAsync() => AsyncEnumerable.Empty(); + + public Task> GetContentToCopyAsync() + => Task.FromResult(System.Collections.Immutable.ImmutableList.Empty); + + public Task> GetContentTocEntriesAsync() + => Task.FromResult(System.Collections.Immutable.ImmutableList.Create( + new ContentTocItem("Page", Route, 0, ["page"], null, null))); + + public Task> GetCrossReferencesAsync() + => Task.FromResult(System.Collections.Immutable.ImmutableList.Empty); + + public Task> GetContentToCreateAsync() + => Task.FromResult(System.Collections.Immutable.ImmutableList.Empty); + + public string DefaultSectionLabel => ""; + public int SearchPriority => 0; + } }