diff --git a/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md b/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md index 32f4b0e..ec14b69 100644 --- a/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md +++ b/docs/superpowers/plans/2026-05-30-completed-game-portfolio.md @@ -77,7 +77,8 @@ - `f493836` `fix(data): reject stale portfolio trigger snapshots` - `da0a306` `fix(data): enforce completed portfolio sessions` - `a28b75d` `fix(data): align portfolio mutation lock order` -- Current fix cycle: `fix(data): serialize portfolio future reschedules` +- `d762ecc` `fix(data): serialize portfolio future reschedules` +- Current fix cycle: `fix(data): lock racing portfolio publications` **Files:** - Create: `tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs` @@ -413,16 +414,38 @@ CREATE FUNCTION unpublish_public_portfolio_games_for_future_session() RETURNS TRIGGER LANGUAGE plpgsql AS $$ +DECLARE + final_scheduled_at TIMESTAMPTZ; BEGIN - IF OLD.scheduled_at IS DISTINCT FROM NEW.scheduled_at - AND NEW.scheduled_at >= now() THEN + SELECT s.scheduled_at + INTO final_scheduled_at + FROM sessions s + WHERE s.id = NEW.id; + + IF final_scheduled_at >= now() THEN + PERFORM pg.id + FROM portfolio_games pg + WHERE EXISTS ( + SELECT 1 + FROM portfolio_game_sessions pgs + JOIN sessions s ON s.id = pgs.session_id + WHERE pgs.portfolio_game_id = pg.id + AND s.scheduled_at >= now() + ) + ORDER BY pg.id + FOR UPDATE OF pg; + UPDATE portfolio_games pg SET is_public = false, updated_at = now() - FROM portfolio_game_sessions pgs - WHERE pgs.portfolio_game_id = pg.id - AND pgs.session_id = NEW.id - AND pg.is_public = true; + WHERE pg.is_public = true + AND EXISTS ( + SELECT 1 + FROM portfolio_game_sessions pgs + JOIN sessions s ON s.id = pgs.session_id + WHERE pgs.portfolio_game_id = pg.id + AND s.scheduled_at >= now() + ); END IF; RETURN NULL; @@ -485,7 +508,7 @@ CREATE INDEX ix_portfolio_game_reviews_pending WHERE moderation_status = 'Pending'; ``` -The deferred constraint triggers retain the link-table `ON DELETE CASCADE` behavior. At transaction commit validators acquire the same transaction-level advisory lock and reject a surviving published card when either required link set is empty or any linked session has `scheduled_at >= now()`. The intentionally global lock is appropriate for low-volume portfolio publication writes: under the application default `READ COMMITTED` isolation level it serializes validation, prevents write-skew across distinct child links, and gives multi-card transactions one lock order. PostgreSQL retains stale snapshots under `REPEATABLE READ` and `SERIALIZABLE`, so the guard rejects every triggered portfolio write at those isolation levels with `0A000`. The deferred future-reschedule trigger re-reads the final session row, skips intermediate future values that end in the past, and for a final future value locks all currently public cards linked to any final-future session in `portfolio_games.id` order before one guarded unpublish update. This separate global row-lock pass avoids opposing batch order without adding the validator advisory lock before card locks. At `READ COMMITTED`, draft edits, explicit unpublishing, future reschedules, and card or club cascade deletion remain valid. Normal session-deletion handlers use the same `sessions` then `portfolio_games` lock order: explicitly lock the target session row, unpublish linked cards, then delete the session. +The deferred constraint triggers retain the link-table `ON DELETE CASCADE` behavior. At transaction commit validators acquire the same transaction-level advisory lock and reject a surviving published card when either required link set is empty or any linked session has `scheduled_at >= now()`. The intentionally global lock is appropriate for low-volume portfolio publication writes: under the application default `READ COMMITTED` isolation level it serializes validation, prevents write-skew across distinct child links, and gives multi-card transactions one lock order. PostgreSQL retains stale snapshots under `REPEATABLE READ` and `SERIALIZABLE`, so the guard rejects every triggered portfolio write at those isolation levels with `0A000`. The deferred future-reschedule trigger re-reads the final session row, skips intermediate future values that end in the past, and for a final future value locks all cards linked to any final-future session in `portfolio_games.id` order before one guarded public-card unpublish update. The lock phase deliberately includes committed drafts so a concurrent draft-to-public publication cannot pass validation against the pre-reschedule session snapshot and commit afterward. This separate global row-lock pass avoids opposing batch order without adding the validator advisory lock before card locks. At `READ COMMITTED`, draft edits, explicit unpublishing, future reschedules, and card or club cascade deletion remain valid. Normal session-deletion handlers use the same `sessions` then `portfolio_games` lock order: explicitly lock the target session row, unpublish linked cards, then delete the session. - [ ] **Step 5: Lock sessions before explicitly unpublishing linked cards in both session-deletion handlers** diff --git a/docs/superpowers/specs/2026-05-30-completed-game-portfolio-design.md b/docs/superpowers/specs/2026-05-30-completed-game-portfolio-design.md index aa7f5be..15372a9 100644 --- a/docs/superpowers/specs/2026-05-30-completed-game-portfolio-design.md +++ b/docs/superpowers/specs/2026-05-30-completed-game-portfolio-design.md @@ -81,7 +81,7 @@ Application validation additionally requires at least one linked session, every Deferred database constraint triggers validate the same invariant at transaction commit after a card transitions to public, a session link is inserted, deleted, moved, or repointed, or a required master link is deleted or moved. They raise a check-violation error if a published card would commit without both required link sets or with any linked session where `scheduled_at >= now()`. Before checking state, each validator acquires the same transaction-level PostgreSQL advisory lock, `pg_advisory_xact_lock(20260530, 108)`. Portfolio publication writes are low volume, so this intentionally global lock serializes invariant validation with one lock order, prevents write-skew under the application default `READ COMMITTED` isolation level, and avoids multi-card deadlocks. PostgreSQL keeps a stale snapshot after waiting under `REPEATABLE READ` or `SERIALIZABLE`, so the guard rejects every triggered portfolio write at those levels; callers must use `READ COMMITTED` for portfolio mutations. -A deferred `sessions.scheduled_at` trigger atomically unpublishes linked public cards when a completed session is finally rescheduled into the future, preserving the first `published_at`. Because deferred row triggers retain their event-time `NEW`, the trigger re-reads the final `sessions.scheduled_at` before acting. For a final future value it takes row locks for all currently public cards linked to any final-future session in `portfolio_games.id` order, then unpublishes the matching cards in one guarded update. The low-volume global pass gives batch reschedules one card-lock order without taking the publication validator advisory lock before card locks. Session mutation paths use `sessions` before linked `portfolio_games`; normal session-deletion handlers explicitly lock the target session row, unpublish linked cards in the same transaction, and only then delete the session. The link foreign keys retain `ON DELETE CASCADE`; when the card itself or its owning club is deleted at `READ COMMITTED`, deferred validation sees no surviving published card and remains harmless. +A deferred `sessions.scheduled_at` trigger atomically unpublishes linked public cards when a completed session is finally rescheduled into the future, preserving the first `published_at`. Because deferred row triggers retain their event-time `NEW`, the trigger re-reads the final `sessions.scheduled_at` before acting. For a final future value it takes row locks for all cards linked to any final-future session in `portfolio_games.id` order, including committed drafts, then unpublishes the matching public cards in one guarded update. Including drafts prevents a concurrent draft-to-public publication from validating against the pre-reschedule session snapshot and committing afterward. The low-volume global pass gives batch reschedules one card-lock order without taking the publication validator advisory lock before card locks. Session mutation paths use `sessions` before linked `portfolio_games`; normal session-deletion handlers explicitly lock the target session row, unpublish linked cards in the same transaction, and only then delete the session. The link foreign keys retain `ON DELETE CASCADE`; when the card itself or its owning club is deleted at `READ COMMITTED`, deferred validation sees no surviving published card and remains harmless. ### `portfolio_game_sessions` diff --git a/src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql b/src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql index dd3fb39..757c1a6 100644 --- a/src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql +++ b/src/GmRelay.Bot/Migrations/V029__add_completed_game_portfolios_and_reviews.sql @@ -130,8 +130,7 @@ BEGIN IF final_scheduled_at >= now() THEN PERFORM pg.id FROM portfolio_games pg - WHERE pg.is_public = true - AND EXISTS ( + WHERE EXISTS ( SELECT 1 FROM portfolio_game_sessions pgs JOIN sessions s ON s.id = pgs.session_id diff --git a/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs b/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs index 1bcef04..37b442b 100644 --- a/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs +++ b/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationPostgresTests.cs @@ -497,9 +497,12 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi var database = await fixture.CreateMigratedDatabaseAsync(); await using var publishConnection = await database.OpenConnectionAsync(); await using var rescheduleConnection = await database.OpenConnectionAsync(); + await using var observerConnection = await database.OpenConnectionAsync(); var seed = await SeedCardAsync(publishConnection, isPublic: false); await using var publishTransaction = await publishConnection.BeginTransactionAsync(); await using var rescheduleTransaction = await rescheduleConnection.BeginTransactionAsync(); + var publishPid = await GetBackendPidAsync(publishConnection, publishTransaction); + var reschedulePid = await GetBackendPidAsync(rescheduleConnection, rescheduleTransaction); await ExecuteNonQueryAsync( publishConnection, @@ -518,14 +521,15 @@ public sealed class PortfolioMigrationPostgresTests(PortfolioMigrationPostgresFi rescheduleTransaction, new NpgsqlParameter("sessionId", seed.SessionIds[0])); - var commitStates = await Task.WhenAll( - CommitAndCaptureSqlStateAsync(publishTransaction), - CommitAndCaptureSqlStateAsync(rescheduleTransaction)).WaitAsync(CommandTimeout); + var forceRescheduleTriggerTask = ExecuteNonQueryAsync( + rescheduleConnection, + "SET CONSTRAINTS trg_sessions_unpublish_public_portfolio_games_for_future_reschedule IMMEDIATE", + rescheduleTransaction); + await WaitUntilBlockedByAsync(observerConnection, reschedulePid, publishPid); - Assert.True( - commitStates[0] is null or PostgresErrorCodes.CheckViolation, - $"Unexpected publish SQLSTATE: {commitStates[0] ?? ""}."); - Assert.Null(commitStates[1]); + Assert.Null(await CommitAndCaptureSqlStateAsync(publishTransaction).WaitAsync(CommandTimeout)); + await forceRescheduleTriggerTask.WaitAsync(CommandTimeout); + await rescheduleTransaction.CommitAsync().WaitAsync(CommandTimeout); await using var verificationConnection = await database.OpenConnectionAsync(); Assert.False(await ExecuteScalarAsync( diff --git a/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs b/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs index 30c58fc..165bf09 100644 --- a/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs +++ b/tests/GmRelay.Bot.Tests/Web/PortfolioMigrationTests.cs @@ -48,6 +48,7 @@ public sealed class PortfolioMigrationTests Assert.Contains("CREATE FUNCTION unpublish_public_portfolio_games_for_future_session() RETURNS TRIGGER LANGUAGE plpgsql", normalizedMigration, StringComparison.Ordinal); Assert.Contains("SELECT s.scheduled_at INTO final_scheduled_at FROM sessions s WHERE s.id = NEW.id;", normalizedMigration, StringComparison.Ordinal); Assert.Contains("IF final_scheduled_at >= now() THEN", normalizedMigration, StringComparison.Ordinal); + Assert.Contains("PERFORM pg.id FROM portfolio_games pg WHERE EXISTS", normalizedMigration, StringComparison.Ordinal); Assert.Contains("ORDER BY pg.id FOR UPDATE OF pg;", normalizedMigration, StringComparison.Ordinal); Assert.Contains("UPDATE portfolio_games pg SET is_public = false, updated_at = now() WHERE pg.is_public = true AND EXISTS (SELECT 1 FROM portfolio_game_sessions pgs JOIN sessions s ON s.id = pgs.session_id WHERE pgs.portfolio_game_id = pg.id AND s.scheduled_at >= now());", normalizedMigration, StringComparison.Ordinal); Assert.Contains("CREATE CONSTRAINT TRIGGER trg_sessions_unpublish_public_portfolio_games_for_future_reschedule AFTER UPDATE OF scheduled_at ON sessions DEFERRABLE INITIALLY DEFERRED FOR EACH ROW EXECUTE FUNCTION unpublish_public_portfolio_games_for_future_session();", normalizedMigration, StringComparison.Ordinal);