From 00b1ca619601302319659ab73d9b2b5dff359bc7 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Sat, 15 Aug 2026 01:08:22 -0500 Subject: [PATCH 1/3] Claiming was complete, tested, and wired to nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §8.5 says nothing sets game.is_claimed, so the listing badge and §7.5's ceiling grace have never been exercised. Claiming shipped in #16 and the note still read true, so I went looking for why. The claim logic was not the problem — it is complete and it has tests. The composition was. The site has two compositions of the same objects. mui-crawl builds the crawl loop by hand and passes every collaborator; the deployed site assembles it through DI. Three things were wrong with the second, and none of them could fail loudly: CrawlCycle takes its ClaimService as an OPTIONAL parameter, deliberately — a crawl with no database should do slightly less rather than refuse to run. The crawler graph registered no ClaimService, so a crawler-only deployment settled no beacons and said nothing about it. ClaimService was registered scoped, and the crawl loop that needs it is a singleton BackgroundService. A scoped dependency is one CrawlCycle can never legally be given: with scope validation on, which is what `dotnet run` does, the container refuses to build and THE SITE DOES NOT START with a connection string set. Production leaves validation off, so there it worked — by accident, and only there. That is also the answer to §8.5's note: is_claimed was being set in production and nowhere else. IClaimStore was registered nowhere at all. Account.razor service-locates it and reads a null as "this site has no database", so every operator's dashboard was empty on a site that had their claims, and /g/{slug}/claim/check threw on request. A service-located dependency fails silently by construction. Both services are stateless over a pooled NpgsqlDataSource, so both are now TryAddSingleton, registered by the crawler graph and the accounts graph alike — the one deployment that runs both gets one of each. CompositionTests resolves the graph Program builds, under scope validation, in both environments, and asserts the claim path is really joined. It fails on the parent commit in seven ways and is the only kind of test that could have caught any of this: every part was correct and the wiring between them was not. §8.5's closing note is rewritten to say what is actually true, and to name the general hazard — an optional dependency and a service-located one both fail silently when the composition is wrong, and the claim path has one of each. 830 tests over five suites, Postgres exercised. Testcontainers 4.13.0 -> 4.14.0 is the same one-line pickup as #26: SSH.NET 2025.1.0 is now advised against and NU1903 fails restore on main without it. Co-Authored-By: Claude Opus 5 --- Directory.Packages.props | 2 +- docs/specs/2026-07-30-mu-directory-design.md | 28 ++- .../CrawlerServiceCollectionExtensions.cs | 11 + src/MUI.Web/Accounts/Passkeys.cs | 24 +- tests/MUI.Web.Tests/CompositionTests.cs | 229 ++++++++++++++++++ 5 files changed, 285 insertions(+), 9 deletions(-) create mode 100644 tests/MUI.Web.Tests/CompositionTests.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index f2cb5ac..9f31a9a 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -64,7 +64,7 @@ never against a database has not been tested against the CHECK constraints that carry half the design, so the storage suite talks to a container or honestly skips. --> - + diff --git a/docs/specs/2026-07-30-mu-directory-design.md b/docs/specs/2026-07-30-mu-directory-design.md index 7d17b64..b8cd97f 100644 --- a/docs/specs/2026-07-30-mu-directory-design.md +++ b/docs/specs/2026-07-30-mu-directory-design.md @@ -818,10 +818,30 @@ a self-report with extra steps. Owner-published outputs: a live player-count SVG badge and a JSON endpoint for the game's own site. -**Claiming lights up two paths that are currently unreachable**, and that is worth knowing when -testing it: nothing sets `game.is_claimed` today, so the `claimed` badge in the listing and -`ArchivePolicy`'s ceiling-grace-for-claimed-games (§7.5) have never once been exercised against real -data. +**Claiming is wired end to end, and it was not for a while after it shipped.** A verified probe now +sets `game.is_claimed`, so the `claimed` badge in the listing and `ArchivePolicy`'s +ceiling-grace-for-claimed-games (§7.5) are both reachable. What kept them dark was not the claim +logic, which was complete and tested, but the **composition** — and the two are worth telling apart, +because the second has no compiler and no unit test looking at it: + +- The site has two compositions of the same objects. `mui-crawl` builds the crawl loop by hand and + passes every collaborator explicitly; the deployed site assembles it through DI. `CrawlCycle` takes + its `ClaimService` as an *optional* parameter — a crawl with no database behind it should do + slightly less rather than refuse to run — so a composition that omits it settles no beacons and + says nothing about it. A crawler-only deployment did exactly that. +- `ClaimService` was registered scoped while the crawl loop that needs it is a singleton + `BackgroundService`. With scope validation on, which is what `dotnet run` does, **the container + refused to build**: the site would not start with a connection string set. Production leaves + validation off, so there it worked — by accident, and only there. +- `IClaimStore` was registered nowhere at all. The dashboard service-locates it and reads a null as + "this site has no database", so every operator's list of claimed games was empty on a site that + had them, and the on-demand check endpoint threw on request. + +The lesson generalises past claiming: **an optional dependency and a service-located one both fail +silently when the composition is wrong**, and this codebase has one of each on the claim path. +`CompositionTests` resolves the graph `Program` builds, under scope validation, and asserts the +services these paths need are really there — a wiring test, because the wiring is what was broken +while every part it joined was correct. ## 9. Site surface, v1 diff --git a/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs b/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs index 657516c..db0558a 100644 --- a/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs +++ b/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs @@ -87,6 +87,17 @@ public static IServiceCollection AddMuiCrawlerCore(this IServiceCollection servi services.TryAddSingleton(s => s.GetRequiredService()); services.TryAddSingleton(s => s.GetRequiredService()); + // §8's claim settling. Every probe of a game whose owner has published a token completes the + // claim, and every probe of a claimed game refreshes beacon_last_seen_at — both on the + // ordinary schedule, which is why this belongs to the crawl graph rather than to the web + // tier that mints the tokens. + // + // It was missing, and CrawlCycle takes its ClaimService as an optional parameter, so a + // crawler-only deployment settled nothing at all and said nothing about it. mui-crawl passes + // one by hand, which is why its tests never noticed. + services.TryAddSingleton(s => new NpgsqlClaimStore(s.GetRequiredService())); + services.TryAddSingleton(); + // The three writers of §6.5, plus the field registry they judge staleness against. services.TryAddSingleton(FieldRegistry.Instance); services.TryAddSingleton(); diff --git a/src/MUI.Web/Accounts/Passkeys.cs b/src/MUI.Web/Accounts/Passkeys.cs index acb7618..12c5438 100644 --- a/src/MUI.Web/Accounts/Passkeys.cs +++ b/src/MUI.Web/Accounts/Passkeys.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection.Extensions; using MUI.Catalog; using MUI.Catalog.Persistence; @@ -52,10 +53,25 @@ public static IServiceCollection AddMuiAccounts( s.GetRequiredService(), s.GetRequiredService())); - services.AddScoped(s => new ClaimService( - new NpgsqlClaimStore(s.GetRequiredService()), - new NpgsqlGameStore(s.GetRequiredService()), - s.GetRequiredService())); + // Claiming's own two services, and both are singletons for a reason that is not tidiness. + // + // The dashboard resolves IClaimStore from the container and treats a null as "this site has + // no database" — so an unregistered store did not throw, it made every operator's list of + // claimed games empty on a site that had them. Nothing registered it, here or anywhere. + // + // ClaimService was scoped, and the crawl loop that settles beacons is a singleton + // BackgroundService: a scoped dependency is one CrawlCycle can never legally be given. With + // scope validation on — which is what `dotnet run` does — the container refused to build at + // all, so the site did not start with a connection string set. Both are stateless over a + // pooled NpgsqlDataSource, so a singleton is what they should always have been. + // + // TryAdd, because AddMuiCrawler registers the same pair for a crawler-only deployment and + // the two compositions overlap in the one that runs both. + services.TryAddSingleton(s => new NpgsqlClaimStore( + s.GetRequiredService())); + services.TryAddSingleton(s => new NpgsqlGameStore( + s.GetRequiredService())); + services.TryAddSingleton(); services.Configure(options => { diff --git a/tests/MUI.Web.Tests/CompositionTests.cs b/tests/MUI.Web.Tests/CompositionTests.cs new file mode 100644 index 0000000..7967178 --- /dev/null +++ b/tests/MUI.Web.Tests/CompositionTests.cs @@ -0,0 +1,229 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +using MUI.Catalog; +using MUI.Catalog.Persistence; +using MUI.Crawler; +using MUI.Web.Accounts; +using MUI.Web.Api; +using MUI.Web.Data; + +namespace MUI.Web.Tests; + +/// +/// The graph Program builds, resolved. +/// +/// +/// +/// This exists because the site has two compositions and only one of them was ever exercised. +/// mui-crawl constructs the crawl loop by hand and passes every collaborator explicitly, so +/// its claim path works and has tests; the deployed site assembles the same objects through DI, and +/// there a missing registration is not a compile error — it is a null in an optional parameter, a +/// service locator answering null, or an exception on the first request to one endpoint. +/// +/// +/// Every assertion below was a live hole when it was written. None of them could have been caught by +/// a unit test of the thing that was broken, because none of the things were broken: the wiring +/// between them was. +/// +/// +public class CompositionTests +{ + /// + /// A connection string is a string. Nothing here opens a connection — NpgsqlDataSource is + /// lazy — so the graph can be built and validated without a database. + /// + private const string ConnectionString = "Host=127.0.0.1;Port=1;Database=mui;Username=mui"; + + /// + /// The dashboard could not list a single claim, because nothing registered the store it reads. + /// + /// + /// Account.razor asks the container for and treats a null as + /// "this site has no database", which is exactly what it looked like: the page rendered, said + /// nothing was claimed, and was wrong for every operator who had ever claimed anything. A + /// service-located dependency fails silently by construction, which is why this is asserted at + /// the composition root rather than left to the page. + /// + [Test] + public async Task TheDashboardCanReachTheClaimsItListsAndTheEndpointThatChecksThem() + { + await using var site = Site(); + var provider = site.Services; + + await Assert.That(provider.GetService()).IsNotNull(); + await Assert.That(provider.GetService()).IsNotNull(); + } + + /// + /// The crawl loop can settle a claim, which is the only thing that ever verifies one. + /// + /// + /// + /// takes its as an optional parameter — a + /// crawl with no database behind it is a crawl doing slightly less, not one that should refuse to + /// run — and the deployed site registered no ClaimService the container could put there. + /// So the parameter took its default, SettleClaimsAsync returned on its first line, and no + /// probe has ever verified a claim on the one deployment that has operators. + /// + /// + /// The symptom named in §8.5 — nothing sets game.is_claimed, so the listing badge and + /// §7.5's ceiling grace are unexercised — is this, and it survived claiming shipping because + /// mui-crawl passes the service by hand and its tests construct the cycle the same way. + /// + /// + [Test] + public async Task TheHostedCrawlerIsGivenTheServiceThatVerifiesAClaim() + { + await using var site = Site(); + var provider = site.Services; + + var cycle = provider.GetRequiredService(); + + await Assert.That(ClaimsOf(cycle)).IsNotNull(); + } + + /// + /// The claim service is not scoped, because the thing that needs it most is a singleton. + /// + /// + /// A background service outlives every request scope, so a scoped is + /// one can never be given: with scope validation on it is a startup + /// failure, and with it off it is a captive dependency holding one scope's objects for the + /// lifetime of the process. Validation is switched on here so the wrong answer cannot pass. + /// + [Test] + public async Task TheGraphSurvivesScopeValidation() + { + await using var site = Site(); + var provider = site.Services; + + await Assert.That(provider.GetRequiredService()).IsNotNull(); + + await using var scope = provider.CreateAsyncScope(); + + await Assert.That(scope.ServiceProvider.GetRequiredService()).IsNotNull(); + } + + /// Everything the owner write path needs, from a request scope. + [Test] + public async Task TheOwnerWritePathResolves() + { + await using var site = Site(); + var provider = site.Services; + await using var scope = provider.CreateAsyncScope(); + + await Assert.That(scope.ServiceProvider.GetService()).IsNotNull(); + await Assert.That(scope.ServiceProvider.GetService()).IsNotNull(); + } + + /// + /// The same graph in Production, where nothing validates it for us. + /// + /// + /// Asserted separately because the two environments fail differently and only one of them fails + /// loudly. Development builds with ValidateOnBuild and refuses to start; Production starts + /// happily and throws the first time the hosted crawler reaches for the cycle, which is inside a + /// BackgroundService — so the site serves pages while the thing that gathers every fact on + /// it is dead. A reviewer reading only the test above could reasonably conclude this was a + /// development-only annoyance. + /// + [Test] + public async Task TheHostedCrawlerCanBeResolvedInProductionToo() + { + await using var site = Site(Environments.Production); + + await Assert.That(site.Services.GetRequiredService()).IsNotNull(); + } + + /// + /// The store the dashboard reads, asked for in the environment that can build a container. + /// + /// + /// Separated from the Development assertion so the two findings cannot be confused for one. The + /// lifetime mistake stops Development dead and leaves Production working; this one is a plain + /// missing registration and is missing in both, which is why every operator's dashboard was + /// empty on a running production site. + /// + [Test] + public async Task TheDashboardsClaimStoreIsMissingInEveryEnvironmentNotJustTheValidatedOne() + { + await using var site = Site(Environments.Production); + + await Assert.That(site.Services.GetService()).IsNotNull(); + } + + /// + /// The crawl loop is handed a claim service in Production, where the container tolerates it. + /// + /// + /// This is what makes §8.5's closing note stale rather than true: a production site does settle + /// beacons and does set game.is_claimed. It gets there by resolving a scoped service from + /// the root, which the container permits only because Production leaves scope validation off — so + /// the behaviour is correct by accident and stops the moment anybody switches validation on. + /// + [Test] + public async Task TheProductionCrawlLoopDoesGetItsClaimService() + { + await using var site = Site(Environments.Production); + + await Assert.That(ClaimsOf(site.Services.GetRequiredService())).IsNotNull(); + } + + /// + /// Program's service registrations, on Program's host, with a database configured. + /// + /// + /// + /// A real rather than a bare , + /// because half the framework's own registrations want IConfiguration and + /// IHostEnvironment and a hand-built collection fails validation on those before it can say + /// anything about ours. What is left after the host supplies them is our graph, and only ours. + /// + /// + /// Scope validation on, which is what WebApplication.CreateBuilder does by itself in + /// Development — so this is the check a developer already gets on dotnet run and a + /// production deployment does not. + /// + /// + /// Nothing is started and no migration runs: Program applies those after + /// , and this stops at the graph. + /// + /// + private static WebApplication Site(string? environment = null) + { + var builder = WebApplication.CreateBuilder(new WebApplicationOptions + { + EnvironmentName = environment ?? Environments.Development, + }); + + builder.Logging.ClearProviders(); + + builder.Services.AddRazorComponents(); + builder.Services.AddMuiApi(builder.Configuration); + builder.Services.AddPostgresCatalogue(ConnectionString); + builder.Services.AddMuiCrawler(ConnectionString, configure => configure.ApplyMigrations = false); + builder.Services.AddSingleton(new CatalogueSource(IsMeasured: true)); + builder.Services.AddMuiAccounts(builder.Configuration); + builder.Services.AddSingleton(TimeProvider.System); + + return builder.Build(); + } + + /// + /// The cycle's claim service, read off the instance. + /// + /// + /// Reflection, because the collaborator is a private primary-constructor parameter and the + /// property under test is precisely that the container supplied one rather than letting the + /// default stand. Exposing it publicly to be asserted on would widen the type for the test's + /// convenience; a wiring test is allowed to look at the wiring. + /// + private static object? ClaimsOf(CrawlCycle cycle) => + typeof(CrawlCycle) + .GetFields(System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic) + .Single(field => field.FieldType == typeof(ClaimService)) + .GetValue(cycle); +} From 9cdd5f8958589a569b8afe2146ad6023b89590cd Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Sat, 15 Aug 2026 09:58:22 -0500 Subject: [PATCH 2/3] Compose the test from Program's own graph, and make "look sooner" look MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on the composition fix. All four held; each is verified rather than taken. CompositionTests restated Program's registrations instead of running them, so a future divergence — a scoped service consumed by a singleton, AddMuiAccounts moving — would break the site while all the tests passed. That is exactly the failure this file exists to catch. The graph moves out of Program's top-level statements into SiteComposition.AddMuiSite/UseMuiSite, which the deployable and the test now both call, so there is one copy and nothing to diverge from. The on-demand check moved nothing. RequestCheckAsync wrote last_checked_at and a check_requested event; due-ness comes only from crawl_target.next_probe_at, so no probe ever came of it while the page said the button dialled a real server. A rate limiter on an action that does not happen is the most convincing possible no-op. IOnDemandProbes brings the game's targets forward with LEAST — an ask can only make a probe sooner — and the crawl loop still does the dialling under CRAWL DELAY and §7.2's address gate. Five Postgres tests assert on the schedule rather than on the audit log. The button now says what it does: "Look sooner", brings your game to the front of the queue, we dial on our own schedule. The remark on TheHostedCrawlerCanBeResolvedInProductionToo described a failure that never happened — it passes on the parent commit, because Production leaves scope validation off and resolved the scoped ClaimService from the root. It is a control, not a finding, and now says so. Measured: the original seven tests fail five ways on the parent, not seven. With the four added here, ten fail seven. "A crawler-only deployment" justified the new registrations in three places and describes nothing: AddMuiCrawler has exactly one caller, MUI.Web's Program, and mui-crawl builds its graph by hand. §4.11 has one deployable. The real consumer is the web tier's in-process CrawlCycle, and all three now say so. Two more while here. The demo composition had no test at all; it has one, and it asserts the claim surfaces are absent rather than broken and that the page still admits nothing on it was measured. And the web tier registered its own NpgsqlAvailabilityStore with AddSingleton, so the crawler's TryAdd was skipped and the concrete type and IReachableHistory pointed at a second instance — harmless on one pool and a direct contradiction of the crawler's own comment that two would be two connection paths answering one question. AddPostgresCatalogue is TryAdd throughout now, and one object answers to all three names. 838 tests over five suites, Postgres exercised. Co-Authored-By: Claude Opus 5 --- docs/specs/2026-07-30-mu-directory-design.md | 20 +- src/MUI.Catalog/Claims.cs | 27 +++ src/MUI.Catalog/Persistence/ClaimService.cs | 33 +++- .../CrawlerServiceCollectionExtensions.cs | 10 +- .../Persistence/NpgsqlOnDemandProbes.cs | 49 +++++ src/MUI.Web/Accounts/Passkeys.cs | 19 +- src/MUI.Web/Components/Pages/Claim.razor | 27 ++- src/MUI.Web/Data/PostgresData.cs | 29 ++- src/MUI.Web/Program.cs | 76 +------ src/MUI.Web/SiteComposition.cs | 122 ++++++++++++ .../OnDemandProbePostgresTests.cs | 187 ++++++++++++++++++ tests/MUI.Web.Tests/CompositionTests.cs | 142 ++++++++++--- 12 files changed, 617 insertions(+), 124 deletions(-) create mode 100644 src/MUI.Crawler/Persistence/NpgsqlOnDemandProbes.cs create mode 100644 src/MUI.Web/SiteComposition.cs create mode 100644 tests/MUI.Crawler.Tests/OnDemandProbePostgresTests.cs diff --git a/docs/specs/2026-07-30-mu-directory-design.md b/docs/specs/2026-07-30-mu-directory-design.md index b8cd97f..fd6f518 100644 --- a/docs/specs/2026-07-30-mu-directory-design.md +++ b/docs/specs/2026-07-30-mu-directory-design.md @@ -828,7 +828,8 @@ because the second has no compiler and no unit test looking at it: passes every collaborator explicitly; the deployed site assembles it through DI. `CrawlCycle` takes its `ClaimService` as an *optional* parameter — a crawl with no database behind it should do slightly less rather than refuse to run — so a composition that omits it settles no beacons and - says nothing about it. A crawler-only deployment did exactly that. + says nothing about it. The crawl graph omitted it, and the site only had one because the accounts + module happened to register the same type for the dashboard. - `ClaimService` was registered scoped while the crawl loop that needs it is a singleton `BackgroundService`. With scope validation on, which is what `dotnet run` does, **the container refused to build**: the site would not start with a connection string set. Production leaves @@ -839,9 +840,20 @@ because the second has no compiler and no unit test looking at it: The lesson generalises past claiming: **an optional dependency and a service-located one both fail silently when the composition is wrong**, and this codebase has one of each on the claim path. -`CompositionTests` resolves the graph `Program` builds, under scope validation, and asserts the -services these paths need are really there — a wiring test, because the wiring is what was broken -while every part it joined was correct. +`CompositionTests` resolves the graph `Program` builds — literally, by calling the same +`AddMuiSite` the deployable calls, because a harness that restated the registrations would be a +second copy that agrees with the first only until somebody edits one of them — under scope +validation, in both environments, and asserts the services these paths need are really there. A +wiring test, because the wiring is what was broken while every part it joined was correct. + +**The on-demand check was the same shape of gap and is fixed with it.** §8.1 offers a claimant one +requested probe per few minutes; `RequestCheckAsync` wrote `last_checked_at` and a `check_requested` +event, and due-ness comes only from `crawl_target.next_probe_at`, so nothing was ever probed and the +page said the button dialled a real server. A rate limiter on an action that does not happen is the +most convincing possible no-op. `IOnDemandProbes` brings the game's targets forward instead — +`LEAST`, so an ask can only make a probe sooner — and the crawl loop still does the dialling under +`CRAWL DELAY` and §7.2's address gate, which is what keeps a button on a page from becoming a way to +make us connect to a stranger's server. ## 9. Site surface, v1 diff --git a/src/MUI.Catalog/Claims.cs b/src/MUI.Catalog/Claims.cs index 4983861..bf1e2a8 100644 --- a/src/MUI.Catalog/Claims.cs +++ b/src/MUI.Catalog/Claims.cs @@ -207,3 +207,30 @@ public enum ClaimVerdict /// A token we issued, matching a claim that has expired or been revoked. Stale, } + +/// +/// Brings a game's next probe forward, for §8.1's on-demand check. +/// +/// +/// +/// An interface here and an implementation over crawl_target in MUI.Crawler, because +/// due-ness is the crawl registry's business and MUI.Catalog may not know a socket exists. +/// decides whether a claimant may ask; this decides nothing and +/// only moves the schedule. +/// +/// +/// It brings a probe forward; it does not dial. The crawler's own loop still does the +/// dialling, still honours CRAWL DELAY and still refuses a target outside scope — so a button +/// on a page cannot become a way to make us connect to a stranger's server on demand. What §8.1 asks +/// for is that an operator who has just edited mush.cnf is not left waiting on the scheduler, +/// and that is exactly this and no more. +/// +/// +public interface IOnDemandProbes +{ + /// + /// Asks for to be probed no later than . + /// + /// Whether any target was brought forward. + Task BringForwardAsync(Guid gameId, DateTimeOffset at, CancellationToken cancellationToken = default); +} diff --git a/src/MUI.Catalog/Persistence/ClaimService.cs b/src/MUI.Catalog/Persistence/ClaimService.cs index 51bf7e7..c4f8396 100644 --- a/src/MUI.Catalog/Persistence/ClaimService.cs +++ b/src/MUI.Catalog/Persistence/ClaimService.cs @@ -17,7 +17,11 @@ namespace MUI.Catalog.Persistence; /// hands this a string and a channel. /// /// -public sealed class ClaimService(IClaimStore claims, IGameStore games, TimeProvider time) +public sealed class ClaimService( + IClaimStore claims, + IGameStore games, + TimeProvider time, + IOnDemandProbes? probes = null) { /// /// How often a claimant may ask us to look again (spec §8.1). @@ -153,7 +157,24 @@ public bool MayRecheck(GameClaim claim) return claim.LastCheckedAt is not { } last || time.GetUtcNow() - last >= RecheckInterval; } - /// Records that a check was asked for. The dialling itself belongs to the crawler. + /// + /// Brings the game's next probe forward, and records that the claimant asked. + /// + /// + /// + /// It moves the schedule; the crawler still does the dialling. Keeping the two apart is + /// what stops a button on a page becoming a way to make us connect to a stranger's server on + /// demand: the rate limit is per claim, a claim cannot exist for a game nobody has been offered, + /// and CRAWL DELAY and the address gate both still bind when the loop gets there. + /// + /// + /// This recorded the ask and moved nothing for as long as it existed. Due-ness comes only from + /// crawl_target.next_probe_at, so last_checked_at and the check_requested + /// event went into the database and no probe ever came of it, while the page told the operator + /// the button dialled a real server. A rate limiter on an action that does not happen is the + /// most convincing possible no-op. + /// + /// public async Task RequestCheckAsync(Guid claimId, CancellationToken cancellationToken = default) { if (await claims.FindAsync(claimId, cancellationToken) is not { } claim || !MayRecheck(claim)) @@ -163,11 +184,19 @@ public async Task RequestCheckAsync(Guid claimId, CancellationToken cancel var now = time.GetUtcNow(); + // Recorded whether or not a target moved. An ask is a thing that happened, and a claim on a + // game the registry has no target for — merged away, or added by hand — is not the + // claimant's mistake to be told about in an audit log. await claims.UpdateAsync(claim with { LastCheckedAt = now }, cancellationToken); await claims.RecordEventAsync( new ClaimEvent(claim.Id, now, ClaimEventKind.CheckRequested), cancellationToken); + if (probes is not null) + { + await probes.BringForwardAsync(claim.GameId, now, cancellationToken); + } + return true; } diff --git a/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs b/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs index db0558a..2fcf1ef 100644 --- a/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs +++ b/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs @@ -92,10 +92,14 @@ public static IServiceCollection AddMuiCrawlerCore(this IServiceCollection servi // ordinary schedule, which is why this belongs to the crawl graph rather than to the web // tier that mints the tokens. // - // It was missing, and CrawlCycle takes its ClaimService as an optional parameter, so a - // crawler-only deployment settled nothing at all and said nothing about it. mui-crawl passes - // one by hand, which is why its tests never noticed. + // The consumer is the in-process CrawlCycle of the one deployable (§4.11) — this method has + // exactly one caller, MUI.Web's Program, and mui-crawl builds its own graph by hand. It was + // missing here, and CrawlCycle takes its ClaimService as an OPTIONAL parameter, so the site + // settled beacons only insofar as some other registration happened to supply one. That is + // the shape of the bug rather than an argument for a deployment nobody builds. services.TryAddSingleton(s => new NpgsqlClaimStore(s.GetRequiredService())); + services.TryAddSingleton( + s => new NpgsqlOnDemandProbes(s.GetRequiredService())); services.TryAddSingleton(); // The three writers of §6.5, plus the field registry they judge staleness against. diff --git a/src/MUI.Crawler/Persistence/NpgsqlOnDemandProbes.cs b/src/MUI.Crawler/Persistence/NpgsqlOnDemandProbes.cs new file mode 100644 index 0000000..6d2d5b9 --- /dev/null +++ b/src/MUI.Crawler/Persistence/NpgsqlOnDemandProbes.cs @@ -0,0 +1,49 @@ +using Dapper; + +using MUI.Catalog; + +using Npgsql; + +namespace MUI.Crawler.Persistence; + +/// +/// §8.1's on-demand check, against the crawl registry: bring this game's next probe forward. +/// +/// +/// +/// LEAST rather than an assignment, so an ask can only ever make a probe sooner. A target +/// already overdue stays where it is in the queue rather than being pushed back to now, and a +/// claimant pressing the button twice cannot walk their own game backwards. +/// +/// +/// Every endpoint of the game moves, because a game is claimed and a target is an address: an +/// operator who has just edited mush.cnf does not know or care which of their listeners we +/// happen to have a row for. +/// +/// +/// It schedules and does not dial. The crawl loop picks the target up on its next pass, where +/// CRAWL DELAY, the concurrency cap and §7.2's resolved-address gate all still apply — none +/// of which a page may bypass. +/// +/// +public sealed class NpgsqlOnDemandProbes(NpgsqlDataSource source) : IOnDemandProbes +{ + public async Task BringForwardAsync( + Guid gameId, + DateTimeOffset at, + CancellationToken cancellationToken = default) + { + await using var connection = await source.OpenConnectionAsync(cancellationToken); + + var moved = await connection.ExecuteAsync(new CommandDefinition( + """ + UPDATE crawl_target + SET next_probe_at = LEAST(next_probe_at, @at) + WHERE game_id = @gameId + """, + new { gameId, at = at.ToUniversalTime() }, + cancellationToken: cancellationToken)); + + return moved > 0; + } +} diff --git a/src/MUI.Web/Accounts/Passkeys.cs b/src/MUI.Web/Accounts/Passkeys.cs index 12c5438..4b73cf9 100644 --- a/src/MUI.Web/Accounts/Passkeys.cs +++ b/src/MUI.Web/Accounts/Passkeys.cs @@ -65,8 +65,9 @@ public static IServiceCollection AddMuiAccounts( // all, so the site did not start with a connection string set. Both are stateless over a // pooled NpgsqlDataSource, so a singleton is what they should always have been. // - // TryAdd, because AddMuiCrawler registers the same pair for a crawler-only deployment and - // the two compositions overlap in the one that runs both. + // TryAdd, because AddMuiCrawler registers the same pair for the crawl loop and this method + // registers it for the dashboard — one deployable calls both (§4.11), and neither may end up + // with a second instance or a second lifetime. services.TryAddSingleton(s => new NpgsqlClaimStore( s.GetRequiredService())); services.TryAddSingleton(s => new NpgsqlGameStore( @@ -212,11 +213,15 @@ await signIn.MakePasskeyRequestOptionsAsync(user: null), : Results.Unauthorized(); }); - // §8.1's on-demand check. It does not dial anything itself — it records that the claimant - // asked, and the crawler's own scheduler is what brings the probe forward. Keeping the two - // apart is what stops a button on a public page becoming a way to make us connect to a - // stranger's server on demand: the rate limit is per claim, and a claim cannot exist for a - // game nobody has been offered. + // §8.1's on-demand check. It does not dial anything itself — it brings the game's crawl + // targets forward and the crawler's own loop does the dialling, under CRAWL DELAY and §7.2's + // address gate. Keeping the two apart is what stops a button on a public page becoming a way + // to make us connect to a stranger's server on demand: the rate limit is per claim, and a + // claim cannot exist for a game nobody has been offered. + // + // For as long as this existed it moved nothing at all — due-ness comes only from + // crawl_target.next_probe_at, and this wrote last_checked_at and an audit event. See + // ClaimService.RequestCheckAsync and IOnDemandProbes. app.MapPost("/g/{slug}/claim/check", async ( HttpContext context, UserManager users, diff --git a/src/MUI.Web/Components/Pages/Claim.razor b/src/MUI.Web/Components/Pages/Claim.razor index 5e0a094..d5fdef2 100644 --- a/src/MUI.Web/Components/Pages/Claim.razor +++ b/src/MUI.Web/Components/Pages/Claim.razor @@ -100,14 +100,25 @@ else
- - @if (!CanCheck) - { - - Just looked. You can ask again in a few minutes — the button dials a real - server, so it is rationed. - - } + + @* + It moves your game to the front of the crawl queue; the crawler still does the + dialling, on its own schedule and under CRAWL DELAY. Saying it "dials a real + server" was both an overstatement and — for as long as nothing moved the + schedule — a description of something that did not happen at all. + *@ + + @if (CanCheck) + { + @("Brings your game to the front of the queue. We dial on our own schedule, " + + "so this is sooner rather than now.") + } + else + { + @("Just asked. You can ask again in a few minutes — it is rationed because it " + + "makes us connect to a real server sooner than we would have.") + } + } diff --git a/src/MUI.Web/Data/PostgresData.cs b/src/MUI.Web/Data/PostgresData.cs index aed984c..d71afc7 100644 --- a/src/MUI.Web/Data/PostgresData.cs +++ b/src/MUI.Web/Data/PostgresData.cs @@ -1,5 +1,8 @@ using MUI.Catalog; using MUI.Catalog.Persistence; + +using Microsoft.Extensions.DependencyInjection.Extensions; + using Npgsql; namespace MUI.Web.Data; @@ -49,13 +52,27 @@ public static class PostgresData /// public static void AddPostgresCatalogue(this IServiceCollection services, string connectionString) { - services.AddSingleton(_ => NpgsqlDataSource.Create(connectionString)); + ArgumentNullException.ThrowIfNull(services); + + // TryAdd throughout, and the availability store registered once and exposed through its + // interfaces rather than newed per interface. Both matter because AddMuiCrawler registers + // the same objects for the crawl loop and one deployable calls both (§4.11): with AddSingleton + // this method won the IAvailabilityStore registration while the crawler's TryAdd was skipped, + // leaving the concrete type and IReachableHistory pointing at a SECOND instance. Harmless on + // one pool, and a direct contradiction of the comment in the crawler that says the store is + // registered once because two would be two connection paths answering one question. + services.TryAddSingleton(_ => NpgsqlDataSource.Create(connectionString)); + + // The registry is a lookup table with no state, so the shared instance rather than a second + // one — which is also what the crawler registers. + services.TryAddSingleton(FieldRegistry.Instance); + + services.TryAddSingleton(s => new NpgsqlAvailabilityStore(s.GetRequiredService())); + services.TryAddSingleton(s => s.GetRequiredService()); + services.TryAddSingleton(s => s.GetRequiredService()); + services.TryAddSingleton(); - services.AddSingleton(); - services.AddSingleton(s => - new NpgsqlAvailabilityStore(s.GetRequiredService())); - services.AddSingleton(); - services.AddSingleton(s => new NpgsqlGameQueries( + services.TryAddSingleton(s => new NpgsqlGameQueries( s.GetRequiredService(), s.GetRequiredService())); } diff --git a/src/MUI.Web/Program.cs b/src/MUI.Web/Program.cs index 0082294..526ad05 100644 --- a/src/MUI.Web/Program.cs +++ b/src/MUI.Web/Program.cs @@ -1,60 +1,17 @@ -using MUI.Catalog; -using MUI.Crawler; -using MUI.Web.Accounts; -using MUI.Web.Api; -using MUI.Web.Components; +using MUI.Web; using MUI.Web.Data; -using MUI.Web.Fixtures; var builder = WebApplication.CreateBuilder(args); -builder.Services.AddRazorComponents(); - -// The read API (spec §10) reads through the same IGameQueries the pages do, so the two surfaces -// cannot disagree about a fact. What it adds of its own — the dataset licence, the slug aliases and -// the attribution list — is configuration, because none of it is a measurement. -builder.Services.AddMuiApi(builder.Configuration); - -// The site reads through IGameQueries and nothing else, and it prefers a real database to a -// fixture. Point MUI_POSTGRES (or ConnectionStrings:MUIndex) at the catalogue the crawler writes and -// every page renders measurements; without one the site still starts, but it says loudly and on -// every page that what it is showing was not measured. -// -// The fixture is not a fallback so much as a confession. A directory whose whole claim is that its -// data is measured must never quietly present invented data as though it were real — so the demo -// path is opt-in by absence, announced in the log, and marked in the page itself. +// The site reads through IGameQueries and nothing else, and it prefers a real database to a fixture. +// Point MUI_POSTGRES (or ConnectionStrings:MUIndex) at the catalogue the crawler writes and every +// page renders measurements; without one the site still starts, but it says loudly and on every page +// that what it is showing was not measured. var connectionString = PostgresData.ResolveConnectionString(builder.Configuration); -if (connectionString is not null) -{ - builder.Services.AddPostgresCatalogue(connectionString); - builder.Services.AddMuiCrawler(connectionString, configure => - { - // MUI.Web already applies migrations during startup; the hosted crawler should not repeat - // them when it takes the lease. - configure.ApplyMigrations = false; - }); -} -else -{ - builder.Services.AddSingleton(); - builder.Services.AddSingleton(s => s.GetRequiredService()); - builder.Services.AddSingleton(s => s.GetRequiredService()); -} - -builder.Services.AddSingleton(new CatalogueSource(connectionString is not null)); - -// Claiming needs a database: an account, a passkey and a claim are all rows (spec §8). Against the -// demo fixture the sign-in and claim surfaces are simply absent rather than present and broken — -// half a claim flow over invented games would be a worse answer than none. -if (connectionString is not null) -{ - builder.Services.AddMuiAccounts(builder.Configuration); -} - -// Ages are relative to a clock, and a clock is a dependency like any other — the plain surface and -// the rendered page must not each reach for DateTimeOffset.UtcNow and disagree by a tick. -builder.Services.AddSingleton(TimeProvider.System); +// The graph itself lives in SiteComposition, so that CompositionTests can resolve THE SAME +// registrations rather than a copy of them that agrees until somebody edits one of the two. +builder.Services.AddMuiSite(builder.Configuration, connectionString); var app = builder.Build(); @@ -73,22 +30,7 @@ PostgresData.EnvironmentVariable, PostgresData.ConfigurationKey); } -app.UseStaticFiles(); - -// Razor Components register anti-forgery metadata on every endpoint, so the middleware has to be -// present even though this site has no POST form yet. The facet panel is a GET form deliberately — -// a filter is a bookmarkable question, not a state change — so nothing here is token-protected. -app.UseAntiforgery(); - -if (connectionString is not null) -{ - app.UseAuthentication(); - app.UseAuthorization(); - app.MapMuiAccounts(); -} - -app.MapRazorComponents(); -app.MapMuiApi(); +app.UseMuiSite(connectionString); app.Run(); diff --git a/src/MUI.Web/SiteComposition.cs b/src/MUI.Web/SiteComposition.cs new file mode 100644 index 0000000..291fb55 --- /dev/null +++ b/src/MUI.Web/SiteComposition.cs @@ -0,0 +1,122 @@ +using MUI.Catalog; +using MUI.Crawler; +using MUI.Web.Accounts; +using MUI.Web.Api; +using MUI.Web.Components; +using MUI.Web.Data; +using MUI.Web.Fixtures; + +namespace MUI.Web; + +/// +/// Everything the deployable is made of, as two calls Program makes and a test can make too. +/// +/// +/// +/// This exists so that the composition has one spelling. The graph used to live in +/// Program.cs's top-level statements, where nothing but the running site could reach it — so +/// a test of the composition had to restate it, and a restatement is a second copy that +/// agrees with the first only until somebody edits one of them. The failure that motivates the whole +/// of CompositionTests is a registration nobody noticed was wrong; a harness that mirrored +/// Program would have gone on passing through exactly that edit. +/// +/// +/// Program.cs keeps what has side effects — reading the connection string, applying +/// migrations, saying in the log which of the two worlds it is in — because those are things a +/// process does on the way up rather than parts of the graph. +/// +/// +public static class SiteComposition +{ + /// + /// Registers the whole site, with a database behind it or on the demo fixture. + /// + /// The host's service collection. + /// The host's configuration. + /// + /// A PostgreSQL connection string, or null for the demo fixture. Null is not a fallback so much + /// as a confession: a directory whose whole claim is that its data is measured must never + /// quietly present invented data as though it were real, so the demo path is opt-in by absence, + /// announced in the log, and marked on every page through . + /// + public static IServiceCollection AddMuiSite( + this IServiceCollection services, + IConfiguration configuration, + string? connectionString) + { + ArgumentNullException.ThrowIfNull(services); + ArgumentNullException.ThrowIfNull(configuration); + + services.AddRazorComponents(); + + // The read API (spec §10) reads through the same IGameQueries the pages do, so the two + // surfaces cannot disagree about a fact. What it adds of its own — the dataset licence, the + // slug aliases and the attribution list — is configuration, because none of it is a + // measurement. + services.AddMuiApi(configuration); + + // Ages are relative to a clock, and a clock is a dependency like any other — the plain + // surface and the rendered page must not each reach for DateTimeOffset.UtcNow and disagree + // by a tick. Registered before the two worlds below, because both want it. + services.AddSingleton(TimeProvider.System); + + if (connectionString is not null) + { + services.AddPostgresCatalogue(connectionString); + services.AddMuiCrawler(connectionString, configure => + { + // MUI.Web already applies migrations during startup; the hosted crawler should not + // repeat them when it takes the lease. + configure.ApplyMigrations = false; + }); + + // Claiming needs a database: an account, a passkey and a claim are all rows (spec §8). + services.AddMuiAccounts(configuration); + } + else + { + // Against the demo fixture the sign-in and claim surfaces are simply absent rather than + // present and broken — half a claim flow over invented games would be a worse answer + // than none. + services.AddSingleton(); + services.AddSingleton(s => s.GetRequiredService()); + services.AddSingleton(s => s.GetRequiredService()); + } + + services.AddSingleton(new CatalogueSource(connectionString is not null)); + + return services; + } + + /// + /// The middleware and the routes, in the order they have to be in. + /// + /// + /// The order is part of the composition and not a detail of it, so it lives here with the rest of + /// it rather than in a file only the running process reads. + /// + public static WebApplication UseMuiSite(this WebApplication app, string? connectionString) + { + ArgumentNullException.ThrowIfNull(app); + + app.UseStaticFiles(); + + // Razor Components register anti-forgery metadata on every endpoint, so the middleware has + // to be present even though this site has no POST form yet. The facet panel is a GET form + // deliberately — a filter is a bookmarkable question, not a state change — so nothing here + // is token-protected. + app.UseAntiforgery(); + + if (connectionString is not null) + { + app.UseAuthentication(); + app.UseAuthorization(); + app.MapMuiAccounts(); + } + + app.MapRazorComponents(); + app.MapMuiApi(); + + return app; + } +} diff --git a/tests/MUI.Crawler.Tests/OnDemandProbePostgresTests.cs b/tests/MUI.Crawler.Tests/OnDemandProbePostgresTests.cs new file mode 100644 index 0000000..9e25ddb --- /dev/null +++ b/tests/MUI.Crawler.Tests/OnDemandProbePostgresTests.cs @@ -0,0 +1,187 @@ +using Dapper; + +using MUI.Catalog; +using MUI.Catalog.Persistence; +using MUI.Crawler.Persistence; +using MUI.Crawler.Tests.Support; +using MUI.Discovery; + +namespace MUI.Crawler.Tests; + +/// +/// §8.1's on-demand check, against the table that decides what gets probed. +/// +/// +/// The claimant-facing half of this was written first and moved nothing: RequestCheckAsync +/// wrote last_checked_at and a check_requested event, due-ness comes only from +/// crawl_target.next_probe_at, and the page told the operator the button dialled their +/// server. A rate limiter on an action that does not happen is the most convincing possible no-op, +/// so these assert on the schedule rather than on the audit log. +/// +public class OnDemandProbePostgresTests +{ + /// An owner who has just edited mush.cnf is not left waiting on the scheduler. + [Test] + public async Task AskingForACheckBringsTheGamesTargetForward() + { + await using var db = await PostgresFixture.MigratedAsync(); + var now = DateTimeOffset.UtcNow; + + var (game, target) = await SeedAsync(db, next: now.AddHours(6)); + + var moved = await new NpgsqlOnDemandProbes(db.DataSource).BringForwardAsync(game, now); + + await Assert.That(moved).IsTrue(); + await Assert.That(await NextProbeAsync(db, target)).IsEqualTo(now).Within(TimeSpan.FromSeconds(1)); + } + + /// + /// An ask can only make a probe sooner, never later. + /// + /// + /// LEAST rather than an assignment. A target already overdue keeps its place in the queue + /// — pushing it back to "now" would let a claimant pressing the button walk their own game + /// backwards, and would let a busy game's ordinary schedule be reset by anyone who claimed it. + /// + [Test] + public async Task AnAskNeverPushesAProbeBack() + { + await using var db = await PostgresFixture.MigratedAsync(); + var now = DateTimeOffset.UtcNow; + var overdue = now.AddHours(-3); + + var (game, target) = await SeedAsync(db, next: overdue); + + await new NpgsqlOnDemandProbes(db.DataSource).BringForwardAsync(game, now); + + await Assert.That(await NextProbeAsync(db, target)) + .IsEqualTo(overdue).Within(TimeSpan.FromSeconds(1)); + } + + /// + /// Every address of the game moves, because a game is claimed and a target is an address. + /// + /// + /// A game may have several endpoints (§5.5) and an operator does not know or care which of them + /// we happen to hold a row for. Moving one would make the button work or not depending on which + /// listener the crawler had found first. + /// + [Test] + public async Task EveryTargetForTheGameMoves() + { + await using var db = await PostgresFixture.MigratedAsync(); + var now = DateTimeOffset.UtcNow; + + var (game, first) = await SeedAsync(db, next: now.AddHours(6)); + var second = await AttachAsync(db, game, "other.example.org", 4202, now.AddHours(9)); + + await new NpgsqlOnDemandProbes(db.DataSource).BringForwardAsync(game, now); + + await Assert.That(await NextProbeAsync(db, first)).IsEqualTo(now).Within(TimeSpan.FromSeconds(1)); + await Assert.That(await NextProbeAsync(db, second)).IsEqualTo(now).Within(TimeSpan.FromSeconds(1)); + } + + /// + /// A game the registry has no target for is not an error, and says so by answering false. + /// + /// + /// It happens: a game merged away under §7.3, or one added to the catalogue by a path that never + /// minted a target. The claimant's ask is still recorded — it is a thing that happened — and the + /// caller learns nothing moved rather than being handed an exception to render. + /// + [Test] + public async Task AGameWithNoTargetMovesNothingAndDoesNotThrow() + { + await using var db = await PostgresFixture.MigratedAsync(); + + var moved = await new NpgsqlOnDemandProbes(db.DataSource) + .BringForwardAsync(Guid.CreateVersion7(), DateTimeOffset.UtcNow); + + await Assert.That(moved).IsFalse(); + } + + /// + /// One game's ask does not disturb another's schedule. + /// + [Test] + public async Task AnAskTouchesOnlyTheGameItNames() + { + await using var db = await PostgresFixture.MigratedAsync(); + var now = DateTimeOffset.UtcNow; + var later = now.AddHours(6); + + var (mine, _) = await SeedAsync(db, next: later); + var (_, theirs) = await SeedAsync(db, next: later, slug: "other", host: "third.example.org"); + + await new NpgsqlOnDemandProbes(db.DataSource).BringForwardAsync(mine, now); + + await Assert.That(await NextProbeAsync(db, theirs)) + .IsEqualTo(later).Within(TimeSpan.FromSeconds(1)); + } + + private static async Task<(Guid Game, Guid Target)> SeedAsync( + TestDatabase db, + DateTimeOffset next, + string slug = "corvid", + string host = "mud.example.org") + { + var game = Guid.CreateVersion7(); + + await new NpgsqlGameStore(db.DataSource).InsertAsync(new GameRecord( + game, + slug, + slug, + Tagline: null, + LifecycleState.Active, + IsClaimed: true, + FirstSeenAt: DateTimeOffset.UtcNow.AddYears(-1), + LastReachableAt: null, + ArchivedAt: null)); + + return (game, await AttachAsync(db, game, host, 4201, next)); + } + + private static async Task AttachAsync( + TestDatabase db, + Guid game, + string host, + int port, + DateTimeOffset next) + { + var targets = new NpgsqlCrawlTargetRepository(db.DataSource); + + var id = await targets.AddAsync( + new CrawlTarget + { + Id = Guid.CreateVersion7(), + Host = host, + Port = port, + Depth = 0, + NextProbeAt = next, + FirstSeenAt = DateTimeOffset.UtcNow, + }, + CancellationToken.None); + + await targets.AttachGameAsync(id, game, CancellationToken.None); + + return id; + } + + /// + /// The stored schedule, read back. + /// + /// + /// Through , because Npgsql hands back a DateTime for a + /// timestamptz and asking Dapper for a is an invalid cast — + /// the same thing NpgsqlClaimStore's own row type exists to work around. + /// + private static async Task NextProbeAsync(TestDatabase db, Guid target) + { + await using var connection = await db.DataSource.OpenConnectionAsync(); + + var at = await connection.ExecuteScalarAsync( + "SELECT next_probe_at FROM crawl_target WHERE id = @target", new { target }); + + return new DateTimeOffset(DateTime.SpecifyKind(at, DateTimeKind.Utc)); + } +} diff --git a/tests/MUI.Web.Tests/CompositionTests.cs b/tests/MUI.Web.Tests/CompositionTests.cs index 7967178..c14f3af 100644 --- a/tests/MUI.Web.Tests/CompositionTests.cs +++ b/tests/MUI.Web.Tests/CompositionTests.cs @@ -6,9 +6,9 @@ using MUI.Catalog; using MUI.Catalog.Persistence; using MUI.Crawler; -using MUI.Web.Accounts; -using MUI.Web.Api; +using MUI.Web; using MUI.Web.Data; +using MUI.Web.Fixtures; namespace MUI.Web.Tests; @@ -123,12 +123,18 @@ public async Task TheOwnerWritePathResolves() /// The same graph in Production, where nothing validates it for us. ///
/// - /// Asserted separately because the two environments fail differently and only one of them fails - /// loudly. Development builds with ValidateOnBuild and refuses to start; Production starts - /// happily and throws the first time the hosted crawler reaches for the cycle, which is inside a - /// BackgroundService — so the site serves pages while the thing that gathers every fact on - /// it is dead. A reviewer reading only the test above could reasonably conclude this was a - /// development-only annoyance. + /// + /// Asserted separately because the two environments behaved differently, and this one is the + /// control rather than the finding: it passed on the parent commit. Production leaves + /// scope validation off, so the container resolved the scoped ClaimService from the root + /// and handed CrawlCycle a working one — a captive dependency, harmless for a stateless + /// service over a pooled data source, and the reason claims verified in production while + /// Development would not start at all. + /// + /// + /// It is kept because it is what stops the fix regressing in the other direction: Production + /// must go on resolving the cycle once the lifetime changes. + /// /// [Test] public async Task TheHostedCrawlerCanBeResolvedInProductionToo() @@ -173,9 +179,84 @@ public async Task TheProductionCrawlLoopDoesGetItsClaimService() } /// - /// Program's service registrations, on Program's host, with a database configured. + /// The demo composition, which is a different graph and not a smaller one. + /// + /// + /// With no connection string there are no accounts, no crawler and no claims — the sign-in and + /// claim surfaces are absent rather than present and broken (§8), and half a claim flow over + /// invented games would be a worse answer than none. The assertion that matters is the last one: + /// the fixture composition must still say on every page that nothing on it was measured. + /// + [Test] + public async Task TheDemoCompositionStandsUpAndAdmitsWhatItIs() + { + await using var site = Site(connectionString: null); + var provider = site.Services; + + await Assert.That(provider.GetService()).IsTypeOf(); + await Assert.That(provider.GetService()).IsNotNull(); + + // Absent, not broken. A page asks the container whether claiming exists at all. + await Assert.That(provider.GetService()).IsNull(); + await Assert.That(provider.GetService()).IsNull(); + await Assert.That(provider.GetService()).IsNull(); + + await Assert.That(provider.GetRequiredService().IsMeasured).IsFalse(); + } + + /// + /// One availability store, however many names it answers to. /// /// + /// The crawler registers it once and exposes it through two interfaces, saying in a comment that + /// two instances would be two connection paths answering one question. The web tier then + /// registered its own with AddSingleton, so the crawler's TryAdd for + /// IAvailabilityStore was skipped and the concrete type and IReachableHistory + /// pointed at a second one. Harmless on a shared pool, and a comment that had stopped being true. + /// + [Test] + public async Task TheAvailabilityStoreIsOneObjectUnderEveryNameItHas() + { + await using var site = Site(); + var provider = site.Services; + + var concrete = provider.GetRequiredService(); + + await Assert.That(provider.GetRequiredService()).IsSameReferenceAs(concrete); + await Assert.That(provider.GetRequiredService()).IsSameReferenceAs(concrete); + } + + /// + /// §8.1's on-demand check can reach the schedule it claims to move. + /// + /// + /// The same shape as the bug this file exists for: ClaimService takes + /// optionally, because it is constructible without a crawl registry + /// and mui-crawl constructs it that way — so nothing but the composition can say whether + /// the deployed site has one. Without it the button records an ask, moves no probe, and tells the + /// operator it dialled their server. + /// + [Test] + public async Task TheOnDemandCheckIsGivenSomethingToBringForward() + { + await using var site = Site(); + + await Assert.That(site.Services.GetService()).IsNotNull(); + await Assert.That(ProbesOf(site.Services.GetRequiredService())).IsNotNull(); + } + + /// + /// Program's graph, built by calling Program's own registration. + /// + /// + /// + /// and not a copy of it. This restated the + /// registrations to begin with, which is a second copy that agrees with the first only until + /// somebody edits one of them — and the edit that breaks it is precisely the one these tests + /// exist to catch: a scoped service consumed by a singleton, or AddMuiAccounts moving. + /// The graph therefore moved out of Program's top-level statements so that the site and + /// this can call the same thing. + /// /// /// A real rather than a bare , /// because half the framework's own registrations want IConfiguration and @@ -183,16 +264,17 @@ public async Task TheProductionCrawlLoopDoesGetItsClaimService() /// anything about ours. What is left after the host supplies them is our graph, and only ours. /// /// - /// Scope validation on, which is what WebApplication.CreateBuilder does by itself in - /// Development — so this is the check a developer already gets on dotnet run and a - /// production deployment does not. + /// Development by default, where + /// switches scope validation on by itself — so this is the check a developer already gets on + /// dotnet run and a production deployment does not. /// /// - /// Nothing is started and no migration runs: Program applies those after - /// , and this stops at the graph. + /// Nothing is started and no migration runs: Program does both after the graph is built, + /// and this stops at the graph. is never connected to — + /// NpgsqlDataSource is lazy. /// /// - private static WebApplication Site(string? environment = null) + private static WebApplication Site(string? environment = null, string? connectionString = ConnectionString) { var builder = WebApplication.CreateBuilder(new WebApplicationOptions { @@ -200,14 +282,7 @@ private static WebApplication Site(string? environment = null) }); builder.Logging.ClearProviders(); - - builder.Services.AddRazorComponents(); - builder.Services.AddMuiApi(builder.Configuration); - builder.Services.AddPostgresCatalogue(ConnectionString); - builder.Services.AddMuiCrawler(ConnectionString, configure => configure.ApplyMigrations = false); - builder.Services.AddSingleton(new CatalogueSource(IsMeasured: true)); - builder.Services.AddMuiAccounts(builder.Configuration); - builder.Services.AddSingleton(TimeProvider.System); + builder.Services.AddMuiSite(builder.Configuration, connectionString); return builder.Build(); } @@ -221,9 +296,22 @@ private static WebApplication Site(string? environment = null) /// default stand. Exposing it publicly to be asserted on would widen the type for the test's /// convenience; a wiring test is allowed to look at the wiring. /// - private static object? ClaimsOf(CrawlCycle cycle) => - typeof(CrawlCycle) + private static object? ClaimsOf(CrawlCycle cycle) => Collaborator(cycle); + + private static object? ProbesOf(ClaimService claims) => Collaborator(claims); + + /// + /// One privately-held collaborator, read off an instance. + /// + /// + /// Reflection, because both of these are private primary-constructor parameters and the property + /// under test is precisely that the container supplied one rather than letting the default stand. + /// Exposing them publicly to be asserted on would widen two types for a test's convenience; a + /// wiring test is allowed to look at the wiring. + /// + private static object? Collaborator(object instance) => + instance.GetType() .GetFields(System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic) - .Single(field => field.FieldType == typeof(ClaimService)) - .GetValue(cycle); + .Single(field => field.FieldType == typeof(T)) + .GetValue(instance); } From 9b514d166a1079475765016ec83ce2733fba0492 Mon Sep 17 00:00:00 2001 From: Harry Cordewener Date: Sat, 15 Aug 2026 11:22:55 -0500 Subject: [PATCH 3/3] Drive the passkey ceremony for real, because nothing ever had MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #30 reported that a JSON POST to a minimal API mapped after UseAntiforgery() is refused with 400 in a slim host, even with .DisableAntiforgery(), which would mean passkey sign-in is broken and §8.2 leaves no other way in. IT DOES NOT REPRODUCE. Measured against the real host — accounts registered, Postgres behind them, migrations applied, ASPNETCORE_ENVIRONMENT=Production: POST /account/passkey/assertion-options -> 200 POST /account/passkey/sign-in (as sent) -> 401 POST /account/passkey/registration-options -> 200 POST /account (Razor route, no token) -> 400 <- control The control is the part that makes the rest mean anything: anti-forgery IS live in that pipeline and refuses an untokened POST one route over. The 401 is the handler answering, not the middleware — the first attempt without the ceremony's cookie got a 500 from SignInManager saying no assertion was underway, which is the handler too. The mechanism: a minimal API is given anti-forgery metadata only when it binds FORM data. These bind JSON or a query string, so they carry none and the middleware passes them through; MapRazorComponents puts metadata on component routes, which is why the control is refused. And .DisableAntiforgery() sets RequiresValidation false — a 400 surviving it was never anti-forgery's. Adding the test anyway. Sign-in is the only door in the building and no suite opened it: the ones that exercise sign-in are the ones that stub it, and the composition tests stop at the graph. These drive the real routes through AddMuiSite/UseMuiSite, with a control that fails if anti-forgery ever stops being live and would catch the reported failure if it ever became real. 841 tests over five suites, Postgres exercised. Co-Authored-By: Claude Opus 5 --- tests/MUI.Web.Tests/PasskeyEndpointTests.cs | 184 ++++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 tests/MUI.Web.Tests/PasskeyEndpointTests.cs diff --git a/tests/MUI.Web.Tests/PasskeyEndpointTests.cs b/tests/MUI.Web.Tests/PasskeyEndpointTests.cs new file mode 100644 index 0000000..72c02e5 --- /dev/null +++ b/tests/MUI.Web.Tests/PasskeyEndpointTests.cs @@ -0,0 +1,184 @@ +using System.Net; +using System.Text; + +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Hosting.Server.Features; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +using MUI.Web; + +namespace MUI.Web.Tests; + +/// +/// The passkey ceremony's four endpoints, driven as the browser drives them. +/// +/// +/// +/// §8.2 makes passkeys the only way in, so if these refuse a request nobody can sign in, +/// nobody can claim anything, and every owner surface on the site is unreachable. Nothing tested +/// them: the suites that exercise sign-in are the ones that stub it, and the composition tests stop +/// at the graph. +/// +/// +/// These drive the real routes through the real pipeline — +/// and , the same two calls the deployable makes — because +/// the property at issue is what the middleware does to a request, and no synthetic endpoint can +/// answer that for the endpoints that actually exist. +/// +/// +/// No database is behind them. The connection string points at a port nothing listens on, which is +/// enough for everything asserted here: assertion-options answers in full without storage, +/// and the two that need it are asserted on what the middleware did rather than on how the handler +/// ended. Against a real Postgres all three answer 200, 200 and 401 — measured, on a host with +/// migrations applied. +/// +/// +public class PasskeyEndpointTests +{ + /// What passkey.js posts: a JSON envelope with the credential as a string. + private const string SignInBody = + """ + {"credential":"{\"id\":\"abc\",\"rawId\":\"abc\",\"type\":\"public-key\",\"response\":{\"clientDataJSON\":\"e30\",\"authenticatorData\":\"e30\",\"signature\":\"e30\",\"userHandle\":null}}","name":null} + """; + + /// + /// The anti-forgery middleware is live on this host, which is what makes the rest meaningful. + /// + /// + /// The control, and it has to come first: "the passkey endpoints are not blocked" says nothing + /// unless something on the same host is. MapRazorComponents puts anti-forgery + /// metadata on every component route, so a POST to one without a token is refused — and that is + /// the same middleware, in the same pipeline, one route over. + /// + [Test] + public async Task AntiforgeryIsLiveAndRefusesAnUntokenedPostToAPage() + { + await using var host = await Host.StartAsync(); + + var response = await host.PostAsync("/account", content: null); + + await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.BadRequest); + } + + /// + /// The ceremony's JSON endpoints are not anti-forgery-gated, and answer. + /// + /// + /// + /// A minimal API is given anti-forgery metadata only when it binds form data — + /// IFormCollection, IFormFile, [FromForm]. These bind JSON or a query + /// string, so they carry none and the middleware passes them through. That is the whole + /// mechanism, and it is why the control above is refused while these are not. + /// + /// + /// Both of these are reachable before any account exists, which is the point: a first-time + /// visitor's very first request to this site is one of them. assertion-options mints a + /// challenge without reading anything, so it is asserted in full; registration-options + /// reads the account's existing credentials and therefore needs the database this host does not + /// have, so it is asserted on the only thing under test here — that it was not refused as forged. + /// + /// + [Test] + public async Task TheOptionsEndpointsAnswerAJsonPostWithNoToken() + { + await using var host = await Host.StartAsync(); + + var assertion = await host.PostAsync("/account/passkey/assertion-options", content: null); + var registration = await host.PostAsync( + "/account/passkey/registration-options?name=probe", content: null); + + await Assert.That(assertion.StatusCode).IsEqualTo(HttpStatusCode.OK); + await Assert.That(await assertion.Content.ReadAsStringAsync()).Contains("challenge"); + + await Assert.That(registration.StatusCode).IsNotEqualTo(HttpStatusCode.BadRequest); + } + + /// + /// The sign-in POST reaches its handler rather than being refused as forged. + /// + /// + /// Asserted as "not 400", deliberately, because what is under test is the middleware and not the + /// outcome of the ceremony: with no database behind it the handler cannot finish, and pinning a + /// specific failure would be pinning the wrong thing. Against a real database this same request + /// answers 401 — measured, on a host with Postgres behind it and migrations applied. + /// + [Test] + public async Task TheSignInPostIsNotRefusedAsForged() + { + await using var host = await Host.StartAsync(); + + var response = await host.PostAsync("/account/passkey/sign-in", SignInBody); + + await Assert.That(response.StatusCode).IsNotEqualTo(HttpStatusCode.BadRequest); + } + + /// The site, composed exactly as the deployable composes it, on a loopback port. + private sealed class Host : IAsyncDisposable + { + /// Never connected to. NpgsqlDataSource is lazy and nothing here dials it. + private const string ConnectionString = "Host=127.0.0.1;Port=1;Database=mui;Username=mui"; + + private readonly WebApplication _app; + private readonly HttpClient _client; + + private Host(WebApplication app, HttpClient client) + { + _app = app; + _client = client; + } + + public static async Task StartAsync() + { + var builder = WebApplication.CreateBuilder(new WebApplicationOptions + { + EnvironmentName = Environments.Production, + }); + + builder.Logging.ClearProviders(); + builder.WebHost.UseUrls("http://127.0.0.1:0"); + builder.Services.AddMuiSite(builder.Configuration, ConnectionString); + + // The crawl loop is not under test and would spend the test dialling a database that is + // not there. It is built never to fault the host, so this is noise rather than risk — + // and removing exactly one descriptor leaves every other registration where it was. + if (builder.Services.FirstOrDefault(d => + d.ImplementationType?.Name == "CrawlerService") is { } crawler) + { + builder.Services.Remove(crawler); + } + + var app = builder.Build(); + app.UseMuiSite(ConnectionString); + + await app.StartAsync(); + + var address = app.Services.GetRequiredService().Features + .Get()!.Addresses.First(); + + return new Host( + app, + new HttpClient(new HttpClientHandler { AllowAutoRedirect = false }) + { + BaseAddress = new Uri(address), + }); + } + + public Task PostAsync(string path, string? content) => + _client.PostAsync( + path, + content is null + ? null + : new StringContent(content, Encoding.UTF8, "application/json")); + + public async ValueTask DisposeAsync() + { + _client.Dispose(); + await _app.StopAsync(); + await _app.DisposeAsync(); + } + } +}