neon-sprawl/docs/plans/NEON-15-implementation-plan.md

NEON-15 — Implementation plan

Story reference

Field	Value
Key	NEON-15
Title	Dispose NpgsqlDataSource on application shutdown
Jira	NEON-15
Parent	NEON-13 — Tech Debt
Related	NS-17 code review — docs/reviews/2026-03-30-NS-17.md (suggestion #2)

Goal, scope, and out-of-scope

Goal: Ensure the shared NpgsqlDataSource registered when ConnectionStrings:NeonSprawl is set is released cleanly on host shutdown, without breaking in-memory overrides or Postgres integration tests / WebApplicationFactory.

In scope

• Explicit, observable shutdown cleanup for the PostgreSQL path (align with Npgsql 10 + ASP.NET Core hosting patterns; no new analyzer warnings).
• Keep single shared data source for PostgresPositionStateStore and PostgresDevPlayerSeedHostedService.
• Update test host overrides that strip Postgres registrations so they remain consistent.

Out of scope

• Connection pooling tuning, migration tooling, or schema changes.
• Client / Godot code.

Decisions (planning + implementation)

Decision	Choice
Lifecycle / double dispose	Option 2 — internal shutdown IHostedService calls DisposeAsync on the shared NpgsqlDataSource; no non-disposable holder. Rationale: smallest change; Npgsql 10 + full dotnet test showed clean teardown (no failures from a second dispose path).
ConfigureAwait	Not used in StopAsync — matches existing server hosted-service style (PostgresDevPlayerSeedHostedService).

Acceptance criteria checklist

• Graceful shutdown disposes the shared NpgsqlDataSource (or a documented alternative if upstream guidance prefers another pattern).
• dotnet test still passes (in-memory and Postgres integration paths).
• No new analyzer warnings; align with Npgsql / ASP.NET Core guidance for the installed package version.

Technical approach

1. Shutdown ordering: IHostedService.StopAsync runs in reverse registration order. Register NpgsqlDataSourceShutdownHostedService before PostgresDevPlayerSeedHostedService so the dispose hook runs last among hosted services (after the seed service’s StopAsync), while ordinary request handling has already wound down per host shutdown semantics.
2. Disposal: In that hosted service’s StopAsync, cancellationToken.ThrowIfCancellationRequested() then await dataSource.DisposeAsync() (Npgsql has no DisposeAsync(CancellationToken)). StartAsync is a no-op.
3. Double disposal: The root ServiceProvider may still dispose the same singleton during host teardown. Settled: Option 2 kept; verification — dotnet test on solution (36 tests passed) including in-memory and Postgres factory dispose paths; no change required to store/seed constructors.
4. Registration clarity: AddSingleton<NpgsqlDataSource>(_ => NpgsqlDataSource.Create(trimmed)) so the service type is explicit to DI and analyzers.
5. Shutdown verification: WebApplicationFactory disposal in integration tests exercises host stop with a live NpgsqlDataSource when ConnectionStrings__NeonSprawl is set. Optional: Run the server with a real connection string, Ctrl+C, confirm clean pool-related logs for extra operator confidence.

Files to add

Path	Purpose
server/NeonSprawl.Server/Game/PositionState/NpgsqlDataSourceShutdownHostedService.cs	Internal IHostedService that disposes the shared NpgsqlDataSource on shutdown (registered before the seed hosted service so it stops last).

Files to modify

Path	Rationale
server/NeonSprawl.Server/Game/PositionState/PositionStateServiceCollectionExtensions.cs	Register NpgsqlDataSource with an explicit service type if adjusted; register the shutdown hosted service before PostgresDevPlayerSeedHostedService; keep the existing branch for in-memory vs Postgres.
server/NeonSprawl.Server.Tests/InMemoryWebApplicationFactory.cs	Extend the descriptor removal loop to drop the new shutdown hosted service when forcing the in-memory store (mirror the PostgresDevPlayerSeedHostedService pattern).
server/NeonSprawl.Server/Game/PositionState/PostgresPositionStateStore.cs	N/A (shipped) — holder pattern not needed after Option 2 verification.
server/NeonSprawl.Server/Game/PositionState/PostgresDevPlayerSeedHostedService.cs	N/A (shipped) — same.

Tests

Path / action	What to cover
dotnet test (full server test project)	Regression: InMemoryWebApplicationFactory suites unchanged; PostgresWebApplicationFactory / PostgresIntegrationHarness / SecondProcess_ShouldReadLastPosition_AfterFirstFactoryDisposed still pass (factory dispose + multi-factory scenario).
No new test file required for kickoff	If shutdown-only logic is covered indirectly by existing factories and await using tests; if implementation adds a holder or non-obvious lifecycle, add a focused test in NeonSprawl.Server.Tests that builds a Postgres factory, disposes it, and asserts clean teardown (optional follow-up).

Open questions / risks

Resolved: Double-dispose risk accepted under Option 2; Npgsql 10 + full test run showed no shutdown regressions. None outstanding for this story.

PR / review

Merge / PR description: Call out two themes so reviewers do not miss non-server files: (1) Server — NpgsqlDataSourceShutdownHostedService, explicit AddSingleton<NpgsqlDataSource>, and InMemoryWebApplicationFactory stripping; (2) Repo process — planning-implementation-docs and cross-links in AGENTS.md, story-kickoff, code-review, and docs-review agent rules.

Cross-check decomposition — server / persistence if referenced for lifecycle expectations; keep NEON-15 in commit subjects per jira-git-naming.