Feat/submit a game

migrations/0008_bounded_field_value_index.sql

@@ -0,0 +1,37 @@
+-- The (field, value) index could not hold a connect screen, and a game whose screen was long enough
+-- failed to ingest at all.
+--
+-- Observed on the first crawl large enough to find it: three of four hundred games died with
+-- "54000: index row size 3048 exceeds btree version 4 maximum 2704 for index
+-- game_field_field_value_idx". PostgreSQL's btree cannot index a row wider than about 2704 bytes,
+-- and a connect screen is routinely thousands of characters — the longest in this catalogue is 9,376.
+-- The failure is not partial: the INSERT is refused, so the whole probe's ingestion is lost for that
+-- game, and it is lost again on every future probe, for ever. A game with a generous piece of ASCII
+-- art was permanently unlistable.
+--
+-- The index's stated purpose (0002) is §9's faceted search: which games have CODEBASE = PennMUSH, or
+-- capability.gmcp.measured = true. Every value that purpose looks up is short. NOTHING HAS EVER
+-- SEARCHED BY CONNECT SCREEN and nothing ever will — it is a display asset and a fingerprint, and
+-- the fingerprint has its own column. So the index covers a bounded prefix, which serves the lookups
+-- it was built for and cannot overflow: 256 characters is under the limit even at four bytes each.
+--
+-- The stored value is untouched. Truncating what a game said in order to fit our own index would be
+-- exactly the kind of quiet lossiness this schema refuses everywhere else; it is the *index* that is
+-- bounded, not the fact.
+DROP INDEX IF EXISTS game_field_field_value_idx;
+
+CREATE INDEX game_field_field_value_idx ON game_field (field, left(value, 256));
+
+-- The same flaw, one index over: §7.3's identity lookup folds case and whitespace on both columns,
+-- and folding does not shorten a connect screen. This one is partial rather than prefixed, because
+-- its reader asks an equality question and a prefix would silently turn that into a
+-- starts-with — over-matching where the raw index merely refused. Every identity signal §7.3 names
+-- is short: a name, a year, a hostname, a hash, a token. A value longer than this is not one of
+-- them, so excluding it from the lookup changes no correct answer.
+--
+-- CatalogueDirectories carries the same predicate, or the planner cannot use a partial index.
+DROP INDEX IF EXISTS game_field_folded_value_idx;
+
+CREATE INDEX game_field_folded_value_idx
+    ON game_field (lower(btrim(field)), lower(btrim(value)))
+ WHERE length(value) <= 256;

migrations/0010_submitted_games.sql

@@ -0,0 +1,48 @@
+-- spec §8 — a signed-in operator may add their own game, and it stays out of the listing until they
+-- have proved they run it.
+--
+-- ONE NULLABLE COLUMN, AND IT IS NOT A LIFECYCLE STATE. `game.state` is derived from availability
+-- history and never set by hand (0001), so a submission cannot live there: it is a different axis
+-- entirely — how a game reached us, rather than how it has been behaving. Two axes in one column
+-- would mean a submitted game that goes dark has nowhere to say both things.
+--
+-- The listing rule is one sentence and reads off this column alone:
+--
+--     a game is public if nobody submitted it, or if it has been claimed.
+--
+-- Which keeps §7.1's auto-listing exactly as it was — anything the crawler found for itself is
+-- listed immediately as discovered-and-unclaimed — while a stranger's assertion that some address is
+-- a game waits for that stranger to prove they run it.
+--
+-- WHY THE ASYMMETRY IS NOT ARBITRARY. Discovery is something we did: a referral we walked and a
+-- resolved-address gate we applied (§7.2). A submission is somebody else pointing us at a host. If a
+-- submitted game listed on sight, the form would be a way to put any address on a public page under
+-- any description, and the answer to "who says this is a game?" would be "a stranger". Requiring the
+-- claim makes the form mean *add your own game*, not *add somebody's*.
+--
+-- THIS IS NOT THE MODERATION QUEUE §3 CONDEMNS, AND THE DIFFERENCE MUST BE KEPT. The incumbents'
+-- queues waited on a human at their end, which is why listings sat unapproved for a year. This waits
+-- on the submitter, is settled by one probe, and has nobody in the middle. If a screen is ever added
+-- where staff approve submissions, that distinction is gone and so is the argument for the feature.
+ALTER TABLE game
+    ADD COLUMN submitted_by uuid REFERENCES app_user (id);
+
+-- The listing filters on this on every read, and the overwhelming majority of rows are NULL, so it
+-- is worth an index only over the rows that are not.
+CREATE INDEX game_submitted_by_idx ON game (submitted_by) WHERE submitted_by IS NOT NULL;
+
+-- How many submissions one account may make in a day (§8). Recorded as a table rather than a
+-- column so the bound is auditable: a burst of submissions from one account is a thing somebody
+-- will want to look at, and a counter that only counts would not say what happened.
+CREATE TABLE game_submission (
+    id           uuid PRIMARY KEY,
+    user_id      uuid NOT NULL REFERENCES app_user (id),
+    game_id      uuid NOT NULL REFERENCES game (id),
+    host         text NOT NULL,
+    port         integer NOT NULL,
+    submitted_at timestamptz NOT NULL,
+
+    CONSTRAINT game_submission_port_is_a_port CHECK (port >= 1 AND port <= 65535)
+);
+
+CREATE INDEX game_submission_user_idx ON game_submission (user_id, submitted_at DESC);

src/MUI.Catalog/Persistence/NpgsqlGameQueries.cs

@@ -31,6 +31,29 @@ namespace MUI.Catalog.Persistence;
 public sealed class NpgsqlGameQueries(NpgsqlDataSource source, IFieldRegistry? registry = null)
     : IGameQueries
 {
+    /// <summary>
+    /// The rule that keeps an unclaimed submission off every public surface (spec §8, migration 0010).
+    /// </summary>
+    /// <remarks>
+    /// <para>
+    /// <b>A game is public if nobody submitted it, or if it has been claimed.</b> Anything the
+    /// crawler found for itself is listed on sight exactly as §7.1 says; a stranger's assertion that
+    /// some address is a game waits until that stranger proves they run it.
+    /// </para>
+    /// <para>
+    /// It is one constant because it has to hold on <em>every</em> read — the listing, the search,
+    /// the three feeds, the ecosystem shares, the rankings, and both lookups. A predicate written
+    /// out per query is a predicate that will be forgotten on the next query somebody adds, and the
+    /// failure mode is a game on a public page that nobody vouched for. The test that enumerates
+    /// <c>IGameQueries</c> and asserts an unclaimed submission appears in none of it is the real
+    /// guard; this constant is what makes passing it easy.
+    /// </para>
+    /// </remarks>
+    private const string Public = "(submitted_by IS NULL OR is_claimed)";
+
+    /// <summary>The same rule where the table is aliased.</summary>
+    private const string PublicG = "(g.submitted_by IS NULL OR g.is_claimed)";
+
     /// <summary>The heatmap's window (spec §5.2).</summary>
     public static readonly TimeSpan ActivityWindow = TimeSpan.FromDays(56);
 
@@ -119,11 +142,12 @@ public async Task<GameListing> SearchAsync(
         var includeArchived = filter.IncludeArchived || filter.Band is ActivityBand.Archived;
 
         var rows = (await connection.QueryAsync<GameRow>(new CommandDefinition(
-            """
+            $"""
             SELECT g.id AS Id, g.slug AS Slug, g.name AS Name, g.tagline AS Tagline,
                    g.state AS State, g.is_claimed AS IsClaimed, g.last_reachable_at AS LastReachableAt
               FROM game g
              WHERE (@includeArchived OR g.state <> 'archived')
+               AND {PublicG}
              ORDER BY g.name
             """,
             new { includeArchived },
@@ -229,11 +253,11 @@ FROM game_endpoint
         await using var connection = await source.OpenConnectionAsync(cancellationToken);
 
         var row = await connection.QuerySingleOrDefaultAsync<GameRow>(new CommandDefinition(
-            """
+            $"""
             SELECT id AS Id, slug AS Slug, name AS Name, tagline AS Tagline, state AS State,
                    is_claimed AS IsClaimed, last_reachable_at AS LastReachableAt
               FROM game
-             WHERE id = @id
+             WHERE id = @id AND {Public}
             """,
             new { id },
             cancellationToken: cancellationToken));
@@ -268,11 +292,11 @@ FROM game
         await using var connection = await source.OpenConnectionAsync(cancellationToken);
 
         var row = await connection.QuerySingleOrDefaultAsync<GameRow>(new CommandDefinition(
-            """
+            $"""
             SELECT id AS Id, slug AS Slug, name AS Name, tagline AS Tagline, state AS State,
                    is_claimed AS IsClaimed, last_reachable_at AS LastReachableAt
               FROM game
-             WHERE slug = @slug
+             WHERE slug = @slug AND {Public}
             """,
             new { slug },
             cancellationToken: cancellationToken));
@@ -333,10 +357,10 @@ public async Task<LivenessFeeds> FeedsAsync(CancellationToken cancellationToken
         // §9's three liveness feeds — the differentiator no incumbent can publish, because none of
         // them measured continuously enough to know when a game came back.
         var discovered = await connection.QueryAsync<FeedRow>(new CommandDefinition(
-            """
+            $"""
             SELECT slug AS Slug, name AS Name, first_seen_at AS At, NULL AS Cause
               FROM game
-             WHERE first_seen_at >= @since
+             WHERE first_seen_at >= @since AND {Public}
              ORDER BY first_seen_at DESC
              LIMIT @limit
             """,
@@ -411,9 +435,9 @@ public async Task<EcosystemDashboard> EcosystemAsync(CancellationToken cancellat
         await using var connection = await source.OpenConnectionAsync(cancellationToken);
 
         var totals = await connection.QuerySingleAsync<EcosystemTotalsRow>(new CommandDefinition(
-            """
+            $"""
             SELECT
-              (SELECT count(*)::int FROM game WHERE state <> 'archived') AS Listed,
+              (SELECT count(*)::int FROM game WHERE state <> 'archived' AND {Public}) AS Listed,
 
               -- A completed session, which is what a measured capability is a capability of.
               (SELECT count(DISTINCT a.game_id)::int
@@ -507,7 +531,7 @@ public async Task<Rankings> RankingsAsync(CancellationToken cancellationToken =
         await using var connection = await source.OpenConnectionAsync(cancellationToken);
 
         var listed = await connection.ExecuteScalarAsync<int>(new CommandDefinition(
-            "SELECT count(*)::int FROM game WHERE state <> 'archived'",
+            $"SELECT count(*)::int FROM game WHERE state <> 'archived' AND {Public}",
             cancellationToken: cancellationToken));
 
         var busiest = (await connection.QueryAsync<BusiestRow>(new CommandDefinition(

src/MUI.Crawler/Persistence/CatalogueDirectories.cs

@@ -101,6 +101,14 @@ SELECT DISTINCT game_id
               FROM game_field
              WHERE lower(btrim(field)) = lower(btrim(@field))
                AND lower(btrim(value)) = lower(btrim(@value))
+               -- The bound is here as well as on the index, and both are deliberate. PostgreSQL's
+               -- btree cannot hold a row past ~2704 bytes, and a connect screen is thousands of
+               -- characters, so an unbounded index refused the INSERT and cost the game its whole
+               -- ingestion. The index is now partial; repeating its predicate here is what lets the
+               -- planner use it rather than sequentially scanning game_field once per identity
+               -- signal per probe. It changes no answer: every §7.3 signal — a name, a year, a
+               -- hostname, a hash, a token — is short, and a value longer than this is not one.
+               AND length(value) <= 256
             """,
             new { field, value },
             cancellationToken: ct));

tests/MUI.Catalog.Tests/Persistence/OversizedFieldValueTests.cs

@@ -0,0 +1,127 @@
+using Dapper;
+
+using MUI.Catalog.Persistence;
+using MUI.Catalog.Tests.Persistence.Support;
+
+namespace MUI.Catalog.Tests.Persistence;
+
+/// <summary>
+/// A field value too large for an index must not cost a game its listing.
+/// </summary>
+/// <remarks>
+/// Found on the first crawl big enough to find it: three games of four hundred died with
+/// <c>54000: index row size … exceeds btree version 4 maximum 2704</c>, because a connect screen is
+/// routinely thousands of characters and the <c>(field, value)</c> index tried to hold one. The
+/// failure is total rather than partial — the insert is refused, the whole probe's ingestion is lost,
+/// and it is lost again on every future probe. A game with a generous piece of ASCII art was
+/// permanently unlistable, and nothing said so.
+/// </remarks>
+public class OversizedFieldValueTests
+{
+    private static readonly DateTimeOffset Now = Seed.Now;
+
+    /// <summary>The real shape: a connect screen far past the btree limit, stored whole.</summary>
+    [Test]
+    public async Task AConnectScreenTooLargeToIndexIsStillStored()
+    {
+        await using var db = await PostgresFixture.MigratedAsync();
+        var game = await Seed.GameAsync(db);
+        var store = new NpgsqlGameFieldStore(db.DataSource);
+
+        // Longer than the longest observed in the wild (9,376 characters) and several times the
+        // index limit, so this fails against the old index and passes against the bounded one.
+        var screen = string.Join('\n', Enumerable.Repeat(new string('=', 78), 160));
+
+        await store.UpsertAsync(new GameField(
+            game, InternalFields.ConnectScreen, FieldSource.Banner, screen, Now, Now));
+
+        var stored = (await store.ForGameAsync(game))
+            .Single(f => f.Field == InternalFields.ConnectScreen);
+
+        // Stored whole. It is the index that is bounded, never the fact — truncating what a game
+        // sent in order to fit our own index is the kind of quiet lossiness this schema refuses.
+        await Assert.That(stored.Value).IsEqualTo(screen);
+        await Assert.That(stored.Value.Length).IsGreaterThan(2704);
+    }
+
+    /// <summary>
+    /// Two long values that differ only past the indexed prefix are still two distinct rows.
+    /// </summary>
+    /// <remarks>
+    /// The prefix is an index, not a key. If bounding it had collapsed rows that share their first
+    /// 256 characters — which two connect screens from one codebase easily do — the fix would have
+    /// traded a loud failure for a silent one.
+    /// </remarks>
+    [Test]
+    public async Task TwoValuesSharingTheirFirstBytesRemainDistinct()
+    {
+        await using var db = await PostgresFixture.MigratedAsync();
+        var one = await Seed.GameAsync(db, slug: "one", name: "One");
+        var two = await Seed.GameAsync(db, slug: "two", name: "Two");
+        var store = new NpgsqlGameFieldStore(db.DataSource);
+
+        var shared = new string('#', 4000);
+
+        await store.UpsertAsync(new GameField(
+            one, InternalFields.ConnectScreen, FieldSource.Banner, shared + "ONE", Now, Now));
+        await store.UpsertAsync(new GameField(
+            two, InternalFields.ConnectScreen, FieldSource.Banner, shared + "TWO", Now, Now));
+
+        var first = (await store.ForGameAsync(one)).Single(f => f.Field == InternalFields.ConnectScreen);
+        var second = (await store.ForGameAsync(two)).Single(f => f.Field == InternalFields.ConnectScreen);
+
+        await Assert.That(first.Value).EndsWith("ONE");
+        await Assert.That(second.Value).EndsWith("TWO");
+    }
+
+    /// <summary>
+    /// Both indexes over this table are bounded, because both could refuse a connect screen.
+    /// </summary>
+    /// <remarks>
+    /// The first fix caught only one of them and the very next probe failed on the other — so this
+    /// asserts the property over every index on <c>game_field</c> rather than over the one that was
+    /// noticed. An index on a raw or merely case-folded value is the shape of the bug: folding does
+    /// not shorten anything.
+    /// </remarks>
+    [Test]
+    public async Task NoIndexOnThisTableCanRefuseALongValue()
+    {
+        await using var db = await PostgresFixture.MigratedAsync();
+
+        await using var connection = await db.DataSource.OpenConnectionAsync();
+
+        var definitions = (await connection.QueryAsync<string>(
+            "SELECT indexdef FROM pg_indexes WHERE tablename = 'game_field'")).ToList();
+
+        foreach (var definition in definitions.Where(d => d.Contains("value", StringComparison.Ordinal)))
+        {
+            // Either the indexed expression is bounded, or the index only covers rows short enough.
+            var bounded = definition.Contains("256", StringComparison.Ordinal);
+
+            await Assert.That(bounded)
+                .IsTrue()
+                .Because($"an unbounded index over `value` refuses a connect screen: {definition}");
+        }
+    }
+
+    /// <summary>The index still exists and still leads on the field, which is what it is for.</summary>
+    [Test]
+    public async Task TheFacetLookupIsStillIndexed()
+    {
+        await using var db = await PostgresFixture.MigratedAsync();
+
+        await using var connection = await db.DataSource.OpenConnectionAsync();
+
+        var definition = await connection.ExecuteScalarAsync<string>(
+            "SELECT indexdef FROM pg_indexes WHERE indexname = 'game_field_field_value_idx'");
+
+        await Assert.That(definition).IsNotNull();
+        await Assert.That(definition!).Contains("field");
+
+        // Asserted on the bound rather than the spelling: PostgreSQL reports the expression back as
+        // "left"(value, 256), quoted, and a test that matched the source text would break on a
+        // formatting difference while saying nothing about whether the index can overflow.
+        await Assert.That(definition!).Contains("256");
+        await Assert.That(definition!.Contains("(field, value)", StringComparison.Ordinal)).IsFalse();
+    }
+}