golf_hub: PendingGameLifecycleWritesThrough saw 2 game rows after abandoning pending (pg_hub_e2e_test)

[aaylward]

Summary

PgGolfHubFixture.PendingGameLifecycleWritesThrough in //domains/games/apis/golf_hub:pg_hub_e2e_test failed in CI: after bob leaves the pending lobby game, Rows() still reports 2 durable game rows instead of the expected 1 (the finished handoff that must remain).

This looks unrelated to the PR that surfaced it (#1430 / iili). That change only forced a full rebuild via a bazel/smithy.MODULE.bazel comment edit; golf_hub was not modified.

Failure

domains/games/apis/golf_hub/pg_hub_e2e_test.cc:655: Failure
Expected equality of these values:
  rows.games.size()
    Which is: 2
  1u
    Which is: 1

[  FAILED  ] PgGolfHubFixture.PendingGameLifecycleWritesThrough

CI: https://github.com/muchq/MoonBase/actions/runs/32526918007/job/96910813250?pr=1430

The assertion is the second half of the test: create a started game → bob leaves (game ends, terminal row kept) → bob creates a pending game → assert 2 rows → bob leaves pending → assert 1 row (pending deleted, terminal remains).

gameLeft was received before the failing Rows() call, so the leave path acknowledged the seat but the pending row was still in Postgres after Flush().

Why this is easy to miss locally

Without GOLF_HUB_TEST_DB_URL, the fixture skips:

GTEST_SKIP() << "GOLF_HUB_TEST_DB_URL unset";

Bazel still reports PASSED. A local green with the env unset has not exercised this suite at all (same trap as CLAUDE.md / Postgres-gated tests).

Evidence so far

• Same test passed when actually executed earlier the same day (e.g. Branch Build run 32496635534, non-cached).
• Other recent greens were often (cached) PASSED, so cold runs are rarer than the dashboard suggests.
• Local repro against Postgres 17 with the env set: 0 failures in 30 --cache_test_results=no runs.
• CI uses postgres:18 (GOLF_HUB_TEST_DB_URL / PG_TEST_DB_URL → shared moonbase_test).

So: intermittent or environment-sensitive, not a deterministic break from #1430.

Likely areas to inspect

1. HubHandler::LeaveGameLocked pending-empty path — should StageLocked(DeleteGame) when !started() && roster.empty(). Confirm CI failures always take this path vs the early “already out of roster” return (which still sends gameLeft but does not delete) or the started/finish commit path (which keeps a durable row).
2. Async writer vs Rows() — DeleteGame is queued; Rows() calls Flush() then LoadSnapshot(). Look for a race that re-inserts or skips the delete (listener catch-up, self-notify filtering, etc.).
3. Silent ExecOrWarn("DeleteGame", ...) failure — if DELETE fails, only a WARNING is logged; tighten the test or the store to fail loudly.
4. Shared scratch DB — exclusive-tagged, but confirm no cross-suite residue on games under full-graph CI load.

Repro

export GOLF_HUB_TEST_DB_URL='postgresql://moonbase_test:moonbase_test@127.0.0.1:5432/moonbase_test'
bazel test //domains/games/apis/golf_hub:pg_hub_e2e_test \
  --cache_test_results=no \
  --test_env=GOLF_HUB_TEST_DB_URL \
  --test_filter=PendingGameLifecycleWritesThrough \
  --test_output=errors

Stress with a loop or --runs_per_test=N if chasing flake rate. Prefer matching CI’s Postgres 18 if the PG 17 local pass keeps holding.

Acceptance

• Root cause identified (product bug vs test race vs infra).
• Either a deterministic regression test that fails without the fix, or (if pure flake) a hardening change that makes the failure mode loud and the leave/delete contract unambiguous.
• Document if anything about local skip / cache made this harder to see.