Skip to content

feat: durable event-log-driven games projection (P0) - #71

Merged
sayed710 merged 6 commits into
mainfrom
claude/durable-games-projection
Sep 26, 2026
Merged

sayed710 merged 6 commits into
mainfrom
claude/durable-games-projection

Conversation

@sayed710

Copy link
Copy Markdown
Owner

Closes the P0 durable games projection defect (audit, row "Game history / projection"). Ratings are out of scope; they are the follow-up that will consume this projection.

Root cause (verified on main @ 93cc29c)

  • Only seek acceptance (PgSeekAcceptor) and bot games (PgGameStarter) inserted a games row, and only at creation. Tournament games (DurableGameLauncher) and direct EventStore.append(…, -1, …) callers (GameAuthority.createGame) wrote events only.
  • GamesRepository.updateProgress and finish had no runtime callers, so rows stayed at ply 0 with no result, termination or end time.
  • As a result, GET /v1/games/:id, /v1/users/:handle/games, the ended-game seek receipt guard, search and achievements all read stale or missing state.

Design (ADR-0147)

  • Work discovery: migration 0040 adds game_events.xact_id xid8 DEFAULT pg_current_xact_id(), the id of the transaction that wrote each row. A batch reads rows after a durable checkpoint whose writing transaction is older than pg_snapshot_xmin(pg_current_snapshot()), i.e. has already finished. A late-committing transaction therefore can't be skipped (a server_ts cursor can skip it). The column default fills the value for every writer, so no append, creation or player-lock path changes.
  • Atomicity and replicas: one transaction per batch locks the checkpoint row FOR UPDATE SKIP LOCKED, projects, and advances the checkpoint in the same commit.
  • Idempotency and monotonicity: each touched game is re-folded from its full stream by the pure projectGameStream, which uses the canonical classifySpeed and event timestamps. The upsert runs only WHERE games.last_seq <= EXCLUDED.last_seq.
  • Corrupt streams: a per-game savepoint isolates the failure, which is recorded in games_projection_failures, logged, and retried with backoff. The stream is never silently passed over and never blocks other games.
  • Runtime: every gateway with DATABASE_URL runs a GamesProjectionWorker: bounded batches, a 1 s idle poll, exponential backoff up to 30 s, and an awaited in-flight batch on shutdown. Compose and Helm already set the variable, so there is no deploy change.
  • Backfill: rows written before 0040 carry the migration's transaction id, so the first pass replays them automatically. npm run games:rebuild re-folds every stream on demand. It is safe beside live projection and leaves games the live projector has yet to reach to that projector.
  • Consumers: the search indexer and achievements worker now wake on gamesProjectedEndedChannel(), which is published once when a projected row becomes terminal, instead of on the earlier broadcast that raced the projection.
  • Migrations: 0041 builds the checkpoint-order index online. The unused, non-monotonic start/updateProgress/finish writers are removed.

Evidence

Local runs against a dedicated pgvector/pgvector:pg16 server and a Redis 7 server:

Suite Result
build:server, lint (19 workspaces) pass
persistence unit 96/96, 0 skipped
persistence PostgreSQL integration 126/126, 0 skipped (15 new projector tests)
api unit 1101/1101, 0 skipped
api PostgreSQL integration 63/63, 0 skipped (new: tournament game → /v1/games/:id + history)
realtime-gateway / gateway (real Redis) 70/70 · 51/51
scripts unit 308/308
static guards (adr-claims, test-topology, ci-parity, observability, build-order, deploy-gates, variant/engine parity) all pass

The new projector tests cover: direct, seek, bot and non-account creation; progress and endings with each ending reported exactly once; idempotent replay and rebuild; no regression of a newer row; rebuild repair of missing and stale rows; automatic backfill of pre-0040 data, with the append-only guard intact after the column rewrite; checkpoint-lock exclusion and two concurrent projectors converging; crash before commit, then resume; the late-commit case; corrupt-stream isolation, backoff and recovery; checkpoint rewind; and a rebuild deferring a live ending.

Deliberate-defect probes against the compiled projector:

  • Caught by a specific test: removing the last_seq guard, removing the horizon filter, removing rewind detection, reporting endings regardless of prior state, removing failure backoff, letting a failure escape its savepoint, and letting a rebuild ignore live work.
  • Removing the checkpoint lock makes the lock-exclusion test hang until killed.

Real stack (Compose images built from this branch, gateway with SEARCH_INDEXER=1):

  • An event-only game ending in checkmate was projected and served by GET /v1/games/:id, and the search indexer indexed it from the projected-ending wake.
  • SIGTERM gave exit 0.
  • A game committed while the gateway was stopped was projected after restart.

Review: a read-only concurrency review found two issues, both fixed and covered by tests. The index had been built inside the rewrite's exclusive lock; it is now the online migration 0041. A rebuild that committed first could drop a live ending's wake; the rebuild now defers such games.

Limits (documented in ADR-0147)

  • The transaction horizon is cluster-wide: a long or idle-in-transaction session anywhere on the server delays projection, but no work is lost.
  • 0040 rewrites game_events once under an exclusive lock. That is acceptable pre-launch.
  • Each touched game is re-folded in full.
  • The projected-ending wake is lossy pub/sub, and achievements still need per-game deduplication.
  • Out of scope and untouched: ratings, rating pools, leaderboards, readiness, first-move clock start, no-show and seek lifecycle, and autonomous flag expiry.

Test plan

  • CI green on the final HEAD
  • Qodo, Greptile and CodeRabbit on the final HEAD
  • Owner review. Do not merge without owner decision.

The games table was documented as a rebuildable projection of game_events,
but nothing derived it: tournament and direct GameCreated appends had no row,
and progress and endings were never written, so every row stayed at ply 0
with no result.

- Migration 0040 records each event's writing transaction (xact_id xid8,
  filled by a column default, so no append path changes) and adds the
  projection checkpoint and failure tables; 0041 builds the checkpoint-order
  index online.
- PgGamesProjector consumes only events from transactions below
  pg_snapshot_xmin(pg_current_snapshot()), so a late commit is never skipped.
  One transaction per batch holds the checkpoint FOR UPDATE SKIP LOCKED,
  re-folds each touched game from its full stream, upserts guarded by
  last_seq, and advances the checkpoint atomically.
- A stream that cannot be folded is isolated by a savepoint, recorded in
  games_projection_failures, and retried with backoff.
- games:rebuild re-folds every stream, deferring games the live projector
  has yet to reach.
- The unused, non-monotonic start/updateProgress/finish writers are removed
  from GamesRepository.
Every gateway with DATABASE_URL (already set in Compose and Helm) runs a
GamesProjectionWorker: batches back to back while behind, a 1 s poll when
idle, exponential backoff to 30 s on failure, and an awaited in-flight batch
on shutdown before pub/sub and the pool close.

A committed terminal projection publishes an `ended` broadcast on the new
gamesProjectedEndedChannel(). The search indexer and achievements worker
subscribe there instead of to gamesEndedChannel(), which fires before the
row they read can have been projected.
ADR-0147 records the xid-horizon checkpoint, atomic batches, failure
handling, runtime placement, rejected alternatives and remaining limits.
DATABASE.md sections 3.2, 4.2 and 6 now describe the asynchronous projector
instead of append-time projection, RUNBOOKS.md gains the lag, failure and
rebuild procedure, and PROJECT_STATE.md appends increment 70.
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: da1c804c-db12-4dc1-8102-e4ea8daabc5e


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add durable event-log-driven games projection

🐞 Bug fix ✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Derive complete game summaries asynchronously without losing late-committing event transactions.
• Isolate corrupt streams, retry durably, and support safe full projection rebuilds.
• Wake projection-dependent consumers only after terminal game rows commit.
Diagram

graph TD
  A["Event Writers"] --> B[("Event Log")] --> C["Projection Worker"] --> D["Stream Fold"] --> E[("Games Projection")] --> F["Projected End Channel"] --> G["Search and Achievements"]
  C --> H[("Checkpoint and Failures")] --> C
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Project inside event appends
  • ➕ Provides immediately consistent game rows
  • ➕ Avoids a background checkpoint and worker
  • ➖ Projection defects could reject authoritative event writes
  • ➖ Extends transactions holding game and player locks
  • ➖ Requires every append path to maintain projection behavior
2. Maintain a dirty-games queue
  • ➕ Makes pending games explicit
  • ➕ Avoids globally scanning event order
  • ➖ Adds writes and row contention to every move
  • ➖ Requires all event writers to enqueue reliably
  • ➖ Contends with projectors on hot game rows
3. Poll stream heads
  • ➕ Uses existing game_events and games sequence data
  • ➕ Avoids transaction-ID cursor semantics
  • ➖ Repeatedly scans all games to discover changes
  • ➖ Scales poorly as historical game count grows
  • ➖ Still requires separate handling for missing projection rows

Recommendation: Keep the transaction-ID checkpointed projector. It protects authoritative appends, cannot skip late commits, coordinates replicas atomically, and provides durable corrupt-stream retries. Its cluster-wide horizon sensitivity and full-stream folding cost are acceptable at current scale and are explicitly documented with operational mitigations.

Files changed (21) +1415 / -45

Enhancement (8) +553 / -5
games-projection.tsImplement the pure game stream fold +87/-0

Implement the pure game stream fold

• Derives every log-backed game summary field from a complete event stream using canonical speed classification. Rejects malformed sequencing, duplicate creation, invalid moves, and post-ending events instead of producing partial rows.

packages/persistence/src/games-projection.ts

index.tsExport game projection APIs +1/-0

Export game projection APIs

• Exposes the pure projection fold and associated types from the persistence package.

packages/persistence/src/index.ts

games-projector.tsImplement the durable PostgreSQL projector +385/-0

Implement the durable PostgreSQL projector

• Adds transaction-horizon event discovery, atomic checkpoint locking, monotonic projection upserts, savepoint-isolated retries, safe rebuilds, and checkpoint rewind handling. Also adds the continuously scheduled worker with idle polling, exponential backoff, callbacks, and graceful shutdown.

packages/persistence/src/pg/games-projector.ts

games-rebuild-cli.tsAdd the projection rebuild CLI +31/-0

Add the projection rebuild CLI

• Re-folds every game stream using DATABASE_URL, reports projected, deferred, and failed streams, and exits unsuccessfully when corruption remains.

packages/persistence/src/pg/games-rebuild-cli.ts

index.tsExport PostgreSQL projector components +1/-0

Export PostgreSQL projector components

• Makes the projector, worker, batch types, and related APIs available through the PostgreSQL package entry point.

packages/persistence/src/pg/index.ts

index.tsExport the projected-ending channel +1/-1

Export the projected-ending channel

• Publishes the new post-projection channel helper through the realtime-gateway package API.

packages/realtime-gateway/src/index.ts

pubsub.tsDefine a post-projection ending channel +7/-0

Define a post-projection ending channel

• Adds a dedicated channel for terminal broadcasts emitted only after the corresponding games projection row commits.

packages/realtime-gateway/src/pubsub.ts

serve.tsRun the projector and notify dependent consumers +40/-4

Run the projector and notify dependent consumers

• Starts a projection worker on every database-backed gateway, logs retries and rewinds, and publishes committed endings. Search and achievements now consume post-projection notifications, while shutdown waits for any in-flight batch before closing dependencies.

services/gateway/src/serve.ts

Refactor (3) +6 / -35
fakes.tsAlign the in-memory repository with projector ownership +1/-5

Align the in-memory repository with projector ownership

• Removes the obsolete progress writer and identifies the remaining terminal helper as a test stand-in for projection behavior.

packages/api/src/fakes.ts

repositories.tsRemove obsolete direct projection writers +1/-27

Remove obsolete direct projection writers

• Deletes unused start, progress, and finish mutations so PostgreSQL game rows cannot be updated through non-monotonic repository methods. Exports canonical UUID validation for projector seat handling.

packages/persistence/src/pg/repositories.ts

repositories.tsConvert GamesRepository to a read-side contract +4/-3

Convert GamesRepository to a read-side contract

• Removes mutation methods and documents creation transactions plus the event-log projector as the only game-row writers.

packages/persistence/src/repositories.ts

Tests (3) +722 / -0
games-projection.integration.test.tsVerify projected games through public APIs +84/-0

Verify projected games through public APIs

• Proves a tournament-created game becomes readable after projection and that its terminal state appears consistently in game details and user history.

packages/api/test/games-projection.integration.test.ts

games-projection.test.tsTest stream folding and worker lifecycle +148/-0

Test stream folding and worker lifecycle

• Covers creation, moves, endings, speed classification, corrupt streams, continuous scheduling, exponential backoff, callback failures, and awaited shutdown.

packages/persistence/test/games-projection.test.ts

games-projector.integration.test.tsExercise projector correctness on PostgreSQL +490/-0

Exercise projector correctness on PostgreSQL

• Tests creation paths, progress, endings, replay, rebuilds, backfill, replica exclusion, atomic rollback, late commits, corrupt-stream recovery, monotonic writes, and foreign-cluster checkpoint rewind against a real database.

packages/persistence/test/games-projector.integration.test.ts

Documentation (4) +94 / -5
DATABASE.mdDocument asynchronous games projection semantics +29/-4

Document asynchronous games projection semantics

• Adds the event transaction ID to the schema documentation and explains checkpointed work discovery, atomic batches, monotonic writes, failure retries, and rebuild behavior. Corrects the consistency section to describe asynchronous projection and deferred ratings.

docs/DATABASE.md

PROJECT_STATE.mdRecord durable games projection milestone +14/-1

Record durable games projection milestone

• Adds Increment 70 with the defect analysis, architecture, runtime behavior, test coverage, limitations, and out-of-scope follow-ups.

docs/PROJECT_STATE.md

RUNBOOKS.mdAdd games projection operations runbook +12/-0

Add games projection operations runbook

• Documents lag diagnosis, failed-stream inspection, automatic retries, and Compose or Kubernetes rebuild procedures.

docs/RUNBOOKS.md

0147-durable-games-projection.mdDefine the durable projection architecture +39/-0

Define the durable projection architecture

• Records the transaction-ID cursor, atomic checkpointing, full-stream folds, retry model, runtime worker, consumer notifications, backfill strategy, rejected alternatives, and known limits.

docs/adr/0147-durable-games-projection.md

Other (3) +40 / -0
0040_games_projection.sqlAdd projection checkpoint and failure schema +36/-0

Add projection checkpoint and failure schema

• Adds transaction IDs to game events, initializes the games projection checkpoint, and creates durable corrupt-stream retry records. Existing events receive the migration transaction ID for automatic backfill.

packages/persistence/migrations/0040_games_projection.sql

0041_games_projection_scan_index.sqlIndex transaction-ordered event scans +3/-0

Index transaction-ordered event scans

• Builds the event-log scan index concurrently over transaction ID, game ID, and sequence.

packages/persistence/migrations/0041_games_projection_scan_index.sql

package.jsonExpose the games rebuild command +1/-0

Expose the games rebuild command

• Adds the games:rebuild script for invoking the compiled projection rebuild CLI.

packages/persistence/package.json

@qodo-code-review

qodo-code-review Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Shutdown interrupts postgame tasks ✓ Resolved 🐞 Bug ☼ Reliability ⭐ New
Description
shutdown calls stop() on the search and achievement workers, but those methods only unsubscribe
and do not await each worker's tracked asynchronous tasks. If the final projection batch starts
indexing or awarding, shutdown can close the database and exit while that work is running,
permanently leaving the projected game unprocessed because its ending is not published again.
Code

services/gateway/src/serve.ts[R767-768]

+        searchIndexWorker?.stop();
+        achievementsAwardWorker?.stop();
Relevance

●●● Strong

Shutdown ordering is explicitly reviewer-sensitive; workers must stop only after projection drain
and before resource closure.

PR-#65

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Both consumer classes explicitly track in-flight promises and expose drain(), whereas stop()
only unsubscribes and clears local state. The shutdown path invokes only stop(), then closes
shared resources and calls process.exit(0), so tasks started synchronously by the final projection
publication are not included in the graceful-shutdown wait.

packages/api/src/search/index-worker.ts[128-149]
packages/api/src/achievements/award-worker.ts[131-150]
services/gateway/src/serve.ts[746-778]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The final projection wake can start asynchronous indexing or achievement work immediately before shutdown stops the consumers. Their `stop()` methods do not await that work, so closing the database and exiting can interrupt it.

## Fix Focus Areas
- services/gateway/src/serve.ts[746-768]
- packages/api/src/search/index-worker.ts[128-149]
- packages/api/src/achievements/award-worker.ts[131-150]

## Recommended Fix
After awaiting the projection worker, stop both consumer subscriptions and then await both workers' `drain()` methods before closing pub/sub or the database. This prevents new tasks while allowing every task triggered by the final committed batch to finish.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Long transactions suppress ending wakes ✗ Dismissed 🐞 Bug ≡ Correctness ⭐ New
Description
gamesAwaitingLiveBatch treats cursor.xact_id >= snapshot.xmin as evidence that the checkpoint
came from another cluster, although an unrelated long-running transaction can hold snapshot.xmin
below a valid local checkpoint. During a concurrent rebuild this disables deferral for pending local
endings, so the rebuild writes the terminal row without publishing and the live projector later
observes no terminal transition for search or achievements.
Code

packages/persistence/src/pg/games-projector.ts[270]

+  if (BigInt(cursor.xact_id) >= BigInt(snapshot.xmin)) return new Set();
Relevance

●●● Strong

Specific correctness risk: long transactions can falsely suppress rebuild deferral and lose terminal
wake publication.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rebuild reads xmin and the checkpoint, then returns an empty pending set solely from their
numeric ordering; otherwise it has a query that can identify visible events after the checkpoint.
projectOne only reports an ending when it changes the row from nonterminal to terminal, while the
rebuild CLI does not publish returned endings, so a rebuild that bypasses deferral consumes the only
transition that would wake consumers.

packages/persistence/src/pg/games-projector.ts[263-277]
packages/persistence/src/pg/games-projector.ts[195-208]
packages/persistence/src/pg/games-rebuild-cli.ts[11-21]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The rebuild mistakes a checkpoint at or above the snapshot minimum for a foreign-cluster checkpoint, even though a normal long-running transaction can produce that ordering. This lets rebuilds consume endings that should remain for the live projector and its postgame notifications.

## Fix Focus Areas
- packages/persistence/src/pg/games-projector.ts[256-277]
- packages/persistence/test/games-projector.integration.test.ts[312-363]

## Recommended Fix
Remove the `cursor.xact_id >= snapshot.xmin` early return and determine pending work from the visible events after the checkpoint. A restored checkpoint whose transaction ID is above the new cluster's events naturally yields no matching rows, while a same-cluster checkpoint held above `xmin` still defers its visible pending games; add a test with an older open transaction, a newer checkpoint, and a pending ending.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Rebuilds can swallow live endings ✓ Resolved 🐞 Bug ≡ Correctness
Description
rebuildAll checks gamesAwaitingLiveBatch and then loads each stream in later statements under
the default read-committed isolation, allowing the stream load to observe events that the deferral
check did not see. If an ending commits between those statements, the rebuild makes the row terminal
without publishing it, and the live projector later sees an already-terminal row so search and
achievement consumers receive no wake.
Code

packages/persistence/src/pg/games-projector.ts[R160-161]

+        const live = await gamesAwaitingLiveBatch(client, ids);
+        const result = await this.projectAll(client, ids.filter((id) => !live.has(id)));
Relevance

●●● Strong

Accepted-style concurrency race: separate read-committed statements can absorb a live ending without
emitting its wake.

PR-#62

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rebuild first classifies pending games and then calls projectAll, while inTransaction starts
an ordinary transaction without repeatable-read isolation. gamesAwaitingLiveBatch performs its
pending query separately from loadStream, so PostgreSQL read-committed semantics permit the latter
statement to observe a concurrently committed ending; the rebuild returns endings but does not
broadcast them, while projectOne only reports an ending when the existing row was not already
terminal.

packages/persistence/src/pg/games-projector.ts[159-167]
packages/persistence/src/pg/games-projector.ts[196-205]
packages/persistence/src/pg/games-projector.ts[224-242]
packages/persistence/src/pg/games-projector.ts[251-274]
packages/persistence/src/pg/games-rebuild-cli.ts[11-21]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A rebuild page checks whether games have pending live-projector work and then loads their streams using later read-committed snapshots. An ending committed between those operations can be projected by the rebuild without a notification and subsequently skipped for notification by the live projector.

## Fix Focus Areas
- packages/persistence/src/pg/games-projector.ts[159-163]
- packages/persistence/src/pg/games-projector.ts[224-242]
- packages/persistence/src/pg/games-projector.ts[251-274]

## Recommended Fix
Run each rebuild page's pending-work check and stream folds against one repeatable-read snapshot, while retaining the checkpoint share lock through commit. Ensure events committed after that snapshot remain invisible to the rebuild so the live projector processes them and emits the terminal notification; add an integration test that pauses after the deferral query, commits an ending, resumes the rebuild, and verifies the ending is deferred and reported by the live batch.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View high (2)
4. Finished games miss postgame work ✓ Resolved 🐞 Bug ☼ Reliability
Description
GamesProjectionWorker.start() is called before the search and achievements workers subscribe to
gamesProjectedEndedChannel(), even though its first batch is scheduled immediately. A pending
terminal stream can therefore be projected and broadcast before either subscriber exists, and
neither worker has a startup reconciliation path to index or award that game later.
Code

services/gateway/src/serve.ts[285]

+    worker.start();
Relevance

●●● Strong

Immediate startup scheduling can publish before subscribers; accepted history supports fixing
missed-broadcast lifecycle races.

PR-#65

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new worker begins scheduling before the later consumer setup. Its first tick is scheduled with a
zero delay, whereas both downstream workers only process messages received through their
subscriptions and do not perform an initial scan of completed games.

services/gateway/src/serve.ts[259-288]
services/gateway/src/serve.ts[405-473]
packages/persistence/src/pg/games-projector.ts[332-375]
packages/api/src/search/index-worker.ts[61-129]
packages/api/src/achievements/award-worker.ts[45-111]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The games projector can publish terminal-game notifications before the search indexer and achievements worker have subscribed. Because this pub/sub channel is lossy and these consumers do not reconcile previously projected terminal games during startup, affected games remain unindexed and do not receive achievement processing.

## Fix Focus Areas
- services/gateway/src/serve.ts[259-288]
- services/gateway/src/serve.ts[405-473]

## Recommended Fix
Construct and start the search and achievements subscribers before starting `GamesProjectionWorker`. Move the projection worker's `start()` call until after optional post-projection consumers have been initialized, while preserving the existing database and feature-flag guards.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Shutdown drops finished-game processing ✓ Resolved 🐞 Bug ☼ Reliability
Description
Shutdown unsubscribes the search and achievements workers before awaiting the new projector's
in-flight batch through gamesProjection.stop(). If that batch commits a terminal row, its
onBatch callback publishes after both consumers have stopped, so the notification is lost
permanently because replay does not republish already-terminal rows.
Code

services/gateway/src/serve.ts[R747-748]

+    // No new projection batch starts from here; an in-flight one is awaited before its pool and pub/sub close.
+    const projectionStopped = gamesProjection?.stop();
Relevance

●●● Strong

Consumers stop before the awaited producer, allowing an in-flight terminal notification to be lost
permanently.

PR-#65

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The worker's stop() explicitly waits for the active tick, and that tick invokes onBatch before
it resolves. The shutdown sequence removes post-projection subscribers first, so an ending emitted
by that active tick has no local consumer; a later replay does not emit it again because the row is
already terminal.

services/gateway/src/serve.ts[259-288]
services/gateway/src/serve.ts[737-769]
packages/persistence/src/pg/games-projector.ts[338-375]
packages/persistence/src/pg/games-projector.ts[191-205]
packages/api/src/search/index-worker.ts[120-127]
packages/api/src/achievements/award-worker.ts[122-129]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Gateway shutdown stops the search and achievement subscribers before waiting for an in-flight games projection batch. That batch may commit a newly terminal game and invoke its publish callback after the subscribers have been removed, losing the only notification for postgame indexing and achievement work.

## Fix Focus Areas
- services/gateway/src/serve.ts[737-766]
- packages/persistence/src/pg/games-projector.ts[338-375]

## Recommended Fix
Stop and await the games projection worker before stopping the search and achievements workers, so any terminal notifications from its in-flight batch are delivered while those consumers remain subscribed. Keep database and pub/sub shutdown after the projection worker has completed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: The push changes rebuild transaction and serialization-conflict behavior in concurrency-sensitive persistence code, creating real correctness risk despite being localized.

Grey Divider

Tip of the day
💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 69e3845

Results up to commit 1679949 🧠 Deep


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Rebuilds can swallow live endings ✓ Resolved 🐞 Bug ≡ Correctness
Description
rebuildAll checks gamesAwaitingLiveBatch and then loads each stream in later statements under
the default read-committed isolation, allowing the stream load to observe events that the deferral
check did not see. If an ending commits between those statements, the rebuild makes the row terminal
without publishing it, and the live projector later sees an already-terminal row so search and
achievement consumers receive no wake.
Code

packages/persistence/src/pg/games-projector.ts[R160-161]

+        const live = await gamesAwaitingLiveBatch(client, ids);
+        const result = await this.projectAll(client, ids.filter((id) => !live.has(id)));
Relevance

●●● Strong

Accepted-style concurrency race: separate read-committed statements can absorb a live ending without
emitting its wake.

PR-#62

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rebuild first classifies pending games and then calls projectAll, while inTransaction starts
an ordinary transaction without repeatable-read isolation. gamesAwaitingLiveBatch performs its
pending query separately from loadStream, so PostgreSQL read-committed semantics permit the latter
statement to observe a concurrently committed ending; the rebuild returns endings but does not
broadcast them, while projectOne only reports an ending when the existing row was not already
terminal.

packages/persistence/src/pg/games-projector.ts[159-167]
packages/persistence/src/pg/games-projector.ts[196-205]
packages/persistence/src/pg/games-projector.ts[224-242]
packages/persistence/src/pg/games-projector.ts[251-274]
packages/persistence/src/pg/games-rebuild-cli.ts[11-21]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A rebuild page checks whether games have pending live-projector work and then loads their streams using later read-committed snapshots. An ending committed between those operations can be projected by the rebuild without a notification and subsequently skipped for notification by the live projector.

## Fix Focus Areas
- packages/persistence/src/pg/games-projector.ts[159-163]
- packages/persistence/src/pg/games-projector.ts[224-242]
- packages/persistence/src/pg/games-projector.ts[251-274]

## Recommended Fix
Run each rebuild page's pending-work check and stream folds against one repeatable-read snapshot, while retaining the checkpoint share lock through commit. Ensure events committed after that snapshot remain invisible to the rebuild so the live projector processes them and emits the terminal notification; add an integration test that pauses after the deferral query, commits an ending, resumes the rebuild, and verifies the ending is deferred and reported by the live batch.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Finished games miss postgame work ✓ Resolved 🐞 Bug ☼ Reliability
Description
GamesProjectionWorker.start() is called before the search and achievements workers subscribe to
gamesProjectedEndedChannel(), even though its first batch is scheduled immediately. A pending
terminal stream can therefore be projected and broadcast before either subscriber exists, and
neither worker has a startup reconciliation path to index or award that game later.
Code

services/gateway/src/serve.ts[285]

+    worker.start();
Relevance

●●● Strong

Immediate startup scheduling can publish before subscribers; accepted history supports fixing
missed-broadcast lifecycle races.

PR-#65

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new worker begins scheduling before the later consumer setup. Its first tick is scheduled with a
zero delay, whereas both downstream workers only process messages received through their
subscriptions and do not perform an initial scan of completed games.

services/gateway/src/serve.ts[259-288]
services/gateway/src/serve.ts[405-473]
packages/persistence/src/pg/games-projector.ts[332-375]
packages/api/src/search/index-worker.ts[61-129]
packages/api/src/achievements/award-worker.ts[45-111]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The games projector can publish terminal-game notifications before the search indexer and achievements worker have subscribed. Because this pub/sub channel is lossy and these consumers do not reconcile previously projected terminal games during startup, affected games remain unindexed and do not receive achievement processing.

## Fix Focus Areas
- services/gateway/src/serve.ts[259-288]
- services/gateway/src/serve.ts[405-473]

## Recommended Fix
Construct and start the search and achievements subscribers before starting `GamesProjectionWorker`. Move the projection worker's `start()` call until after optional post-projection consumers have been initialized, while preserving the existing database and feature-flag guards.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Shutdown drops finished-game processing ✓ Resolved 🐞 Bug ☼ Reliability
Description
Shutdown unsubscribes the search and achievements workers before awaiting the new projector's
in-flight batch through gamesProjection.stop(). If that batch commits a terminal row, its
onBatch callback publishes after both consumers have stopped, so the notification is lost
permanently because replay does not republish already-terminal rows.
Code

services/gateway/src/serve.ts[R747-748]

+    // No new projection batch starts from here; an in-flight one is awaited before its pool and pub/sub close.
+    const projectionStopped = gamesProjection?.stop();
Relevance

●●● Strong

Consumers stop before the awaited producer, allowing an in-flight terminal notification to be lost
permanently.

PR-#65

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The worker's stop() explicitly waits for the active tick, and that tick invokes onBatch before
it resolves. The shutdown sequence removes post-projection subscribers first, so an ending emitted
by that active tick has no local consumer; a later replay does not emit it again because the row is
already terminal.

services/gateway/src/serve.ts[259-288]
services/gateway/src/serve.ts[737-769]
packages/persistence/src/pg/games-projector.ts[338-375]
packages/persistence/src/pg/games-projector.ts[191-205]
packages/api/src/search/index-worker.ts[120-127]
packages/api/src/achievements/award-worker.ts[122-129]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Gateway shutdown stops the search and achievement subscribers before waiting for an in-flight games projection batch. That batch may commit a newly terminal game and invoke its publish callback after the subscribers have been removed, losing the only notification for postgame indexing and achievement work.

## Fix Focus Areas
- services/gateway/src/serve.ts[737-766]
- packages/persistence/src/pg/games-projector.ts[338-375]

## Recommended Fix
Stop and await the games projection worker before stopping the search and achievements workers, so any terminal notifications from its in-flight batch are delivered while those consumers remain subscribed. Keep database and pub/sub shutdown after the projection worker has completed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread packages/persistence/src/pg/games-projector.ts Outdated
Comment thread services/gateway/src/serve.ts Outdated
Comment thread services/gateway/src/serve.ts Outdated
@greptile-apps

greptile-apps Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Critical risk] Rebuilds how game state is stored and kept current.

The PR appears safe to merge from this review; no outstanding finding was identified.

Summary

The PR adds a checkpointed event-log projector for games, a rebuild command, and post-projection ending wakes for search and achievements.

  • The change since the previous review retries a persistently conflicting rebuild page one game at a time, allowing healthy games on that page to be repaired.
  • No new actionable issue was identified. The previous findings are resolved.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  E[Committed game events] --> P[Checkpointed projector]
  P --> G[Games projection]
  P --> W[Projected-ending wake]
  W --> S[Search and achievements]
  R[Rebuild command] --> G
Loading

Reviews (4) · Last reviewed commit: "fix(persistence): redo a conflicting reb..."

Comment thread packages/persistence/src/pg/games-projector.ts Outdated
Comment thread services/gateway/src/serve.ts Outdated
Comment thread packages/persistence/src/pg/games-projector.ts Outdated
Comment thread packages/persistence/src/pg/games-projector.ts
…wake ordering

- Rebuild pages run under REPEATABLE READ, so the deferral decision and the
  folds it guards read one snapshot; an ending committed mid-page reaches
  the live batch that reports it. Pages that collide with live writes retry.
- After a logical restore (checkpoint above this cluster's horizon) the
  rebuild defers nothing and repairs every game itself.
- Only stream-data failures (projection errors, SQLSTATE 22/23) are recorded
  per game; transient database errors abort the batch without moving the
  checkpoint past a healthy game.
- The gateway starts projection after the search and achievements workers
  subscribe, and unsubscribes them only after the in-flight batch publishes.

Raised by Qodo and Greptile on PR #71.
@sayed710

Copy link
Copy Markdown
Owner Author

/review

@sayed710

Copy link
Copy Markdown
Owner Author

@greptileai review

Comment thread packages/persistence/src/pg/games-projector.ts
Comment thread services/gateway/src/serve.ts
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 3376b81

Comment thread packages/persistence/src/pg/games-projector.ts Outdated
…cting pages

- Gateway shutdown now drains the search indexer's and achievements
  worker's in-flight tasks after the last projection batch and before the
  pool closes, so a final wake's indexing or awarding is not cut off.
- A rebuild page that keeps hitting serialization conflicts with live
  projection retries with backoff, then reports its games as failed and
  continues with later pages instead of aborting the whole command.

Raised by Qodo and Greptile on PR #71.
@sayed710

Copy link
Copy Markdown
Owner Author

/review

@sayed710

Copy link
Copy Markdown
Owner Author

@greptileai review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 340af57

Comment thread packages/persistence/src/pg/games-projector.ts Outdated
A rebuild page that kept hitting serialization conflicts reported every game
on it as failed, so up to 199 healthy games could stay unrepaired, and a
rerun grouped them with the same conflicting game again. Such a page is now
redone with one REPEATABLE READ transaction per game, so only a game that
itself keeps conflicting is reported.

Raised by Greptile on PR #71.
@sayed710

Copy link
Copy Markdown
Owner Author

/review

@sayed710

Copy link
Copy Markdown
Owner Author

@greptileai review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 69e3845

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant