diff --git a/docs/plans/NEO-146-implementation-plan.md b/docs/plans/NEO-146-implementation-plan.md index 2083e50..78be83c 100644 --- a/docs/plans/NEO-146-implementation-plan.md +++ b/docs/plans/NEO-146-implementation-plan.md @@ -68,8 +68,8 @@ Durable per-player **`ContractInstance`** rows (active/completed) and append-onl - **Types:** `ContractInstanceStatus`, `ContractInstanceState`, `ContractInstanceIds`, `ContractInstanceReasonCodes`, `ContractOutcomeRow`, `ContractOutcomeIds`. - **Stores:** `IContractInstanceStore` / `IContractOutcomeStore`; in-memory + Postgres (`V011`, `V012`) when connection string set; partial unique index + app-level one-active enforcement. - **DI:** `AddContractInstanceStores` in `Program.cs`; in-memory override in `InMemoryWebApplicationFactory`. -- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`867` tests green). -- **In-memory outcome guard:** `InMemoryContractOutcomeStore.TryAppend` denies when **`IContractInstanceStore.TryGet`** is false (Postgres FK parity). +- **Tests:** `InMemoryContractInstanceStoreTests`, `InMemoryContractOutcomeStoreTests`, Postgres persistence integration tests; host DI smoke (`871` tests green). +- **In-memory outcome guard:** `InMemoryContractOutcomeStore.TryAppend` denies when **`IContractInstanceStore.TryGet`** is false (Postgres FK + player/instance SQL guard parity). - **Docs:** `server/README.md` contract instance + outcome store sections. - **Bruno:** `bruno/neon-sprawl-server/contract-stores/` health smoke (startup-only; no contract HTTP in E7M4-03). diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs index f8aaae4..3bd6c0a 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/ContractOutcomePersistenceIntegrationTests.cs @@ -129,6 +129,104 @@ public sealed class ContractOutcomePersistenceIntegrationTests(PostgresIntegrati Assert.False(appended); } + [RequirePostgresFact] + public async Task TryAppend_ShouldReturnFalse_WhenPlayerIdDoesNotMatchInstanceOwner() + { + // Arrange + await ResetContractTablesAsync(); + const string otherPlayerId = "other-player-pg-001"; + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(otherPlayerId, InstanceId); + var outcomeRow = new ContractOutcomeRow( + OutcomeRowId, + InstanceId, + otherPlayerId, + idempotencyKey, + [new RewardItemGrantApplied("scrap_metal_bulk", 5)], + [], + [], + RecordedAt); + using (var scope = Factory.Services.CreateScope()) + { + var instanceStore = scope.ServiceProvider.GetRequiredService(); + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + await SeedPostgresPlayerAsync(otherPlayerId); + // Act + var appended = outcomeStore.TryAppend(outcomeRow); + // Assert + Assert.False(appended); + Assert.Empty(outcomeStore.GetOutcomesForInstance(InstanceId)); + } + } + + [RequirePostgresFact] + public async Task TryAppend_ShouldAllowOnlyOneSuccess_WhenConcurrentDuplicateRowsRace() + { + // Arrange + await ResetContractTablesAsync(); + var idempotencyKey = ContractOutcomeIds.MakeIdempotencyKey(PlayerId, InstanceId); + var outcomeRow = new ContractOutcomeRow( + OutcomeRowId, + InstanceId, + PlayerId, + idempotencyKey, + [new RewardItemGrantApplied("scrap_metal_bulk", 5)], + [], + [], + RecordedAt); + using (var scope = Factory.Services.CreateScope()) + { + var instanceStore = scope.ServiceProvider.GetRequiredService(); + Assert.True(instanceStore.TryCreateActive( + PlayerId, + InstanceId, + TemplateId, + SeedBucket, + IssuedAt, + out _)); + } + + // Act + var results = await Task.WhenAll( + Enumerable.Range(0, 8).Select(_ => Task.Run(() => + { + using var scope = Factory.Services.CreateScope(); + var outcomeStore = scope.ServiceProvider.GetRequiredService(); + return outcomeStore.TryAppend(outcomeRow); + }))); + + // Assert + Assert.Equal(1, results.Count(static r => r)); + Assert.Equal(7, results.Count(static r => !r)); + } + + private static async Task SeedPostgresPlayerAsync(string playerId) + { + var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl"); + if (string.IsNullOrWhiteSpace(cs)) + { + throw new InvalidOperationException("ConnectionStrings__NeonSprawl is not set."); + } + + await using var conn = new NpgsqlConnection(cs); + await conn.OpenAsync(); + await using var cmd = new NpgsqlCommand( + """ + INSERT INTO player_position (player_id, pos_x, pos_y, pos_z, sequence) + VALUES (@pid, 0, 0, 0, 0) + ON CONFLICT (player_id) DO NOTHING; + """, + conn); + cmd.Parameters.AddWithValue("pid", playerId); + await cmd.ExecuteNonQueryAsync(); + } + private async Task ResetContractTablesAsync() { var cs = Environment.GetEnvironmentVariable("ConnectionStrings__NeonSprawl"); diff --git a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs index 9acbaf2..ac1783c 100644 --- a/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Contracts/InMemoryContractOutcomeStoreTests.cs @@ -93,6 +93,20 @@ public sealed class InMemoryContractOutcomeStoreTests Assert.Empty(store.GetOutcomesForInstance(InstanceId)); } + [Fact] + public void TryAppend_ShouldReturnFalse_WhenPlayerIdDoesNotMatchInstanceOwner() + { + // Arrange + var (instanceStore, store) = CreateStores(); + SeedActiveInstance(instanceStore); + var row = CreatePrototypeOutcome() with { PlayerId = "other-player-xyz" }; + // Act + var appended = store.TryAppend(row); + // Assert + Assert.False(appended); + Assert.Empty(store.GetOutcomesForInstance(InstanceId)); + } + private static (InMemoryContractInstanceStore InstanceStore, InMemoryContractOutcomeStore OutcomeStore) CreateStores() { var options = Options.Create(new GamePositionOptions { DevPlayerId = PlayerId }); diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs index 3e56365..ef6f253 100644 --- a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeBootstrap.cs @@ -13,6 +13,8 @@ public static class PostgresContractOutcomeBootstrap public static void EnsureSchema(Npgsql.NpgsqlDataSource dataSource) { + PostgresContractInstanceBootstrap.EnsureSchema(dataSource); + if (Volatile.Read(ref _schemaReady) != 0) { return; diff --git a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs index fcd694d..f9bab91 100644 --- a/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs +++ b/server/NeonSprawl.Server/Game/Contracts/PostgresContractOutcomeStore.cs @@ -29,7 +29,10 @@ public sealed class PostgresContractOutcomeStore(Npgsql.NpgsqlDataSource dataSou SELECT @id, @instance_id, @pid, @key, @items::jsonb, @skill_xp::jsonb, @rep::jsonb, @recorded WHERE NOT EXISTS ( SELECT 1 FROM contract_outcome - WHERE id = @id OR idempotency_key = @key); + WHERE id = @id OR idempotency_key = @key) + AND EXISTS ( + SELECT 1 FROM contract_instance + WHERE contract_instance_id = @instance_id AND player_id = @pid); """, conn); cmd.Parameters.AddWithValue("id", normalized.Id); @@ -44,7 +47,9 @@ public sealed class PostgresContractOutcomeStore(Npgsql.NpgsqlDataSource dataSou { return cmd.ExecuteNonQuery() == 1; } - catch (Npgsql.PostgresException ex) when (ex.SqlState == Npgsql.PostgresErrorCodes.ForeignKeyViolation) + catch (Npgsql.PostgresException ex) when ( + ex.SqlState == Npgsql.PostgresErrorCodes.ForeignKeyViolation || + ex.SqlState == Npgsql.PostgresErrorCodes.UniqueViolation) { return false; } diff --git a/server/README.md b/server/README.md index dc42b43..ce67670 100644 --- a/server/README.md +++ b/server/README.md @@ -292,7 +292,7 @@ Append-only contract completion audit rows live in **`IContractOutcomeStore`**. **Store interface methods:** -- **`TryAppend`** — first row id or idempotency key returns `true`; duplicates return `false`. Missing **`contract_instance`** row returns `false` (Postgres FK; in-memory checks **`IContractInstanceStore.TryGet`**) — orchestrators (NEO-149) must create the instance before appending an outcome. +- **`TryAppend`** — first row id or idempotency key returns `true`; duplicates return `false`. Missing **`contract_instance`** row, or a **`player_id`** that does not match the instance owner, returns `false` (Postgres SQL guard + FK/unique catch; in-memory checks **`IContractInstanceStore.TryGet`**) — orchestrators (NEO-149) must create the instance before appending an outcome. - **`TryGetByIdempotencyKey`** — read one outcome for delivery replay / audit tests. - **`GetOutcomesForInstance`** — list outcomes for one contract instance id (ordered by recorded time). - **`TryClearForInstance`** — dev fixture only.