server/observability existed fully unit-tested but was imported by
nothing outside its own package -- no HTTP handler ever called it, so
its credential redaction protected zero real log output. Wire it into
Service via an optional Log field (nil-safe, so every existing Service
literal keeps compiling unchanged) and call it from the two
workload-authenticated server routes -- register and result -- at
every outcome: unauthorized, rejected, conflict and success. Wire
cmd/control-plane to actually emit those events as JSON lines on
stderr.
Add a secret canary test that drives both routes end to end with
realistic bearer-token and result-nonce values and asserts neither
literal secret appears anywhere in what Service.Log actually received
-- a stronger claim than the existing observability unit test, which
only proves redact() strips a synthetic value under a denylisted key
name. redact() is still key-name-based, not content-based: a future
call site that logs a secret under an unlisted key name would not be
caught by this test or by redact() itself, only by the same discipline
applied here of never putting raw request/token bytes into Fields.
Queue, proposal and assignment mutation routes are not wired yet.
Tasks 8.5, 8.14, 8.18 and 8.23 all claimed opt-in PostgreSQL execution
already covered their durable paths. It existed, but per this session's
run had apparently never actually been exercised clean: it surfaced
three real bugs (queue ticket insert param-count mismatch, an
unconditional proposal-participant timeout, a missing seasons row in
one integration test) that a passing unit-test suite could not have
caught, since the unit tests mock the driver. Record what was found,
fixed and re-verified live, and that the new migration rollback runner
is now in place.
ranked_season_rollovers.season_id has a foreign key into seasons, but
the integration test never inserted a seasons row for 'season-1' --
ApplyRankedSeasonRollover failed on the FK constraint before the
rollover logic itself ran at all. Insert a matching seasons row,
mirroring how a real 12-week season would already exist when
maintenance's rollover sweep runs. Verified against a real
PostgreSQL instance.
ProposalParticipantExpireSQL marked every PENDING participant on a
proposal TIMED_OUT unconditionally -- it took a proposal_id and 'now'
but never actually compared 'now' against the proposal's expires_at,
unlike its sibling ProposalExpireSQL (which does gate on
'expires_at <= $2'). Both GetProposal and RespondToProposal run this
statement on every call as a recovery step, so the very first
RespondToProposal for any proposal timed out every participant
(including the one about to respond) before checking their response,
then rejected the real accept/decline with ErrConflict. Add the same
expiry gate via an EXISTS against proposals.expires_at, matching
ProposalExpireSQL's own condition, and update the SQL-fragment test to
assert the gate is present. Verified end to end against a real
PostgreSQL instance: TestPostgreSQLProposalClaimAndResponseAreAtomic
now passes a two-participant accept/accept sequence that previously
failed on the first response.
CreateQueueTicket passed 9 arguments to QueueTicketInsertSQL, which
only has 8 placeholders (state is a hardcoded 'QUEUED' literal in the
SQL, not $4) -- every real queue-ticket creation against PostgreSQL
failed with 'mismatched param and argument count'. Found by actually
running the opt-in Postgres integration suite (previously never
exercised locally, per its own gating) rather than trusting the unit
tests, which mock the driver and can't catch a placeholder-count
mismatch. Verified fixed against a real postgres:17-alpine container.
Add migrations.Rollback(ctx, db, dir, steps): reverses the N most
recently applied migrations, newest first, each in its own committed
transaction under the same advisory lock Apply uses. Down SQL lives in
migrations/down/<version>.sql (a subdirectory, so Apply's *.sql glob
over the main directory is untouched); a missing down file for a
migration being rolled back is a hard error rather than a silent
partial reversal. Wire it into cmd/migrate as --rollback=N.
Add down files for all six existing migrations, each dropping objects
in FK-safe reverse dependency order.
Adversarial review: could not run the new integration test
(TestPostgreSQLMigrationsRollBackAndReapplyCleanly, gated behind
COSMIC_CLASH_POSTGRES_DSN / scripts/run_postgres_integration.sh)
against a real database in this sandbox - Docker Desktop's own
overlayfs ran out of space pulling postgres:17-alpine, unrelated to
this change. Verified instead by hand-tracing every DROP against its
forward migration's FK graph, confirming Apply's directory glob does
not pick up the down/ subdirectory, and a clean go build/vet/test
-tags integration. Worth an explicit real run before this is trusted
in CI.
Add POST /v1/servers/{id}/register (and its /api/v1 contract alias),
authenticated by the same workload binding as the result route. A
game server reports its protocol version and image digest and asks
to advance ALLOCATING -> PROCESS_READY -> ASSIGNMENT_READY; the store
boundary (AdvanceServerRegistration) does this as one idempotent
SERIALIZABLE transaction that also advances every participant's queue
ticket, and gates the final transition on every participant having a
live, unexpired assignment.
Adversarial review of the surrounding routing turned up a pre-existing
bug: contractServerMutation rejected any path containing '/', so the
already-documented /api/v1/servers/{id}/result route (and this new
/register route) 404'd for every real caller despite being declared
in the OpenAPI contract. Fix it to delegate shape validation to
serverMutation, matching how contractQueueMutation handles its own
two-segment paths, and add a regression test covering both contract
routes end to end.
multiplayer-todo.md and multiplayer-next.md tracked overlapping
information in two places. Fold everything into multiplayer-next.md
(architecture decisions, wire format, task breakdown with checkboxes,
gotchas list, testing notes) and delete multiplayer-todo.md. Section
numbers are unchanged, so existing code comments citing them by
section/task number still resolve; update every such reference to
point at the new filename.
Josh Creek