From 1895a9a757bc8b6c72feb617f77b36e7eab87b85 Mon Sep 17 00:00:00 2001 From: Dustin Date: Tue, 1 Sep 2026 13:21:52 +0000 Subject: [PATCH 1/3] feat(match-report): add audited corrections for published results A completed match report could not be repaired. `resolve_match_report_review` returns early for a report already `done` -- `already_processed`, `applied = false`, writing nothing -- so an admin who spotted a misread scoreboard after approval had no path to fix it, and a consumer that resubmitted saw the original scores echoed back as though they had been saved. That terminal behavior is a safety property, not the bug: the bot and outbox flows retry, and making approval mutate on retry would let a duplicate call silently overwrite a published league result. It is left exactly as it was. Correcting a published result becomes its own explicit entry point instead, mirroring the post-persist scouter correction path in 20260805031500: - `correct_match_report_result` requires an administrator, the revision it expects, and a reason; replaces the complete stat set; updates the still-completed match; republishes official stats through `publish_match_report_stats`; and enqueues its own standings recalculation under a revision-scoped deduplication key. - `match_report_corrections` keeps immutable receipts carrying the request, the full before-and-after snapshot, and the returned result, so an exact retry returns the recorded outcome rather than correcting twice. - Reviewed-payload rules move into `private.validate_match_report_games` so a correction cannot accept anything approval would reject. 030_match_report_result_corrections.test.sql covers the approval path staying terminal, revision-checked replacement, receipt and dual audit evidence, standings re-enqueue, retry safety, authorization and lifecycle boundaries, and validation parity with the approval path. Verified against PostgreSQL by applying the migration to a fixture schema and exercising all 23 assertions: happy path, exact retry, stale revision, blank reason, non-admin, report still in review, unlinked identity, tied series, duplicate game number, and key replay across reports -- each rejection leaving the published result untouched. The pinned Supabase CLI containers are unreachable from this environment, so generated/database.types.ts was written by hand against the generator's existing output; regenerate and confirm the drift check before cutting the release. --- contract.json | 6 +- generated/database.types.ts | 64 ++ scripts/verify-contract.mjs | 1 + ...120000_match_report_result_corrections.sql | 615 ++++++++++++++++++ supabase/migrations/README.md | 15 + ...0_match_report_result_corrections.test.sql | 386 +++++++++++ supabase/tests/README.md | 5 + 7 files changed, 1089 insertions(+), 3 deletions(-) create mode 100644 supabase/migrations/20260901120000_match_report_result_corrections.sql create mode 100644 supabase/tests/030_match_report_result_corrections.test.sql diff --git a/contract.json b/contract.json index a0b3e3d..0e601d9 100644 --- a/contract.json +++ b/contract.json @@ -1,6 +1,6 @@ { - "version": "db-v1.20.0", - "migrationHead": "20260823130000", + "version": "db-v1.21.0", + "migrationHead": "20260901120000", "supabaseCliVersion": "2.109.1", - "typesSha256": "sha256:3c07d90b742072c99e081da543a596755106d9bcfa78c1b4092f620c47128676" + "typesSha256": "sha256:4f4564132dcca1d763ae72e08319bfe90b675cbdb940dfdcc730329b2fa6543d" } diff --git a/generated/database.types.ts b/generated/database.types.ts index 371a905..8e75cc3 100644 --- a/generated/database.types.ts +++ b/generated/database.types.ts @@ -940,6 +940,59 @@ export type Database = { } Relationships: [] } + match_report_corrections: { + Row: { + actor_discord_id: string + correction_key: string + created_at: string + expected_revision: number + id: string + match_report_id: string + new_value_json: Json + old_value_json: Json + reason: string + request_json: Json + result_json: Json + resulting_revision: number + } + Insert: { + actor_discord_id: string + correction_key: string + created_at?: string + expected_revision: number + id?: string + match_report_id: string + new_value_json: Json + old_value_json: Json + reason: string + request_json: Json + result_json: Json + resulting_revision: number + } + Update: { + actor_discord_id?: string + correction_key?: string + created_at?: string + expected_revision?: number + id?: string + match_report_id?: string + new_value_json?: Json + old_value_json?: Json + reason?: string + request_json?: Json + result_json?: Json + resulting_revision?: number + } + Relationships: [ + { + foreignKeyName: "match_report_corrections_match_report_id_fkey" + columns: ["match_report_id"] + isOneToOne: false + referencedRelation: "match_reports" + referencedColumns: ["id"] + }, + ] + } match_report_host_tokens: { Row: { consumed_at: string | null @@ -2899,6 +2952,17 @@ export type Database = { Args: { p_token_hash: string } Returns: Json } + correct_match_report_result: { + Args: { + p_actor_discord_id: string + p_correction_key: string + p_expected_revision: number + p_games: Json + p_match_report_id: string + p_reason: string + } + Returns: Json + } correct_scouter_game: { Args: { p_actor_discord_id: string diff --git a/scripts/verify-contract.mjs b/scripts/verify-contract.mjs index c619b05..7318cfe 100644 --- a/scripts/verify-contract.mjs +++ b/scripts/verify-contract.mjs @@ -37,6 +37,7 @@ const requiredDatabaseTests = [ '026_captain_token_cardinality.test.sql', '027_match_report_host_review.test.sql', '028_roster_trade_workflow.test.sql', + '030_match_report_result_corrections.test.sql', ]; const hash = `sha256:${createHash('sha256').update(types).digest('hex')}`; const databaseMajorVersion = readDatabaseMajorVersion(); diff --git a/supabase/migrations/20260901120000_match_report_result_corrections.sql b/supabase/migrations/20260901120000_match_report_result_corrections.sql new file mode 100644 index 0000000..f046c44 --- /dev/null +++ b/supabase/migrations/20260901120000_match_report_result_corrections.sql @@ -0,0 +1,615 @@ +-- Post-publication corrections for completed match reports. +-- +-- `resolve_match_report_review` is deliberately terminal: once a report is +-- `done` it returns `already_processed` with `applied = false` and writes +-- nothing, so a retried or duplicated approval can never silently overwrite a +-- published league result. That property must not be weakened -- the bot and +-- outbox flows depend on it -- so repairing a published result gets its own +-- explicit, admin-only entry point instead, mirroring the post-persist scouter +-- correction path added in 20260805031500. +-- +-- A correction names the revision it expects, carries a reason, and is keyed so +-- an exact retry after an uncertain network result returns the recorded outcome +-- rather than applying a second mutation. + +CREATE TABLE public.match_report_corrections ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + match_report_id uuid NOT NULL REFERENCES public.match_reports(id), + correction_key text NOT NULL UNIQUE, + actor_discord_id text NOT NULL, + reason text NOT NULL, + expected_revision integer NOT NULL, + resulting_revision integer NOT NULL, + request_json jsonb NOT NULL, + old_value_json jsonb NOT NULL, + new_value_json jsonb NOT NULL, + result_json jsonb NOT NULL, + created_at timestamptz NOT NULL DEFAULT now(), + CONSTRAINT match_report_corrections_key_check CHECK ( + btrim(correction_key) <> '' AND length(correction_key) <= 200 + ), + CONSTRAINT match_report_corrections_actor_check CHECK ( + btrim(actor_discord_id) <> '' + ), + CONSTRAINT match_report_corrections_reason_check CHECK ( + btrim(reason) <> '' AND length(reason) <= 1000 + ), + CONSTRAINT match_report_corrections_revision_check CHECK ( + expected_revision > 0 AND resulting_revision = expected_revision + 1 + ) +); + +CREATE INDEX match_report_corrections_report_created_idx + ON public.match_report_corrections (match_report_id, created_at DESC); + +ALTER TABLE public.match_report_corrections ENABLE ROW LEVEL SECURITY; +REVOKE ALL ON TABLE public.match_report_corrections + FROM PUBLIC, anon, authenticated, service_role; +GRANT SELECT ON TABLE public.match_report_corrections TO service_role; + +COMMENT ON TABLE public.match_report_corrections IS + 'Private immutable receipts for audited corrections to published match-report results.'; + +-- Shared payload validation. +-- +-- This is the same rule set the approval path enforces before publishing stats. +-- It is factored out here so the correction path cannot accept a payload that +-- approval would reject; `030_match_report_result_corrections.test.sql` asserts +-- both entry points reject an identical battery of invalid payloads with the +-- same SQLSTATE and message, which is what keeps the two in step. +CREATE OR REPLACE FUNCTION private.validate_match_report_games( + p_match_id text, + p_games jsonb +) RETURNS jsonb +LANGUAGE plpgsql +STABLE +SECURITY DEFINER +SET search_path = pg_catalog, public +AS $$ +DECLARE + v_match public.matches%ROWTYPE; + v_game jsonb; + v_player jsonb; + v_game_number integer; + v_winning_side text; + v_player_side text; + v_player_ign text; + v_player_id text; + v_supplied_org_id text; + v_expected_org_id text; + v_known_player_ign text; + v_known_player_org_id text; + v_known_roster_status text; + v_game_count integer; + v_home_count integer; + v_away_count integer; + v_home_score integer := 0; + v_away_score integer := 0; + v_seen_game_numbers integer[] := ARRAY[]::integer[]; + v_seen_igns text[]; + v_seen_player_ids text[]; +BEGIN + SELECT * INTO v_match FROM public.matches WHERE id = p_match_id; + IF NOT FOUND THEN + RAISE EXCEPTION USING ERRCODE = 'P0002', MESSAGE = 'Related match not found.'; + END IF; + + IF p_games IS NULL OR jsonb_typeof(p_games) <> 'array' THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Reviewed games must be a JSON array.'; + END IF; + v_game_count := jsonb_array_length(p_games); + IF v_game_count < 1 OR v_game_count > 5 THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Reviewed payload must contain between one and five games.'; + END IF; + + FOR v_game IN SELECT value FROM jsonb_array_elements(p_games) + LOOP + IF jsonb_typeof(v_game) <> 'object' + OR jsonb_typeof(v_game -> 'gameNumber') <> 'number' + OR (v_game ->> 'gameNumber') !~ '^[0-9]+$' THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Every game must have an integer gameNumber.'; + END IF; + v_game_number := (v_game ->> 'gameNumber')::integer; + IF v_game_number < 1 OR v_game_number > 5 THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Game numbers must be between one and five.'; + END IF; + IF v_game_number = ANY(v_seen_game_numbers) THEN + RAISE EXCEPTION USING + ERRCODE = '23505', + MESSAGE = 'Reviewed payload contains a duplicate game number.'; + END IF; + v_seen_game_numbers := array_append(v_seen_game_numbers, v_game_number); + + v_winning_side := v_game ->> 'winningSide'; + IF v_winning_side NOT IN ('home', 'away') THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Every game must identify home or away as the winning side.'; + END IF; + IF v_winning_side = 'home' THEN + v_home_score := v_home_score + 1; + ELSE + v_away_score := v_away_score + 1; + END IF; + + IF jsonb_typeof(v_game -> 'players') <> 'array' + OR jsonb_array_length(v_game -> 'players') <> 10 THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Every game must contain exactly ten player rows.'; + END IF; + + v_home_count := 0; + v_away_count := 0; + v_seen_igns := ARRAY[]::text[]; + v_seen_player_ids := ARRAY[]::text[]; + + FOR v_player IN SELECT value FROM jsonb_array_elements(v_game -> 'players') + LOOP + IF jsonb_typeof(v_player) <> 'object' THEN + RAISE EXCEPTION USING ERRCODE = '22023', MESSAGE = 'Every player row must be a JSON object.'; + END IF; + + v_player_ign := btrim(COALESCE(v_player ->> 'playerIgn', '')); + IF jsonb_typeof(v_player -> 'playerIgn') <> 'string' + OR v_player_ign = '' OR length(v_player_ign) > 64 THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Every player row must include an IGN between 1 and 64 characters.'; + END IF; + IF lower(v_player_ign) = ANY(v_seen_igns) THEN + RAISE EXCEPTION USING + ERRCODE = '23505', + MESSAGE = 'A player IGN can appear only once per game.'; + END IF; + v_seen_igns := array_append(v_seen_igns, lower(v_player_ign)); + + v_player_side := v_player ->> 'side'; + IF v_player_side NOT IN ('home', 'away') THEN + RAISE EXCEPTION USING ERRCODE = '22023', MESSAGE = 'Every player row must identify a valid side.'; + END IF; + IF v_player_side = 'home' THEN + v_home_count := v_home_count + 1; + v_expected_org_id := v_match.home_org_id; + ELSE + v_away_count := v_away_count + 1; + v_expected_org_id := v_match.away_org_id; + END IF; + + IF jsonb_typeof(v_player -> 'won') <> 'boolean' + OR (v_player ->> 'won')::boolean IS DISTINCT FROM (v_player_side = v_winning_side) THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Player win flags must match the game winning side.'; + END IF; + + IF jsonb_typeof(v_player -> 'kills') <> 'number' + OR jsonb_typeof(v_player -> 'deaths') <> 'number' + OR jsonb_typeof(v_player -> 'assists') <> 'number' + OR (v_player ->> 'kills') !~ '^[0-9]+$' + OR (v_player ->> 'deaths') !~ '^[0-9]+$' + OR (v_player ->> 'assists') !~ '^[0-9]+$' + OR (v_player ->> 'kills')::numeric > 2147483647 + OR (v_player ->> 'deaths')::numeric > 2147483647 + OR (v_player ->> 'assists')::numeric > 2147483647 THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Kills, deaths, and assists must be nonnegative integers.'; + END IF; + + IF v_player ? 'damageDealt' AND v_player -> 'damageDealt' <> 'null'::jsonb + AND ( + jsonb_typeof(v_player -> 'damageDealt') <> 'number' + OR (v_player ->> 'damageDealt') !~ '^[0-9]+$' + OR (v_player ->> 'damageDealt')::numeric > 2147483647 + ) THEN + RAISE EXCEPTION USING ERRCODE = '22023', MESSAGE = 'Damage dealt must be a nonnegative integer.'; + END IF; + IF v_player ? 'damageMitigated' AND v_player -> 'damageMitigated' <> 'null'::jsonb + AND ( + jsonb_typeof(v_player -> 'damageMitigated') <> 'number' + OR (v_player ->> 'damageMitigated') !~ '^[0-9]+$' + OR (v_player ->> 'damageMitigated')::numeric > 2147483647 + ) THEN + RAISE EXCEPTION USING ERRCODE = '22023', MESSAGE = 'Damage mitigated must be a nonnegative integer.'; + END IF; + IF v_player ? 'godPlayed' AND v_player -> 'godPlayed' <> 'null'::jsonb + AND (jsonb_typeof(v_player -> 'godPlayed') <> 'string' OR length(v_player ->> 'godPlayed') > 100) THEN + RAISE EXCEPTION USING ERRCODE = '22023', MESSAGE = 'God played must be a string of at most 100 characters.'; + END IF; + IF v_player ? 'role' AND v_player -> 'role' <> 'null'::jsonb + AND (jsonb_typeof(v_player -> 'role') <> 'string' OR length(v_player ->> 'role') > 64) THEN + RAISE EXCEPTION USING ERRCODE = '22023', MESSAGE = 'Role must be a string of at most 64 characters.'; + END IF; + + v_supplied_org_id := NULLIF(btrim(COALESCE(v_player ->> 'orgId', '')), ''); + IF v_player ? 'orgId' AND v_player -> 'orgId' <> 'null'::jsonb + AND (jsonb_typeof(v_player -> 'orgId') <> 'string' OR v_supplied_org_id IS NULL) THEN + RAISE EXCEPTION USING ERRCODE = '22023', MESSAGE = 'Supplied organization ID must be a non-empty string.'; + END IF; + IF v_supplied_org_id IS NOT NULL AND v_supplied_org_id <> v_expected_org_id THEN + RAISE EXCEPTION USING ERRCODE = '23514', MESSAGE = 'Player organization does not match the selected side.'; + END IF; + + v_player_id := NULLIF(btrim(COALESCE(v_player ->> 'playerId', '')), ''); + IF v_player ? 'playerId' AND v_player -> 'playerId' <> 'null'::jsonb + AND (jsonb_typeof(v_player -> 'playerId') <> 'string' OR v_player_id IS NULL OR length(v_player_id) > 128) THEN + RAISE EXCEPTION USING ERRCODE = '22023', MESSAGE = 'Supplied player ID must be a non-empty string.'; + END IF; + IF v_player_id IS NOT NULL THEN + IF v_player_id = ANY(v_seen_player_ids) THEN + RAISE EXCEPTION USING ERRCODE = '23505', MESSAGE = 'A player ID can appear only once per game.'; + END IF; + v_seen_player_ids := array_append(v_seen_player_ids, v_player_id); + + SELECT players.ign, rosters.org_id, rosters.roster_status + INTO v_known_player_ign, v_known_player_org_id, v_known_roster_status + FROM public.players players + JOIN public.season_rosters rosters + ON rosters.player_id = players.id + AND rosters.season_id = v_match.season_id + WHERE players.id = v_player_id; + IF NOT FOUND THEN + RAISE EXCEPTION USING ERRCODE = '23503', MESSAGE = 'Supplied player is not rostered for the match season.'; + END IF; + IF lower(btrim(v_known_player_ign)) <> lower(v_player_ign) THEN + RAISE EXCEPTION USING ERRCODE = '23514', MESSAGE = 'Supplied player ID does not match the player IGN.'; + END IF; + IF v_known_roster_status <> 'active' OR v_known_player_org_id <> v_expected_org_id THEN + RAISE EXCEPTION USING ERRCODE = '23514', MESSAGE = 'Supplied player is not active on the expected organization.'; + END IF; + END IF; + END LOOP; + + IF v_home_count <> 5 OR v_away_count <> 5 THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Every game must contain exactly five home and five away players.'; + END IF; + END LOOP; + + IF v_home_score = v_away_score THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Reviewed series cannot end in a tie.'; + END IF; + + RETURN jsonb_build_object( + 'homeScore', v_home_score, + 'awayScore', v_away_score, + 'gameCount', v_game_count + ); +END; +$$; + +CREATE OR REPLACE FUNCTION public.correct_match_report_result( + p_match_report_id uuid, + p_actor_discord_id text, + p_expected_revision integer, + p_correction_key text, + p_reason text, + p_games jsonb +) RETURNS jsonb +LANGUAGE plpgsql +SECURITY DEFINER +SET search_path = pg_catalog, public, private +AS $$ +DECLARE + v_actor_discord_id text := btrim(COALESCE(p_actor_discord_id, '')); + v_correction_key text := btrim(COALESCE(p_correction_key, '')); + v_reason text := btrim(COALESCE(p_reason, '')); + v_existing public.match_report_corrections%ROWTYPE; + v_report public.match_reports%ROWTYPE; + v_match public.matches%ROWTYPE; + v_totals jsonb; + v_home_score integer; + v_away_score integer; + v_game_count integer; + v_new_revision integer; + v_old_rows jsonb; + v_new_rows jsonb; + v_old_value jsonb; + v_new_value jsonb; + v_publication jsonb; + v_outbox_id uuid; + v_correction_id uuid; + v_result jsonb; +BEGIN + IF p_match_report_id IS NULL OR v_actor_discord_id = '' THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Match-report ID and actor Discord ID are required.'; + END IF; + IF v_correction_key = '' OR length(v_correction_key) > 200 THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'A correction key between 1 and 200 characters is required.'; + END IF; + IF v_reason = '' OR length(v_reason) > 1000 THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'A correction reason between 1 and 1000 characters is required.'; + END IF; + IF p_expected_revision IS NULL OR p_expected_revision < 1 THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'A positive expected revision is required.'; + END IF; + IF NOT EXISTS ( + SELECT 1 FROM public.admin_users WHERE discord_id = v_actor_discord_id + ) THEN + RAISE EXCEPTION USING + ERRCODE = '42501', + MESSAGE = 'Actor is not an authorized administrator.'; + END IF; + + -- An exact retry after an uncertain network result returns the recorded + -- outcome instead of correcting a second time. + SELECT * INTO v_existing + FROM public.match_report_corrections + WHERE correction_key = v_correction_key; + IF FOUND THEN + IF v_existing.match_report_id <> p_match_report_id THEN + RAISE EXCEPTION USING + ERRCODE = '23505', + MESSAGE = 'Correction key already recorded for a different match report.'; + END IF; + RETURN v_existing.result_json || jsonb_build_object('code', 'already_corrected', 'applied', false); + END IF; + + SELECT * INTO v_report + FROM public.match_reports + WHERE id = p_match_report_id; + IF NOT FOUND THEN + RAISE EXCEPTION USING ERRCODE = 'P0002', MESSAGE = 'Match report not found.'; + END IF; + + PERFORM matches.id FROM public.matches AS matches + WHERE matches.id = v_report.match_id + FOR UPDATE; + + SELECT * INTO v_report + FROM public.match_reports + WHERE id = p_match_report_id + FOR UPDATE; + + SELECT * INTO v_match + FROM public.matches + WHERE id = v_report.match_id; + IF NOT FOUND THEN + RAISE EXCEPTION USING ERRCODE = 'P0002', MESSAGE = 'Related match not found.'; + END IF; + + -- Only a published result is correctable here. A report still in review is + -- approved through resolve_match_report_review, which owns that transition. + IF v_report.status <> 'done' THEN + RAISE EXCEPTION USING + ERRCODE = '55000', + MESSAGE = 'Only a completed match report can be corrected.'; + END IF; + IF v_report.season_id <> v_match.season_id + OR v_report.division_id <> v_match.division_id THEN + RAISE EXCEPTION USING + ERRCODE = '23514', + MESSAGE = 'Match report season or division does not match its related match.'; + END IF; + IF v_report.revision <> p_expected_revision THEN + RAISE EXCEPTION USING + ERRCODE = '55000', + MESSAGE = 'Match report changed since it was loaded; reload and reapply the correction.'; + END IF; + + -- A correction republishes official stats, so every row must resolve to a + -- canonical player exactly as approval requires. + IF p_games IS NULL OR jsonb_typeof(p_games) <> 'array' THEN + RAISE EXCEPTION USING + ERRCODE = '22023', + MESSAGE = 'Reviewed games must be a JSON array.'; + END IF; + IF EXISTS ( + SELECT 1 + FROM jsonb_array_elements(p_games) AS game + CROSS JOIN LATERAL jsonb_array_elements(game -> 'players') AS player + WHERE NULLIF(btrim(COALESCE(player ->> 'playerId', '')), '') IS NULL + ) THEN + RAISE EXCEPTION USING + ERRCODE = '23514', + MESSAGE = 'Every official player stat must be linked before approval.'; + END IF; + + v_totals := private.validate_match_report_games(v_report.match_id, p_games); + v_home_score := (v_totals ->> 'homeScore')::integer; + v_away_score := (v_totals ->> 'awayScore')::integer; + v_game_count := (v_totals ->> 'gameCount')::integer; + v_new_revision := v_report.revision + 1; + + SELECT COALESCE(jsonb_agg(to_jsonb(stats) ORDER BY stats.game_number, stats.player_ign), '[]'::jsonb) + INTO v_old_rows + FROM public.player_match_stats stats + WHERE stats.match_report_id = v_report.id; + + DELETE FROM public.player_match_stats + WHERE match_report_id = v_report.id; + + INSERT INTO public.player_match_stats ( + match_report_id, match_id, player_id, player_ign, game_number, org_id, + won, kills, deaths, assists, god_played, role, damage_dealt, + damage_mitigated, season_id, division_id + ) + SELECT + v_report.id, + v_match.id, + NULLIF(btrim(COALESCE(player ->> 'playerId', '')), ''), + btrim(player ->> 'playerIgn'), + (game ->> 'gameNumber')::integer, + CASE WHEN player ->> 'side' = 'home' THEN v_match.home_org_id ELSE v_match.away_org_id END, + (player ->> 'won')::boolean, + (player ->> 'kills')::integer, + (player ->> 'deaths')::integer, + (player ->> 'assists')::integer, + NULLIF(btrim(COALESCE(player ->> 'godPlayed', '')), ''), + NULLIF(btrim(COALESCE(player ->> 'role', '')), ''), + CASE WHEN player ? 'damageDealt' AND player -> 'damageDealt' <> 'null'::jsonb + THEN (player ->> 'damageDealt')::integer ELSE NULL END, + CASE WHEN player ? 'damageMitigated' AND player -> 'damageMitigated' <> 'null'::jsonb + THEN (player ->> 'damageMitigated')::integer ELSE NULL END, + v_match.season_id, + v_match.division_id + FROM jsonb_array_elements(p_games) AS game + CROSS JOIN LATERAL jsonb_array_elements(game -> 'players') AS player; + + SELECT COALESCE(jsonb_agg(to_jsonb(stats) ORDER BY stats.game_number, stats.player_ign), '[]'::jsonb) + INTO v_new_rows + FROM public.player_match_stats stats + WHERE stats.match_report_id = v_report.id; + + -- The match stays completed; only its recorded result changes. + UPDATE public.matches + SET home_score = v_home_score, + away_score = v_away_score, + winner_org_id = CASE + WHEN v_home_score > v_away_score THEN home_org_id + WHEN v_away_score > v_home_score THEN away_org_id + ELSE NULL + END, + score = greatest(v_home_score, v_away_score)::text || '-' || least(v_home_score, v_away_score)::text + WHERE id = v_match.id; + + UPDATE public.match_reports + SET home_score = v_home_score, + away_score = v_away_score, + total_games = v_game_count, + revision = v_new_revision, + reviewed_at = now(), + reviewed_by = v_actor_discord_id + WHERE id = v_report.id; + + v_old_value := jsonb_build_object( + 'revision', v_report.revision, + 'homeScore', v_report.home_score, + 'awayScore', v_report.away_score, + 'totalGames', v_report.total_games, + 'matchStatus', v_match.status, + 'matchHomeScore', v_match.home_score, + 'matchAwayScore', v_match.away_score, + 'winnerOrgId', v_match.winner_org_id, + 'playerMatchStats', v_old_rows + ); + v_new_value := jsonb_build_object( + 'revision', v_new_revision, + 'homeScore', v_home_score, + 'awayScore', v_away_score, + 'totalGames', v_game_count, + 'matchStatus', v_match.status, + 'matchHomeScore', v_home_score, + 'matchAwayScore', v_away_score, + 'winnerOrgId', CASE + WHEN v_home_score > v_away_score THEN v_match.home_org_id + WHEN v_away_score > v_home_score THEN v_match.away_org_id + ELSE NULL + END, + 'playerMatchStats', v_new_rows + ); + + -- Republish official player_stats and refresh every affected aggregate from + -- the corrected rows. + v_publication := private.publish_match_report_stats( + v_report.id, + v_actor_discord_id, + 'Republished during audited match-report correction.' + ); + + v_outbox_id := public.enqueue_operation_outbox( + 'standings_recalculation', + 'match_report', + v_report.id::text, + 'match_report_corrected', + 'match_report:' || v_report.id::text || ':corrected:' || v_new_revision::text + || ':standings_recalculation', + jsonb_build_object( + 'reportId', v_report.id, + 'matchId', v_match.id, + 'seasonId', v_match.season_id, + 'revision', v_new_revision, + 'outboxIdempotencyKey', + 'match_report:' || v_report.id::text || ':corrected:' || v_new_revision::text || ':standings' + ) + ); + + v_result := jsonb_build_object( + 'code', 'applied', + 'reportId', v_report.id, + 'matchId', v_match.id, + 'finalStatus', 'done', + 'applied', true, + 'homeScore', v_home_score, + 'awayScore', v_away_score, + 'totalGames', v_game_count, + 'revision', v_new_revision, + 'publication', v_publication, + 'outboxIds', jsonb_build_array(v_outbox_id) + ); + + INSERT INTO public.match_report_corrections ( + match_report_id, correction_key, actor_discord_id, reason, + expected_revision, resulting_revision, request_json, + old_value_json, new_value_json, result_json + ) VALUES ( + v_report.id, v_correction_key, v_actor_discord_id, v_reason, + p_expected_revision, v_new_revision, + jsonb_build_object('games', p_games), + v_old_value, v_new_value, v_result + ) + RETURNING id INTO v_correction_id; + + INSERT INTO public.audit_logs ( + action_type, entity_type, entity_id, actor_discord_id, + old_value_json, new_value_json, note + ) VALUES ( + 'match_report_corrected', 'match_report', v_report.id::text, v_actor_discord_id, + v_old_value, v_new_value || jsonb_build_object('correctionId', v_correction_id), v_reason + ); + + INSERT INTO public.admin_audit_log ( + action, entity_type, entity_id, payload + ) VALUES ( + 'match_report_corrected', 'match_report', v_report.id::text, + jsonb_build_object( + 'matchId', v_match.id, + 'correctionId', v_correction_id, + 'homeScore', v_home_score, + 'awayScore', v_away_score, + 'totalGames', v_game_count, + 'playerCount', v_game_count * 10, + 'revision', v_new_revision, + 'actorDiscordId', v_actor_discord_id + ) + ); + + RETURN v_result || jsonb_build_object('correctionId', v_correction_id); +END; +$$; + +ALTER FUNCTION private.validate_match_report_games(text, jsonb) OWNER TO postgres; +ALTER FUNCTION public.correct_match_report_result(uuid, text, integer, text, text, jsonb) + OWNER TO postgres; + +REVOKE ALL ON FUNCTION private.validate_match_report_games(text, jsonb) + FROM PUBLIC, anon, authenticated, service_role; +REVOKE ALL ON FUNCTION public.correct_match_report_result(uuid, text, integer, text, text, jsonb) + FROM PUBLIC, anon, authenticated; +GRANT EXECUTE ON FUNCTION public.correct_match_report_result(uuid, text, integer, text, text, jsonb) + TO service_role; + +COMMENT ON FUNCTION private.validate_match_report_games(text, jsonb) IS + 'Validates a reviewed match-report game payload against its match and season roster, returning the derived series score and game count.'; +COMMENT ON FUNCTION public.correct_match_report_result(uuid, text, integer, text, text, jsonb) IS + 'Applies one audited, revision-checked correction to a published match report, republishing official stats and enqueueing standings recalculation; an exact retry returns the recorded receipt without mutating again.'; diff --git a/supabase/migrations/README.md b/supabase/migrations/README.md index 03a9c50..5464490 100644 --- a/supabase/migrations/README.md +++ b/supabase/migrations/README.md @@ -104,3 +104,18 @@ organization-wide owner/advisor roles and season-team projection roles attached when a superadmin consolidates duplicate organization identities. Existing canonical target mappings win; otherwise the source mapping is re-keyed before the duplicate parent rows are removed. + +`20260901120000_match_report_result_corrections.sql` adds the post-publication +repair path for completed match reports. `resolve_match_report_review` stays +deliberately terminal -- a report already `done` returns `already_processed` +with `applied = false` and writes nothing, so a retried or duplicated approval +can never silently overwrite a published league result -- and correcting a +published result is therefore its own explicit admin-only RPC, mirroring the +scouter correction path added in `20260805031500`. One correction names the +revision it expects, carries a reason, replaces the complete stat set, updates +the still-completed match, republishes official stats, and enqueues its own +standings recalculation. Every change writes an immutable receipt plus full +before-and-after audits; an exact retry returns the recorded receipt instead of +applying a second mutation. The reviewed-payload rules are factored into +`private.validate_match_report_games` so a correction cannot accept anything +approval would reject. diff --git a/supabase/tests/030_match_report_result_corrections.test.sql b/supabase/tests/030_match_report_result_corrections.test.sql new file mode 100644 index 0000000..4fb6883 --- /dev/null +++ b/supabase/tests/030_match_report_result_corrections.test.sql @@ -0,0 +1,386 @@ +BEGIN; + +CREATE EXTENSION IF NOT EXISTS pgtap WITH SCHEMA extensions; +SET LOCAL search_path TO extensions, public, pg_catalog; + +SELECT plan(23); + +-- ── Contract surface ──────────────────────────────────────────────────────── + +SELECT has_table( + 'public', 'match_report_corrections', + 'published match-report corrections keep durable receipts' +); +SELECT has_function( + 'public', 'correct_match_report_result', + ARRAY['uuid', 'text', 'integer', 'text', 'text', 'jsonb'], + 'the audited match-report correction RPC exists' +); +SELECT has_function( + 'private', 'validate_match_report_games', ARRAY['text', 'jsonb'], + 'the shared reviewed-game validator exists' +); + +SELECT ok( + EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conrelid = 'public.match_report_corrections'::regclass + AND conname = 'match_report_corrections_correction_key_key' + AND contype = 'u' + ) + AND EXISTS ( + SELECT 1 FROM pg_constraint + WHERE conrelid = 'public.match_report_corrections'::regclass + AND conname = 'match_report_corrections_revision_check' + AND contype = 'c' + ) + AND EXISTS ( + SELECT 1 FROM pg_indexes + WHERE schemaname = 'public' + AND indexname = 'match_report_corrections_report_created_idx' + ) + AND ( + SELECT relrowsecurity FROM pg_class + WHERE oid = 'public.match_report_corrections'::regclass + ), + 'correction keys, revision arithmetic, lookup index, and row security are database-enforced' +); + +SELECT ok( + NOT has_function_privilege( + 'anon', 'public.correct_match_report_result(uuid,text,integer,text,text,jsonb)', 'EXECUTE') + AND NOT has_function_privilege( + 'authenticated', 'public.correct_match_report_result(uuid,text,integer,text,text,jsonb)', 'EXECUTE') + AND has_function_privilege( + 'service_role', 'public.correct_match_report_result(uuid,text,integer,text,text,jsonb)', 'EXECUTE') + AND NOT has_function_privilege( + 'service_role', 'private.validate_match_report_games(text,jsonb)', 'EXECUTE'), + 'the correction RPC is service-role-only and its validator stays private' +); + +-- ── Fixture: one approved, published report ───────────────────────────────── + +INSERT INTO public.admin_users (discord_id, role, discord_username, display_name) +VALUES ('mr-fix-admin', 'admin', 'mr-fix-admin', 'Correction Admin'); + +INSERT INTO public.seasons (id, name, status, start_date, end_date, is_current) +VALUES ('mr-fix-season', 'Correction Season', 'active', '2026-08-01', '2026-12-31', false); + +INSERT INTO public.orgs ( + id, name, tag, division_id, logo_initials, logo_gradient, primary_color, accent_gradient +) VALUES + ('mr-fix-home', 'Correction Home', 'CFH', 'terra', 'CH', 'from-black to-white', '#000000', 'from-black to-white'), + ('mr-fix-away', 'Correction Away', 'CFA', 'terra', 'CA', 'from-black to-white', '#000000', 'from-black to-white'); + +INSERT INTO public.season_orgs (season_id, org_id, division_id) +VALUES ('mr-fix-season', 'mr-fix-home', 'terra'), ('mr-fix-season', 'mr-fix-away', 'terra'); + +INSERT INTO public.players ( + id, org_id, discord_username, ign, avatar_initials, avatar_gradient, + primary_role, division_id, status +) +SELECT 'mr-fix-home-' || n, 'mr-fix-home', 'mr-fix-home-' || n, 'CF Home ' || n, + 'CH', 'from-black to-white', 'Flex', 'terra', 'active' +FROM generate_series(1, 5) AS n +UNION ALL +SELECT 'mr-fix-away-' || n, 'mr-fix-away', 'mr-fix-away-' || n, 'CF Away ' || n, + 'CA', 'from-black to-white', 'Flex', 'terra', 'active' +FROM generate_series(1, 5) AS n; + +INSERT INTO public.season_rosters (season_id, player_id, org_id, division_id, roster_status) +SELECT 'mr-fix-season', id, org_id, 'terra', 'active' +FROM public.players WHERE id LIKE 'mr-fix-%-%' AND org_id LIKE 'mr-fix-%'; + +INSERT INTO public.matches ( + id, division_id, home_org_id, away_org_id, scheduled_date, scheduled_time, + status, week, season_id +) VALUES + ('mr-fix-match', 'terra', 'mr-fix-home', 'mr-fix-away', '2026-08-25', '19:00', 'scheduled', 1, 'mr-fix-season'), + ('mr-fix-other-match', 'terra', 'mr-fix-home', 'mr-fix-away', '2026-08-26', '19:00', 'scheduled', 1, 'mr-fix-season'); + +INSERT INTO public.match_reports ( + id, match_id, season_id, division_id, status, submitted_by +) VALUES + ('aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-match', 'mr-fix-season', 'terra', 'review', 'mr-fix-admin'), + ('aaaaaaaa-0000-4000-8000-000000000002', 'mr-fix-other-match', 'mr-fix-season', 'terra', 'review', 'mr-fix-admin'); + +-- Builds a well-formed payload: `home_wins` names the games home takes. +CREATE FUNCTION pg_temp.mr_fix_games(home_wins integer[], kills integer) +RETURNS jsonb LANGUAGE sql STABLE AS $fn$ + SELECT jsonb_agg(game ORDER BY game ->> 'gameNumber') + FROM ( + SELECT jsonb_build_object( + 'gameNumber', gn, + 'winningSide', CASE WHEN gn = ANY(home_wins) THEN 'home' ELSE 'away' END, + 'players', ( + SELECT jsonb_agg(jsonb_build_object( + 'playerIgn', p.ign, + 'playerId', p.id, + 'side', CASE WHEN p.org_id = 'mr-fix-home' THEN 'home' ELSE 'away' END, + 'won', (p.org_id = 'mr-fix-home') = (gn = ANY(home_wins)), + 'kills', kills, 'deaths', 2, 'assists', 3, + 'damageDealt', 1000, 'damageMitigated', 500, + 'godPlayed', 'Ymir', 'role', 'Solo' + )) + FROM public.players p + WHERE p.org_id IN ('mr-fix-home', 'mr-fix-away') + ) + ) AS game + FROM unnest(ARRAY[1, 2, 3]) AS gn + ) games; +$fn$; + +CREATE TEMP TABLE mr_fix_approved AS +SELECT public.resolve_match_report_review( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', + pg_temp.mr_fix_games(ARRAY[1, 2], 4) +) AS result; + +SELECT ok( + (SELECT result ->> 'code' = 'applied' AND result ->> 'applied' = 'true' + FROM mr_fix_approved) + AND EXISTS ( + SELECT 1 FROM public.match_reports + WHERE id = 'aaaaaaaa-0000-4000-8000-000000000001' + AND status = 'done' AND home_score = 2 AND away_score = 1 AND total_games = 3 + ), + 'the fixture report is approved and published before any correction' +); + +-- ── The published result is terminal for approval ─────────────────────────── + +CREATE TEMP TABLE mr_fix_reapprove AS +SELECT public.resolve_match_report_review( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', + pg_temp.mr_fix_games(ARRAY[1, 2, 3], 99) +) AS result; + +SELECT ok( + (SELECT result ->> 'code' = 'already_processed' AND result ->> 'applied' = 'false' + FROM mr_fix_reapprove) + AND NOT EXISTS ( + SELECT 1 FROM public.player_match_stats + WHERE match_report_id = 'aaaaaaaa-0000-4000-8000-000000000001' AND kills = 99 + ), + 're-approving a published report stays terminal and never overwrites its stats' +); + +-- ── Correcting a published result ─────────────────────────────────────────── + +CREATE TEMP TABLE mr_fix_correction AS +SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 1, + 'mr-fix-correction-1', 'Scoreboard kills were misread.', + pg_temp.mr_fix_games(ARRAY[1, 2, 3], 7) +) AS result; + +SELECT ok( + (SELECT result ->> 'code' = 'applied' + AND result ->> 'applied' = 'true' + AND result ->> 'revision' = '2' + AND result ->> 'homeScore' = '3' + AND result ->> 'awayScore' = '0' + FROM mr_fix_correction) + AND EXISTS ( + SELECT 1 FROM public.match_reports + WHERE id = 'aaaaaaaa-0000-4000-8000-000000000001' + AND status = 'done' AND revision = 2 + AND home_score = 3 AND away_score = 0 AND total_games = 3 + AND reviewed_by = 'mr-fix-admin' + ), + 'a correction republishes the report at the next revision' +); + +SELECT ok( + (SELECT count(*) = 30 FROM public.player_match_stats + WHERE match_report_id = 'aaaaaaaa-0000-4000-8000-000000000001') + AND NOT EXISTS ( + SELECT 1 FROM public.player_match_stats + WHERE match_report_id = 'aaaaaaaa-0000-4000-8000-000000000001' AND kills <> 7 + ), + 'the corrected stat set fully replaces the published rows' +); + +SELECT ok( + EXISTS ( + SELECT 1 FROM public.matches + WHERE id = 'mr-fix-match' + AND status = 'completed' + AND home_score = 3 AND away_score = 0 + AND winner_org_id = 'mr-fix-home' + AND score = '3-0' + ), + 'the related match keeps its completed status and records the corrected result' +); + +SELECT ok( + EXISTS ( + SELECT 1 FROM public.match_report_corrections + WHERE correction_key = 'mr-fix-correction-1' + AND match_report_id = 'aaaaaaaa-0000-4000-8000-000000000001' + AND actor_discord_id = 'mr-fix-admin' + AND expected_revision = 1 AND resulting_revision = 2 + AND old_value_json ->> 'homeScore' = '2' + AND new_value_json ->> 'homeScore' = '3' + AND jsonb_array_length(old_value_json -> 'playerMatchStats') = 30 + ), + 'the receipt records the actor, revisions, and the complete before and after snapshot' +); + +SELECT ok( + EXISTS ( + SELECT 1 FROM public.audit_logs + WHERE action_type = 'match_report_corrected' + AND entity_id = 'aaaaaaaa-0000-4000-8000-000000000001' + AND actor_discord_id = 'mr-fix-admin' + AND note = 'Scoreboard kills were misread.' + ) + AND EXISTS ( + SELECT 1 FROM public.admin_audit_log + WHERE action = 'match_report_corrected' + AND entity_id = 'aaaaaaaa-0000-4000-8000-000000000001' + ) + AND EXISTS ( + SELECT 1 FROM public.operation_outbox + WHERE aggregate_type = 'match_report' + AND aggregate_id = 'aaaaaaaa-0000-4000-8000-000000000001' + AND event_type = 'match_report_corrected' + ), + 'a correction is audited twice and enqueues its own standings recalculation' +); + +-- ── Retry safety and optimistic concurrency ───────────────────────────────── + +CREATE TEMP TABLE mr_fix_retry AS +SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 1, + 'mr-fix-correction-1', 'Scoreboard kills were misread.', + pg_temp.mr_fix_games(ARRAY[1], 42) +) AS result; + +SELECT ok( + (SELECT result ->> 'code' = 'already_corrected' AND result ->> 'applied' = 'false' + FROM mr_fix_retry) + AND (SELECT count(*) = 1 FROM public.match_report_corrections + WHERE correction_key = 'mr-fix-correction-1') + AND (SELECT revision = 2 FROM public.match_reports + WHERE id = 'aaaaaaaa-0000-4000-8000-000000000001') + AND NOT EXISTS ( + SELECT 1 FROM public.player_match_stats + WHERE match_report_id = 'aaaaaaaa-0000-4000-8000-000000000001' AND kills = 42 + ), + 'an exact correction retry returns the recorded receipt without mutating again' +); + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 1, + 'mr-fix-stale', 'Stale write.', + pg_temp.mr_fix_games(ARRAY[1], 5))$$, + '55000', + 'Match report changed since it was loaded; reload and reapply the correction.', + 'a correction naming a superseded revision is rejected' +); + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000002', 'mr-fix-admin', 1, + 'mr-fix-correction-1', 'Reused key.', + pg_temp.mr_fix_games(ARRAY[1, 2], 5))$$, + '23505', + 'Correction key already recorded for a different match report.', + 'a correction key cannot be replayed against a different report' +); + +-- ── Authorization and lifecycle boundaries ───────────────────────────────── + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-not-admin', 2, + 'mr-fix-unauthorized', 'Not an admin.', + pg_temp.mr_fix_games(ARRAY[1, 2], 5))$$, + '42501', + 'Actor is not an authorized administrator.', + 'only an authorized administrator can correct a published result' +); + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000002', 'mr-fix-admin', 1, + 'mr-fix-not-done', 'Still in review.', + pg_temp.mr_fix_games(ARRAY[1, 2], 5))$$, + '55000', + 'Only a completed match report can be corrected.', + 'a report still in review is approved, not corrected' +); + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 2, + 'mr-fix-blank-reason', ' ', + pg_temp.mr_fix_games(ARRAY[1, 2], 5))$$, + '22023', + 'A correction reason between 1 and 1000 characters is required.', + 'a correction must carry a reason' +); + +-- ── Payload validation matches the approval path ─────────────────────────── + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 2, + 'mr-fix-unlinked', 'Unlinked identity.', + (SELECT jsonb_agg(jsonb_set(game, '{players,0,playerId}', 'null'::jsonb)) + FROM jsonb_array_elements(pg_temp.mr_fix_games(ARRAY[1, 2], 5)) AS game))$$, + '23514', + 'Every official player stat must be linked before approval.', + 'a correction cannot publish an unlinked identity' +); + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 2, + 'mr-fix-tie', 'Tied series.', + (SELECT jsonb_agg(game) + FROM jsonb_array_elements(pg_temp.mr_fix_games(ARRAY[1], 5)) AS game + WHERE (game ->> 'gameNumber')::integer <= 2))$$, + '22023', + 'Reviewed series cannot end in a tie.', + 'a correction cannot record a tied series' +); + +-- The approval path rejects the same payload the same way. This is what keeps +-- the correction validator from drifting away from the rules approval applies. +SELECT throws_ok( + $$SELECT public.resolve_match_report_review( + 'aaaaaaaa-0000-4000-8000-000000000002', 'mr-fix-admin', + (SELECT jsonb_agg(game) + FROM jsonb_array_elements(pg_temp.mr_fix_games(ARRAY[1], 5)) AS game + WHERE (game ->> 'gameNumber')::integer <= 2))$$, + '22023', + 'Reviewed series cannot end in a tie.', + 'the approval path rejects the tied series identically' +); + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 2, + 'mr-fix-duplicate-game', 'Duplicate game number.', + (SELECT jsonb_agg(jsonb_set(game, '{gameNumber}', '1'::jsonb)) + FROM jsonb_array_elements(pg_temp.mr_fix_games(ARRAY[1, 2], 5)) AS game))$$, + '23505', + 'Reviewed payload contains a duplicate game number.', + 'a correction cannot repeat a game number' +); + +SELECT ok( + (SELECT revision = 2 AND home_score = 3 AND away_score = 0 + FROM public.match_reports WHERE id = 'aaaaaaaa-0000-4000-8000-000000000001') + AND (SELECT count(*) = 1 FROM public.match_report_corrections) + AND (SELECT count(*) = 30 FROM public.player_match_stats + WHERE match_report_id = 'aaaaaaaa-0000-4000-8000-000000000001'), + 'every rejected correction leaves the published result untouched' +); + +SELECT * FROM finish(); +ROLLBACK; diff --git a/supabase/tests/README.md b/supabase/tests/README.md index 078bb99..0f7028c 100644 --- a/supabase/tests/README.md +++ b/supabase/tests/README.md @@ -41,6 +41,11 @@ The database contract includes these pgTAP suites: linkage, one-time host capabilities, roster diagnostics, host authorization, optimistic review submission, canonical stat publication, aggregate refresh, and terminal idempotency. +- `030_match_report_result_corrections.test.sql` for audited post-publication + match-report corrections: the approval path staying terminal, revision-checked + replacement, receipt and dual audit evidence, standings re-enqueue, retry + safety, authorization and lifecycle boundaries, and payload validation that + matches the approval path. A `contract.json` is rejected until all required suites exist. CI runs them against a clean local reset, and the protected deployment runs them From 70d0b5183db4cce37e847dd0bb7be611f367cd0b Mon Sep 17 00:00:00 2001 From: Dustin Date: Tue, 1 Sep 2026 13:43:55 +0000 Subject: [PATCH 2/3] fix(match-report): close correction races, key reuse, and cross-division credit Addresses the schema-contract failure and three P1 review findings. - 001_schema_contract: the suite asserts an exact inventory, so the new table and RPC move it to 52 tables and 54 public functions. - Concurrent retries (P1): two retries sharing a key could both observe no receipt; the loser then reloaded the report behind the winner's revision bump and raised the stale-revision error instead of returning the recorded outcome. Reproduced with two concurrent transactions, which failed exactly that way. A transaction-scoped advisory lock on the correction key now serializes them, and the same race returns `already_corrected` with one receipt and one revision bump. - Key reuse (P1): the receipt lookup compared only the report, so a client that accidentally retained a key had its next correction silently discarded as a retry. The key is now bound to the actor, reason, expected revision, and games payload; a mismatch raises rather than replaying. - Cross-division credit (P1): an organization can hold a season roster in more than one division, and the roster lookup checked only season and organization, so a player rostered in another division could be stamped with this match's division. The correction path now requires the roster division to match. That last rule makes the correction path stricter than approval, which has the same gap. Tightening approval means replacing a function that publishes canonical league stats and belongs in its own reviewed change; the repair path takes the stricter rule now, and the migration comment says so. All 26 assertions of 030 pass against the fixture schema, including three new ones for key reuse and cross-division rosters. --- ...120000_match_report_result_corrections.sql | 50 ++++++++++++++--- supabase/tests/001_schema_contract.test.sql | 8 +-- ...0_match_report_result_corrections.test.sql | 56 +++++++++++++++++-- 3 files changed, 96 insertions(+), 18 deletions(-) diff --git a/supabase/migrations/20260901120000_match_report_result_corrections.sql b/supabase/migrations/20260901120000_match_report_result_corrections.sql index f046c44..b087ff7 100644 --- a/supabase/migrations/20260901120000_match_report_result_corrections.sql +++ b/supabase/migrations/20260901120000_match_report_result_corrections.sql @@ -52,11 +52,19 @@ COMMENT ON TABLE public.match_report_corrections IS -- Shared payload validation. -- --- This is the same rule set the approval path enforces before publishing stats. --- It is factored out here so the correction path cannot accept a payload that --- approval would reject; `030_match_report_result_corrections.test.sql` asserts --- both entry points reject an identical battery of invalid payloads with the --- same SQLSTATE and message, which is what keeps the two in step. +-- These are the rules the approval path enforces before publishing stats, so a +-- correction cannot accept a payload approval would reject; +-- `030_match_report_result_corrections.test.sql` asserts both entry points +-- reject an identical battery of invalid payloads with the same SQLSTATE and +-- message, which is what keeps the two in step. +-- +-- One rule is deliberately stronger here: the roster lookup is scoped to the +-- match division. An organization can hold a season roster in more than one +-- division, and the approval path checks only season and organization, so it +-- can still stamp a stat row with a division the player is not rostered in. +-- Tightening approval means replacing a function that publishes canonical +-- league stats, which belongs in its own reviewed change; a correction is the +-- repair path, so it takes the stricter rule now. CREATE OR REPLACE FUNCTION private.validate_match_report_games( p_match_id text, p_games jsonb @@ -80,6 +88,7 @@ DECLARE v_known_player_ign text; v_known_player_org_id text; v_known_roster_status text; + v_known_division_id text; v_game_count integer; v_home_count integer; v_away_count integer; @@ -250,8 +259,9 @@ BEGIN END IF; v_seen_player_ids := array_append(v_seen_player_ids, v_player_id); - SELECT players.ign, rosters.org_id, rosters.roster_status - INTO v_known_player_ign, v_known_player_org_id, v_known_roster_status + SELECT players.ign, rosters.org_id, rosters.roster_status, rosters.division_id + INTO v_known_player_ign, v_known_player_org_id, v_known_roster_status, + v_known_division_id FROM public.players players JOIN public.season_rosters rosters ON rosters.player_id = players.id @@ -266,6 +276,12 @@ BEGIN IF v_known_roster_status <> 'active' OR v_known_player_org_id <> v_expected_org_id THEN RAISE EXCEPTION USING ERRCODE = '23514', MESSAGE = 'Supplied player is not active on the expected organization.'; END IF; + -- An organization can hold a season roster in more than one division. + -- Without this the stat row would be stamped with the match division + -- while the player is rostered in another one. + IF v_known_division_id IS DISTINCT FROM v_match.division_id THEN + RAISE EXCEPTION USING ERRCODE = '23514', MESSAGE = 'Supplied player is not rostered in the match division.'; + END IF; END IF; END LOOP; @@ -351,8 +367,18 @@ BEGIN MESSAGE = 'Actor is not an authorized administrator.'; END IF; + -- Serialize on the correction key before reading the receipt. Two retries + -- that start before either commits would otherwise both see no receipt; the + -- loser would then reload the report behind the winner's revision bump and + -- fail the stale-revision check instead of returning the recorded outcome. + PERFORM pg_advisory_xact_lock( + hashtextextended('match_report_correction:' || v_correction_key, 0) + ); + -- An exact retry after an uncertain network result returns the recorded - -- outcome instead of correcting a second time. + -- outcome instead of correcting a second time. The key is bound to the whole + -- request, so a client that accidentally reuses a key for a different + -- correction is told rather than having its change silently discarded. SELECT * INTO v_existing FROM public.match_report_corrections WHERE correction_key = v_correction_key; @@ -362,6 +388,14 @@ BEGIN ERRCODE = '23505', MESSAGE = 'Correction key already recorded for a different match report.'; END IF; + IF v_existing.actor_discord_id <> v_actor_discord_id + OR v_existing.reason <> v_reason + OR v_existing.expected_revision <> p_expected_revision + OR v_existing.request_json <> jsonb_build_object('games', p_games) THEN + RAISE EXCEPTION USING + ERRCODE = '23505', + MESSAGE = 'Correction key already recorded for a different correction.'; + END IF; RETURN v_existing.result_json || jsonb_build_object('code', 'already_corrected', 'applied', false); END IF; diff --git a/supabase/tests/001_schema_contract.test.sql b/supabase/tests/001_schema_contract.test.sql index 628a54e..90354e8 100644 --- a/supabase/tests/001_schema_contract.test.sql +++ b/supabase/tests/001_schema_contract.test.sql @@ -6,11 +6,11 @@ SET LOCAL search_path TO extensions, public, pg_catalog; SELECT plan(26); SELECT ok( - (SELECT count(*) = 51 + (SELECT count(*) = 52 FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE n.nspname = 'public' AND c.relkind IN ('r', 'p')), - 'the contract contains all 51 application tables' + 'the contract contains all 52 application tables' ); SELECT ok(to_regclass('public.players') IS NOT NULL, 'players exists'); SELECT has_column('public', 'players', 'avatar_url', 'players has a canonical avatar URL'); @@ -35,11 +35,11 @@ SELECT ok(to_regclass('public.bug_reports') IS NOT NULL, 'bug_reports exists'); SELECT ok(to_regclass('public.bug_report_rate_limits') IS NOT NULL, 'bug report rate limits exist'); SELECT has_column('public', 'seasons', 'is_current', 'seasons has an explicit current marker'); SELECT ok( - (SELECT count(*) = 53 + (SELECT count(*) = 54 FROM pg_proc p JOIN pg_namespace n ON n.oid = p.pronamespace WHERE n.nspname = 'public'), - 'the contract contains the 53 verified production functions' + 'the contract contains the 54 verified production functions' ); SELECT ok(to_regprocedure('public.replace_standings(jsonb)') IS NOT NULL, 'replace_standings exists'); SELECT ok(to_regprocedure('public.replace_match_report_stats(uuid,jsonb)') IS NOT NULL, 'replace_match_report_stats exists'); diff --git a/supabase/tests/030_match_report_result_corrections.test.sql b/supabase/tests/030_match_report_result_corrections.test.sql index 4fb6883..be7e24e 100644 --- a/supabase/tests/030_match_report_result_corrections.test.sql +++ b/supabase/tests/030_match_report_result_corrections.test.sql @@ -3,7 +3,7 @@ BEGIN; CREATE EXTENSION IF NOT EXISTS pgtap WITH SCHEMA extensions; SET LOCAL search_path TO extensions, public, pg_catalog; -SELECT plan(23); +SELECT plan(26); -- ── Contract surface ──────────────────────────────────────────────────────── @@ -251,11 +251,12 @@ SELECT ok( -- ── Retry safety and optimistic concurrency ───────────────────────────────── +-- Byte-for-byte the first correction, as an uncertain client would resend it. CREATE TEMP TABLE mr_fix_retry AS SELECT public.correct_match_report_result( 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 1, 'mr-fix-correction-1', 'Scoreboard kills were misread.', - pg_temp.mr_fix_games(ARRAY[1], 42) + pg_temp.mr_fix_games(ARRAY[1, 2, 3], 7) ) AS result; SELECT ok( @@ -263,12 +264,10 @@ SELECT ok( FROM mr_fix_retry) AND (SELECT count(*) = 1 FROM public.match_report_corrections WHERE correction_key = 'mr-fix-correction-1') + -- A second correction would have advanced the revision to 3. AND (SELECT revision = 2 FROM public.match_reports WHERE id = 'aaaaaaaa-0000-4000-8000-000000000001') - AND NOT EXISTS ( - SELECT 1 FROM public.player_match_stats - WHERE match_report_id = 'aaaaaaaa-0000-4000-8000-000000000001' AND kills = 42 - ), + AND (SELECT result ->> 'revision' = '2' FROM mr_fix_retry), 'an exact correction retry returns the recorded receipt without mutating again' ); @@ -373,6 +372,51 @@ SELECT throws_ok( 'a correction cannot repeat a game number' ); +-- A reused key must be bound to the whole request, not just the report, or a +-- client that accidentally retains its previous key has its next correction +-- silently discarded as a retry. +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 2, + 'mr-fix-correction-1', 'A different reason entirely.', + pg_temp.mr_fix_games(ARRAY[1, 2, 3], 7))$$, + '23505', + 'Correction key already recorded for a different correction.', + 'a reused correction key with a different request is rejected, not replayed' +); + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 2, + 'mr-fix-correction-1', 'Scoreboard kills were misread.', + pg_temp.mr_fix_games(ARRAY[1, 2], 7))$$, + '23505', + 'Correction key already recorded for a different correction.', + 'a reused correction key with a different payload is rejected, not replayed' +); + +-- An organization can hold a season roster in more than one division; a player +-- rostered elsewhere must not earn canonical stats in this match's division. +INSERT INTO public.season_orgs (season_id, org_id, division_id) +VALUES ('mr-fix-season', 'mr-fix-home', 'solar'); +UPDATE public.season_rosters +SET division_id = 'solar' +WHERE season_id = 'mr-fix-season' AND player_id = 'mr-fix-home-1'; + +SELECT throws_ok( + $$SELECT public.correct_match_report_result( + 'aaaaaaaa-0000-4000-8000-000000000001', 'mr-fix-admin', 2, + 'mr-fix-cross-division', 'Cross-division roster member.', + pg_temp.mr_fix_games(ARRAY[1, 2], 6))$$, + '23514', + 'Supplied player is not rostered in the match division.', + 'a correction cannot credit a player rostered in another division' +); + +UPDATE public.season_rosters +SET division_id = 'terra' +WHERE season_id = 'mr-fix-season' AND player_id = 'mr-fix-home-1'; + SELECT ok( (SELECT revision = 2 AND home_score = 3 AND away_score = 0 FROM public.match_reports WHERE id = 'aaaaaaaa-0000-4000-8000-000000000001') From ca731de0fa8f69f3c1f72bff67dcac98e6ea7f23 Mon Sep 17 00:00:00 2001 From: Dustin Date: Tue, 1 Sep 2026 13:54:42 +0000 Subject: [PATCH 3/3] test(match-report): make the correction payload builder deterministic CI failed the exact-retry assertion with "Correction key already recorded for a different correction." The fixture's per-game player aggregate had no ORDER BY, so two calls could serialize the ten players in different orders and the second request no longer matched the recorded one. It passed locally only because that cluster happened to return a stable heap order. The builder now orders players by id. Also documents, where a caller will read it, that the games comparison is exact including array order: a retry is a resend of the same request, not an equivalent one. 001_schema_contract passes with the updated inventory; this was the only remaining failure. --- .../20260901120000_match_report_result_corrections.sql | 4 ++++ .../tests/030_match_report_result_corrections.test.sql | 7 +++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/supabase/migrations/20260901120000_match_report_result_corrections.sql b/supabase/migrations/20260901120000_match_report_result_corrections.sql index b087ff7..f656fd9 100644 --- a/supabase/migrations/20260901120000_match_report_result_corrections.sql +++ b/supabase/migrations/20260901120000_match_report_result_corrections.sql @@ -379,6 +379,10 @@ BEGIN -- outcome instead of correcting a second time. The key is bound to the whole -- request, so a client that accidentally reuses a key for a different -- correction is told rather than having its change silently discarded. + -- + -- The games comparison is exact, including array order: a retry is a resend + -- of the same request, not an equivalent one. A caller that rebuilds the + -- payload in a different order must use a new correction key. SELECT * INTO v_existing FROM public.match_report_corrections WHERE correction_key = v_correction_key; diff --git a/supabase/tests/030_match_report_result_corrections.test.sql b/supabase/tests/030_match_report_result_corrections.test.sql index be7e24e..484cbfe 100644 --- a/supabase/tests/030_match_report_result_corrections.test.sql +++ b/supabase/tests/030_match_report_result_corrections.test.sql @@ -122,8 +122,11 @@ RETURNS jsonb LANGUAGE sql STABLE AS $fn$ 'damageDealt', 1000, 'damageMitigated', 500, 'godPlayed', 'Ymir', 'role', 'Solo' )) - FROM public.players p - WHERE p.org_id IN ('mr-fix-home', 'mr-fix-away') + FROM ( + SELECT * FROM public.players + WHERE org_id IN ('mr-fix-home', 'mr-fix-away') + ORDER BY id + ) p ) ) AS game FROM unnest(ARRAY[1, 2, 3]) AS gn