Owning a game: add what MSSP has no field for, and take your screen back - #29
Merged
Merged
Conversation
The dashboard was read-only and GamePage.ConnectScreenSuppressed was a flag every surface read and nothing could ever set. This is the write half of a claim (§8.5, §11), and nothing more than that half. OwnerEnrichment is the one gate. The writable set IS the field registry's OwnerEnrichable flag — derived, never listed a second time — so a write to a player count, a capability or a codebase is refused out loud and takes the rest of its submission with it. A silent no-op teaches an owner the site is broken; a successful one would make the whole site a self-report with extra steps. Every owner value is DECLARED. It goes through the same reconciler a probe's observations do, lands under FieldSource.Owner in a row of its own, carries its age onto the chip, the plain surface and the API, and cannot overwrite, shadow or silence a measurement — asserted by comparing every non-owner row before and after a write. Clearing writes an empty value rather than deleting a row, and the withdrawal reaches the change feed like any other change; a clear of a field nobody ever set writes nothing at all. Suppression is one button, no reason field, no second step. It now takes effect on the API too, which published `suppressed: true` beside the full text — the one surface most likely to be republished by somebody else was the only one ignoring the request. The anti-forgery middleware moves after authentication. A token issued to a signed-in operator carries their identity and is compared against HttpContext .User, so validating first compares a token minted for somebody against nobody and rejects every owner's post as forged. No public surface could notice: they are all GET. Pinned by a test that builds the wrong order and watches it fail. 845 tests over five suites, Postgres exercised for the gate and a loopback server for the form. Testcontainers 4.13.0 -> 4.14.0 is an unrelated pickup: it drags in SSH.NET 2025.1.0, now advised against, and NU1903 was failing restore on main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
This was referenced Aug 15, 2026
Member
Author
|
@coderabbitai review |
|
Review findings on the enrichment write path. All five held. BLOCKER. A cleared owner row could silence a measurement. `owner` outranks `mssp` for enrichment fields, so an emptied owner row won its group and my Value.Length guard then dropped the whole group — taking the mssp row with it. A game publishing its own unofficial FANDOM could have had it removed from the page, the plain surface and the API by its owner typing a space into a box: an owner editing a measurement by the back door, which §8.5 forbids outright. The filter moves before the ladder, where it belongs, because an empty row is an ABSENCE and an absence does not get to win. Clearing now uncovers what was underneath instead of hiding it, which is also what withdrawing a declaration ought to mean. Proved by reverting the one line and watching the test fail. The change feed published a privacy decision. connect_screen_suppressed goes through the reconciler and the feed filtered nothing, so reversing suppression put "connect_screen_suppressed changed from true to false (owner)" on the public game page, in plain text and in the API — the first internal field ever to reach those surfaces, announcing a choice §11 grants with no questions asked. Filtered in SQL rather than after the read, because the query is limited and a filter applied afterwards would spend that limit on rows nobody may see. Nothing is deleted: the rows stay in field_change and are simply not events about the game. A withdrawn field claimed to have been declared. The panel read the row's presence as a declaration, so an emptied box printed "declared 5w ago" beside it, and every subsequent save re-confirmed the empty row and walked its age forward — a field withdrawn last year reading as touched this morning. The panel checks the value, and an empty edit over an already-empty field is skipped entirely. Hiding a connect screen reported the enrichment sentence. Both endpoints redirected with ?saved= and nothing else, so §11's toggle came back as "your page now shows it as owner-declared" — a statement about an action nobody took, on the surface whose whole job is to say what happened. Three outcomes now, and the dashboard says which. The dashboard read every row of every claimed game — connect screens included, 9,376 characters at the longest here — to keep at most four owner rows. IGameFieldStore gained a by-source read; the comment that claimed a single read now says what it does. 857 tests over five suites, Postgres exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Eighth link: main → #27 → #32 → #31 → #21 → #28 → #30 → #26 → #29. THE ANTI-FORGERY REORDER, WHICH WOULD HAVE VANISHED. This branch moves UseAntiforgery after UseAuthentication, and it was written against the Program.cs #32 deleted — so it merged clean and applied to nothing. The rule is reapplied inside the composition, and it is now a named method rather than three lines: UseMuiAntiforgeryAfterAuthentication. A method because the test that proves the rule matters built its own copy of the three lines. OwnerEndpointTests' harness said "Program's order" in a comment and restated it in code, so it asserted its own ordering and would have gone on passing with the site's reversed — which is the failure it exists to catch, one level up. The harness now calls the site's own method for the correct order and hand-builds only the wrong one, because there is no other way to build a thing that is not supposed to exist. Verified: swapping the two lines inside the method fails five of this branch's tests by name, and it failed none of them before. The ordering is worth that trouble. An anti-forgery token issued to a signed-in operator carries their identity; validating before authentication compares it against nobody and every owner's form post is refused as forged, while every public page — all GET — goes on working perfectly. PlainText — #27 factored the plain label into PlainText.Label, this branch added a third word to it at one of the four call sites. The three-way choice moved into Label: an owner's answer is owner-declared on the listing, on the game page and in the archive alike, and "declared" alone would put what an operator typed into our form and what their config file emits under one word. That was this branch's own argument for making the distinction at all; it just has one place to live now. app.css is two separate blocks, both kept. InMemoryFieldStore grew #30's LastChangedAtAsync, answering null — which is true of a store whose RecordChangeAsync already discards what it is handed. DeclaredOf's empty-value filter survived intact, and the ladder conflict its author expected with #28 did not happen: #28 never touched NpgsqlGameQueries. A cleared owner row is still filtered before the ladder rather than after it, so it cannot win its group and silence the MSSP value underneath.
HarryCordewener
pushed a commit
that referenced
this pull request
Aug 15, 2026
Ninth link, and the first of the three that were based on feat/claim-wiring rather than on main: main → #27 → #32 → #31 → #21 → #28 → #30 → #26 → #29 → #33. Two conflicts, and one of them is the reason these three were flagged. Account.razor — this branch put the scorecard link in the <li> a claimed game used to be, and #29 has since turned each claimed game into a <details> with an owner panel inside it. Textually the two edits are the same lines; what the branch meant was "one link per verified claim, on the dashboard and nowhere else", so the link moved into the <summary> beside the game's name and the verified date. Taking either side wholesale would have lost the owner panel or lost the only route to the scorecard. app.css is two independent blocks, both kept. Read the branch's tests rather than trusting them: MsspLintTests and MsspScorecardSurfaceTests assert MsspLint's judgements and that the page is CLOSED to a stranger, neither of which touches the dashboard's markup, so nothing here went stale. They also do not cover the dashboard link itself — noted rather than fixed, because Account.razor has no render harness and building one for one anchor is a bigger change than this merge.
HarryCordewener
pushed a commit
that referenced
this pull request
Aug 15, 2026
Tenth link: main → #27 → #32 → #31 → #21 → #28 → #30 → #26 → #29 → #33 → #34. THE BADGE WAS PUBLISHING A GAME'S OWN ASSERTION AS OUR MEASUREMENT, and this merge is where that became visible. This branch added GameSummary.PlayersNowAt — "when the count was measured" — and said so in its own doc comment: "not the whole of §10.1's fix, the codebase still has no chip". #27 landed the whole fix while this branch sat on feat/claim-wiring, so the summary now carries PlayersNowProvenance, a chip with the count's source on it. PlayersNowAt is dropped and the badge reads the instant off the chip; keeping both would have been two answers to one question, which is the thing #27 exists to stop. But an instant is not the interesting half. PlayersNow can come from MSSP PLAYERS — PresenceChoice ranks WHO, then MSSP, then the connect screen — and this badge writes "N players measured 4m ago", returns state "measured" in its JSON, and paints the accent that means measured on every other surface here. On somebody else's front page, where we cannot correct it. That is rule 5 broken by a format string, which is very nearly the sentence §10.1 uses about the unlabelled listing. So Counted now requires ProvenanceChip.IsMeasured — the same predicate ApiMapper.Counted already uses to decide playersNowState, so the badge and /api/games/{slug} cannot disagree about one game. A declared count reads as unknown: three states, no new vocabulary, and the badge says only what we measured. What the game says about itself is on its page, attributed. Nothing covered this: BadgeApiTests used ashen-court only to assert two URLs, and ashen-court is the fixture's MSSP-declared row — put there by #27 precisely as the argument for labelling. ADeclaredCountIsNotPublishedAsA- MeasuredOne now pins it, with m-u-s-h as the measured control, and it fails if the IsMeasured guard is relaxed. Account.razor for the third time: the badge snippet was written against the <li> a claimed game used to be. It is now in the <details> body beside the owner panel rather than in the <summary>, because a <details> nested inside a <summary> is interactive content inside a control. NEEDS A HUMAN: ProvenanceChip.IsMeasured is Handshake-or-Who, so a count read off a connect screen is "declared" — while migration 0003 says in terms that a banner count "is still a measurement of ours". The two disagree, the disagreement predates this chain, and it now decides whether Aardwolf's badge shows a number. I have kept the badge consistent with the API rather than picking a side.
HarryCordewener
pushed a commit
that referenced
this pull request
Aug 15, 2026
Eleventh and last link of the restack: main → #27 → #32 → #31 → #21 → #28 → #30 → #26 → #29 → #33 → #34 → #35. MIGRATION RENUMBERED. 0012_claim_intent.sql → 0013_claim_intent.sql, since #30's slug history now holds 0012. Thirteen migrations are embedded in MUI.Catalog.dll, 0013 sorts last, and nothing referred to it by number. Three conflicts and one clean merge that did not compile. Account.razor, for the third merge running, and this one had the most in it. The dashboard's status banner is now one else-if chain — resigned, then saved, then refused — because a redirect carries exactly one outcome and two banners at once would be two answers to one action. A claimed game's block holds, in the order an owner would want them: who else owns it, the enrichment panel, the badge snippet, the history, and giving up the claim last. #35 was written when a claimed game was an <li>, so all of it had to move inside the <details> #29 introduced; the co-owner line in particular was inside the <summary>, which is not somewhere a <p> may live. Passkeys.cs maps both write surfaces rather than choosing: MapMuiOwnerWrites is §8.5's enrichment and §11's suppression, MapMuiOwnership is §8.4's counter-claim and resignation. They are different routes. Claim.razor merged clean and broke the build, which is the useful kind of failure. #21 changed this page from IGameQueries to IGameStore on purpose — a submitted game is hidden from the public read until somebody claims it, so looking it up through the listing's own query made claiming the one thing a hidden game could never do — and #35 added three uses of the old Page.Summary against the view model that is no longer loaded. They now read the row, which is what the rest of the page already did. Read #35's tests rather than trusting them, as asked: OwnershipPostgresTests and OwnershipSchemaTests assert ClaimService and the claim_intent schema against a real database, neither of which touches the dashboard markup or the page's lookup, so nothing in them went stale. The dashboard markup itself has no render harness on any of these three branches.
Carries the shared measured/declared predicate and /submit's opt-out refusal down the chain.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The owner dashboard was read-only, and
GamePage.ConnectScreenSuppressedwas a flag every surface read and nothing could ever set. This is the write half of a claim (§8.5, §11) — and deliberately nothing beyond that half.What a claim now grants
Enrichment. The four fields §3.2 names as genuinely absent from MSSP —
FANDOM,APPLICATION PROCESS,RP ENFORCEMENT,CONSENT TOOLS— editable from/account, one collapsed block per claimed game.Connect-screen suppression (§11). One button, no reason field, no second step, no delay. It takes effect on the game page, the plain surface and the API.
The gate has one spelling
OwnerEnrichment(MUI.Catalog) owns both the authorisation and the writable set, and both endpoints go through it.OwnerEnrichableflag, derived rather than listed a second time, so the form an owner sees and the gate the endpoint applies are the same set by construction.FANDOMandCODEBASEapplies neither and comes back namingCODEBASE.Declared, and never over a measurement
Every owner value goes through the same
FieldReconcilera probe's observations do: it lands underFieldSource.Ownerin a row of its own, confirms rather than repeating itself, and reaches the change feed when it moves. The test compares every non-owner row before and after a write and requires it untouched.Nothing is deleted. Clearing writes an empty value: the row survives, the withdrawal is a change-feed event, the panel simply has one fewer line.
Review findings — all five held, all fixed
1. BLOCKER: a cleared owner row could silence a measurement.
owneroutranksmsspfor enrichment fields, so an emptied owner row won its precedence group and theValue.Lengthguard then dropped the whole group — taking themssprow with it. A game publishing its own unofficialFANDOMcould have had it removed from the page, the plain surface and the API by its owner typing a space into a box. An owner editing a measurement by the back door, which §8.5 forbids outright.The filter moves before the ladder, where it belongs: an empty row is an absence, and an absence does not get to win. Clearing now uncovers what was underneath rather than hiding it — which is what withdrawing a declaration ought to mean anyway. Verified by reverting the one line and watching
ClearingAnOwnerValueUncoversTheMeasurementRatherThanHidingItfail (KeyNotFoundException: 'fandom'— the measurement gone entirely).2. The change feed published a privacy decision.
connect_screen_suppressedgoes through the reconciler and the feed filtered nothing, so reversing suppression putconnect_screen_suppressed changed from true to false (owner)on the public game page, in plain text and in the API — the first internal field ever to reach those surfaces, announcing a choice §11 grants with no questions asked.Filtered in SQL rather than after the read, because the query is limited and filtering afterwards would spend that limit on rows nobody may see — a game whose owner toggled twenty times would have shown an empty feed. Nothing is deleted: the rows stay in
field_changeand are simply not events about the game.3. A withdrawn field claimed to have been declared. The panel read the row's presence as a declaration, so an emptied box printed "declared 5w ago" beside it — and every subsequent save re-confirmed the empty row and walked its age forward, so a field withdrawn last year read as touched this morning. The panel checks the value now, and an empty edit over an already-empty field is skipped entirely.
4. Hiding a connect screen reported the enrichment sentence. Both endpoints redirected with
?saved=and nothing else, so §11's toggle came back as "your page now shows it as owner-declared" — a statement about an action nobody took, on the surface whose whole job is to say what happened. Three outcomes now, and the dashboard says which.5. The dashboard read everything to keep four rows. Every field row of every claimed game — connect screens included, 9,376 characters at the longest in this catalogue — to keep at most four owner rows.
IGameFieldStoregained a by-source read; the comment that claimed a single read now says what it does.Two bugs found earlier in review
ConnectScreenViewshippedsuppressed: truebeside the full text. The flag stays, the text does not.HttpContext.User, so validating first rejects every owner's post as forged. No public surface could notice — they are all GET. Pinned by a test that builds the wrong order and watches it fail.Verified
dotnet build MUIndex.slnx -c Releaseclean, each suite run directly — 857 tests, 0 failed:Catalog and Crawler against real PostgreSQL via Testcontainers/Podman.
Notes for review
DeclaredOf, to stay legible against Roll an hour up without letting it lose one of its three states #28's work in the same area.main, so it does not contain Claiming was complete, tested, and wired to nothing #32'sSiteCompositionmove. Both touchPasskeys.cs's registration block; the conflict is adjacent lines.Testcontainers.PostgreSql4.13.0 → 4.14.0 is the same one-line pickup as Package the deployable, and refuse to build one that carries no schema #26.Left undone
WHO-format override and opt-out need crawler changes rather than dashboard ones.🤖 Generated with Claude Code