Each ten-second queue heartbeat mints a fresh idempotency key and
permanently inserts a row. Published outbox rows and expired/revoked
sessions were never purged either -- the maintenance role performed
lifecycle reconciliation only. At 10,000 queued players heartbeats alone
add roughly 60,000 durable rows per minute, so table and index growth,
vacuum pressure, backup size and recovery time were all unbounded on a
service intended to scale horizontally.
Add retention windows chosen to exceed every retry and recovery horizon
that could still consult the row -- deleting an idempotency key early
would turn a client replay into a second real mutation, so this is a
correctness bound, not just a housekeeping one. Dead-lettered outbox
rows are kept longest, being the record of events never delivered.
Deletes run in bounded SKIP LOCKED batches so a purge never blocks live
traffic, never holds a long transaction, and concurrent maintenance
replicas do not contend. Indexes back each predicate so a pass cannot
degrade into a sequential scan of the table it is bounding. The
maintenance role reports rows purged, the backlog past its window
(deletion lag), and any dead-lettered events.
Also make the migration-rollback test derive its step counts instead of
hardcoding them: adding a migration silently shifted the fixed counts so
the failure surfaced as an unrelated "0006 rollback did not drop
matches.allocation_id".
The Deployment runs two replicas, but WebSocket subscribers live only in
each process's in-memory hub. Every replica races to read the same
global unpublished outbox rows, and publishing succeeded even when the
winning replica held no matching local subscriber -- that replica then
set the single global published_at. A client connected to the other
replica never received the event, and delivery degraded further with
each replica added. REST recovery eventually converged, but short-lived
proposal transitions could be observed late or not at all.
Publish committed events through PostgreSQL LISTEN/NOTIFY so the replica
that owns the subscriber's connection delivers it, regardless of which
replica drained the row. The listener holds its own pgx connection --
LISTEN is session state, so a pooled database/sql connection cannot
carry it -- and reconnects with backoff, since losing it would silently
downgrade that replica's subscribers to REST-only recovery.
The fan-out is optional: without EventFanout configured, behaviour is
unchanged local-hub publication, which stays correct for a single
replica and for tests. Only outbox-sourced events are routed through it;
the in-request-path publishes remain local, as those are a latency
optimisation for the caller's own connection.
Fan-out needs a wire shape of its own because ControlPlaneEvent hides
PlayerID from clients, and the recipient is exactly what a peer replica
needs to route on.
banned_until and ban_reason have been in the schema since 0001, but no
production query ever read them -- grepping the tree found no reference
outside the migration itself. The only ban check was an in-memory map on
domain.TicketVerifier used by domain tests. Once real Steam login is
wired, a banned identity would keep full access through every existing
session until expiry and could obtain new ones.
Make the ban part of the durable authentication transaction rather than
a policy each login adapter must remember to re-implement:
- Session issuance inserts only when the identity exists and has no
active ban, so a banned player cannot mint a session.
- Authentication joins the identity and rejects an active ban on every
request, so a ban takes effect immediately on every replica rather
than at session expiry.
- ApplyIdentityBan sets the ban and revokes that identity's sessions in
one serializable transaction, closing the window where the ban is
durable but another replica still accepts an issued session.
Bans are time-bounded and clearing one does not resurrect sessions the
ban revoked.
Tests cover enforcement across two independently constructed stores
standing in for two replicas, expiry/unban semantics, and -- separately,
because revocation would otherwise mask it -- that a ban applied without
revoking anything still blocks the next request.
Both playlists shared one hash and sorted set, causing two independent
failures.
Starvation: Snapshot performed an unbounded ZRANGEBYSCORE and HMGET,
decoded the whole queue, and the matcher then truncated to its candidate
limit *before* filtering by playlist. A large casual prefix could
therefore leave the ranked worker with zero candidates indefinitely even
while ranked tickets were queued further down the set.
Mutual erasure: each matcher captured only its own playlist as the
durable source, but Rebuild replaced the shared keys, so a casual repair
wiped ranked projections and vice versa.
Namespace the keys per playlist, push the limit into Redis (LIMIT 0 N)
so reads no longer scale with total queue depth, and scope Rebuild to
one namespace. Rebuild now rejects a candidate whose playlist does not
match the namespace, which would reintroduce the starvation. Upsert
derives the namespace from the candidate; Remove takes the playlist,
since a ticket ID alone no longer identifies its namespace.
Add tests for a 300-deep casual backlog not starving ranked, for neither
playlist's rebuild erasing the other, and for the limit being applied
without losing enqueue ordering.
The candidate projection selected only from queue_tickets and its scan
never set Candidate.Rating, so every PostgreSQL-sourced ranked candidate
arrived with Go's zero value. Rating tolerance, selection scoring and
team partitioning all read that field, so ranked matchmaking treated a
900-rated player as identical to a 2100-rated one. Unit tests missed it
because they construct candidates with ratings already populated.
Join the ratings table, defaulting to domain.GlickoInitialRating for a
player with no ratings row yet -- a genuinely new profile, matching the
column default.
Fix the same defect on the Redis path too, which is reached differently:
the projection is seeded from the candidate CreateQueueTicket builds,
not from the candidate query, and that candidate also left Rating unset.
Resolve the rating inside the enqueue transaction so both projections
agree on one authoritative value. The rating is never client-supplied.
Add a store-backed test with deliberately distant ratings (900 vs 2100)
plus an unrated player, asserting both projections and that the spread
survives. Verified it fails without the fix.
ApplyInitialConnectPlan wrote a payload of {match_id,state,action},
omitting event, revision, resource_id, occurred_at and player_ids --
every field deliverStateOutboxEvent requires. Delivery rejected the row,
dispatch returned on the first error so it was never acknowledged, and
because reads are ordered oldest-first it was retried ahead of every
later state_changed event on every 100ms poll. One initial-connect
transition therefore blocked lifecycle delivery for all matches, not
just its own.
Two independent fixes, since either alone leaves the system fragile:
Build envelopes through one validating helper (MarshalOutboxEnvelope)
and convert all five writers to it. A writer that omits a required
field now fails its own transaction instead of committing a row that
can only ever poison the queue. The helper takes revision as int64 so
the -1 "nothing matched" sentinel some CTEs return surfaces as an error
rather than wrapping to a huge uint64.
Make dispatch resilient regardless: a delivery failure is now counted
against that row and the batch continues, with the row dead-lettered
after MaxOutboxDeliveryAttempts so a poison event degrades to one lost
notification instead of a stalled queue. Ordering within an aggregate
is still honoured -- later events of a failed match are deferred, so no
client observes that match's newer state before its older state. An ack
failure still stops the batch, being a database rather than a payload
problem.
Initial-connect events now address every participant, not just the
connected ones: a no-show needs to learn their ticket was failed and a
penalty applied.
test_contracts.py required operation ID `recordPlayerConnected`, but
openapi.json names that endpoint `claimPlayerConnection` — the accurate
name, since POST /servers/{id}/connect claims a connection lease and
returns a generation. Align the test on the document and assert the set
difference, so a future mismatch names the missing operation instead of
reporting "False is not true".
test_observability_manifests.py copied only two of the four files the
checker reads, so it died on a missing kustomization.yaml before ever
reaching the mutated namespace. Copy the full fixture, split the
namespace and scrape-path mutations into separate cases so either
defect produces its own diagnostic, and add an unmutated-copy case so a
broken fixture can't make the mutation cases pass vacuously.
Neither suite was invoked by any Make target or workflow, which is why
both could sit red. Add them, plus test_threat_model.py, to
verify_multiplayer_local.sh.
File the outstanding work that needs a person — hardware, an external
account, a playtest, a design decision — as GitHub issues (#14-#29),
labelled needs:human plus a P0-blocker..P3-low priority, an area and a
phase. Link each one inline from the doc bullet it came from.
The split: issues carry status for human-gated work, these documents
keep the detail, and the numbered agent-actionable tasks in
multiplayer-next.md §7 deliberately get no issues. Mark the 8.48 Compose
fixture bullet as agent-actionable so its lack of an issue does not read
as an omission.
multiplayer-next.md was a 1662-line mix of standing architecture spec
and task-completion tracking, most of which was dense per-task DONE
evidence for finished Phases 0-6. Split it:
- MULTIPLAYER_SPEC.md (new): the locked architecture decisions, wire
format, server-side input handling, prediction/reconciliation,
latency/frame-rate budget, and match lifecycle state machine -
standing design reference, not task-tracked.
- multiplayer-next.md (trimmed 1662 -> ~370 lines): only outstanding
work remains - §0 status, §7 Phase 7/8 task tables condensed to
"what's left" per task, §8-11 reference material (refactoring notes,
gotchas, testing, flagged items). Phases 0-6 collapsed to a pointer
at git history instead of ~500 lines of DONE evidence.
Also:
- Repointed every `multiplayer-next.md §N` code comment (N 1-6) across
Game/scripts, Game/tools and Game/tests to MULTIPLAYER_SPEC.md, since
those sections moved. Task-number references (`task N.N`, §7-11)
correctly still point at multiplayer-next.md.
- Updated CLAUDE.md's doc index and docs/TECH_STACK.md's spec-section
citations to match.
- TODO.md: added a "what's left to actually finish multiplayer
(human-actionable)" checklist pulled from multiplayer-next.md §0 and
docs/MATCHMAKING.md - things that need a person (hardware, a design
decision, a Steam App ID, hands on a controller), not more agent code.
Add a Go-vs-C#/Rust/C++ rationale for the matchmaking control plane to
TECH_STACK.md, and point at it from MATCHMAKING.md and README.md.
Also correct CLAUDE.md and README.md, which still described the backend
as unstarted/not built even though server/ has ~13k lines of Go across
matcher, allocator, api, store, security, supervisor and agones.
Found while investigating why the connect-wiring fix (§8.41,
ControlPlaneClient._connect_when_assigned) would still not work
end to end in a real deployment: nothing in production ever
publishes a player's signed match assignment.
store.SaveAssignment/SaveAssignments/SaveVerifiedAssignmentRoster --
the only functions that ever write the assignments table -- are
called only from tests, never from allocator/worker.go, cmd/allocator,
or anywhere else in the real service. allocator.Service.PublishRoster
(wired to store.PostgresRosterStore) is likewise never called from
production code.
allocation_match_sql.go's AdvanceServerRegistration SQL requires an
assignments row for every match participant before allowing the
ASSIGNMENT_READY transition. With nothing ever creating those rows, a
real match cannot advance past PROCESS_READY -- no player can ever
receive a real assignment or connect, regardless of how correct the
client-side connect-wiring fix from earlier this session is.
TestRealSupervisorRegistersAllocatedServerThroughControlPlane -- the
test that was supposed to prove this end to end -- manually seeds
store.SaveAssignment in its own setup rather than exercising the real
production write path, which is why this was never caught.
Per the user's explicit direction, this is flagged rather than fixed:
closing it needs new security-relevant design (a join-signing key
shared between the allocator, which would sign, and the game server,
which fleet.yaml already mounts a verification key for via
--join-authorisations-key-file but which no control-plane binary has
a matching signing flag for; roster-digest computation; per-player
domain.JoinAuthorisation construction from match_participants/identities
via the already-built domain.SignJoinAuthorisationHMAC), not a simple
wiring fix -- a wrong design choice here is a join-authorization
forgery risk, not just a UX gap, so it isn't something to build
unprompted the way the smaller fixes earlier this session were.
Recorded in three places for visibility: a new root-blocker callout
in §0 (the outstanding-work index), the top Status line, and expanded
detail in §8.31's own row, which previously undersold this as merely
'production signer... remain'.
No code changes. Verified the doc edit didn't touch anything else:
go build/vet/test -race clean, full Godot suite 220/220 clean
(unchanged, as expected).
Closes §8.43's 'failed-reconnect UX' gap, found immediately after
wiring connect_to_assignment() itself: even with that fix in place, a
connection failure had nowhere to go. connect_to_assignment()'s
synchronous failures (assignment missing/expired, invalid endpoint,
NetworkManager.join() erroring immediately) only ever emitted
assignment_connection_failed -- a signal nothing in the client
listened to. state.phase would stay stuck at ASSIGNED, the UI would
keep showing "Your match server is ready" forever, with no way back
to a fresh search.
Worse, the likelier real-world failure mode had no handler at all:
NetworkManager.join() returns OK immediately once the attempt starts,
but the actual ENet handshake can still fail asynchronously afterward
(unreachable server, refused connection, ENet's own ~5s connect
timeout). This is exactly the gap main_menu.gd's own
_on_connection_failed exists to cover for the direct-join flow (see
its header comment) -- nothing covered the equivalent for a
matchmaking-driven connect.
ControlPlaneClient now connects both assignment_connection_failed and
NetworkManager.connection_failed (guarded to state.phase == CONNECTING,
so it never misattributes an unrelated direct-join failure to a
matchmaking search) to state.fail(...), so either failure mode now
surfaces as a failed search the player can actually retry from.
Verified: three new test_control_plane_client.gd tests cover the
synchronous failure path, the async NetworkManager.connection_failed
path (via a real end-to-end ASSIGNED -> CONNECTING flow), and that an
unrelated connection_failed outside CONNECTING is correctly ignored.
220/220 tests pass, stable across 3 repeated runs, no crash, no
engine-level error; full make verify-multiplayer-local and the
complete make verify-enet-integration suite (all five cases) both
clean; zero new crash reports throughout.
Also fixes a markdown table-integrity mistake introduced while
documenting this in the same edit pass: an earlier Edit call
accidentally duplicated a sentence and dropped the row's closing
'remains' clause in §8.43 -- caught and corrected before commit via
the usual pipe-count check.
Closes §8.43's 'version-mismatch-specific client messaging' gap. Per
the user's explicit go-ahead to design new server behavior for this
(rather than only wiring up something that already existed, the
pattern every other fix this session followed): before this, there
was no server-side protocol rejection at all. queue_create accepted
any protocol_version >= 1 unconditionally, so an outdated client could
only ever discover a mismatch by waiting in the queue forever
unmatched -- the matcher's own compatibility check requires every
formed player to share an identical protocol_version -- with no error
and no explanation given to the player.
Server: Service.MinProtocolVersion (opt-in, zero by default so every
existing caller keeps accepting protocol_version 1 unconditionally)
rejects a below-floor queue_create with 426 Upgrade Required /
client_outdated before the request ever reaches the candidate
provider. Wired via cmd/control-plane's new --min-protocol-version
flag (validated non-negative at startup).
Client: ControlPlaneClient recognises HTTPClient.RESPONSE_UPGRADE_REQUIRED
on queue_create specifically and sets a distinct 'Your client is out
of date -- please update to continue searching' message instead of
the server's raw generic error string, and clears _last_queue_create
so can_retry_queue_create() never offers 'Retry Search' for a failure
that retrying with the same build can never fix.
Verified: go build/vet/test -race clean across every server package.
TestQueueCreateEnforcesMinProtocolVersion covers below-floor rejection
(candidate provider never reached), exactly-at-floor acceptance, and
the error body naming client_outdated; TestQueueCreateMinProtocolVersionZeroIsDisabled
proves the opt-in default doesn't change behavior for every existing
caller. Godot: test_outdated_client_receives_a_distinct_message_and_no_retry_offer
proves the distinct message and suppressed retry offer. Full Godot
suite (217/217, 0 failed, no crash), full make verify-multiplayer-local
gate, zero new crash reports.
Investigating §8.16's 'arena selection... remain' note (looking for
the next real Godot-side gap) found the described work was already
fully done, not missing: domain.RankedArenaForProposal selects the
ranked arena deterministically at proposal time; agones.Client.Allocate
already requests it (plus playlist/region/build/protocol/transport) as
Agones GameServerAllocation annotations; supervisor.withAllocatedCompatibility
already overlays every one of those onto the allocated Godot process's
launch command, overriding the Fleet's static per-image defaults --
fully tested (TestWithAllocatedCompatibilityOverridesStaleFlagsAndRejectsUnsafeValues)
and wired into Supervisor.Start(). Casual deliberately leaves
proposal.ArenaPath empty and the allocated server falls back to its
own ArenaRegistry.path_for_match rotation -- the same mechanism the
community server already used, always the intended design, not a gap.
§8.41's 'dynamic per-match launch flags... remain' note was the same
stale claim about the same already-built mechanism, described from
the other side (the assignment/roster row instead of the matcher
row). Corrected there too, pointing back to §8.16 rather than
duplicating the explanation.
No code changes -- this is the same category of finding as the
crash-loop and connect-wiring fixes earlier this session, just
resolved by correcting the record instead of writing new code, since
the record was wrong rather than the implementation. Verified the
referenced test by name: go test ./supervisor/... -run
TestWithAllocatedCompatibilityOverridesStaleFlagsAndRejectsUnsafeValues
passes; full go build/vet/test -race clean across every server
package (unchanged from before, since only the doc changed).
A player who completed the entire queue -> proposal -> allocate ->
assign pipeline would reach ASSIGNED, see "Your match server is
ready", and then simply sit there forever. connect_to_assignment()
existed in control_plane_client.gd, fully validated (checks the
assignment is available and fresh, splits and validates the
endpoint, carries the join authorisation via MatchNet's hello payload
rather than the URL) with its own assignment_connection_started/
assignment_connection_failed signals, and multiplayer-next.md's own
§8.41 row already described it as wired -- but grepping the whole
client found zero callers. Nothing anywhere in matchmaking.gd or
control_plane_client.gd itself ever invoked it.
ControlPlaneClient._connect_when_assigned() now calls it automatically
the moment state.phase reaches ASSIGNED. Wired into the single call
site every queue-shaped HTTP response already shares (heartbeat,
recover, and resync-triggered recover alike, since the WebSocket
match-lifecycle path always funnels into a REST resync first), so
both the ordinary poll path and the WebSocket-push path are covered
without a second call site to keep in sync. Two orderings are handled:
if the assignment fetch triggered earlier by ASSIGNMENT_READY has
already completed, it connects immediately; if not, it defers via
_pending_connect_match_id and resolves once the assignment becomes
available. _connect_attempted_match_id guards against a duplicate or
replayed ASSIGNED event reattempting the connection.
Verified against the real Godot 4.7.1 binary now that headless
testing has resumed: two new test_control_plane_client.gd tests cover
both orderings and the duplicate-attempt guard directly (216/216
total, 0 failed, no crash, no engine-level error, stable across
repeated runs); full make verify-multiplayer-local and the complete
make verify-enet-integration suite (all five cases, including the
3-process match) both pass clean; zero new crash reports throughout.
multiplayer-next.md's §8.41 row is corrected to describe what was
actually true (connect_to_assignment existed but was never called)
rather than repeating the prior, inaccurate 'already wired' claim.
Closes most of §8.43's 'decline, regional outage retry UI, failed
reconnect, duplicate-action recovery beyond proposals' remaining
list -- turned out to be mostly stale doc, not missing code.
matchmaking.gd's decline button/handler already existed
(%DeclineButton, _on_decline_pressed, visibility toggled by
MatchmakingState.PROPOSED alongside accept). ControlPlaneClient's
can_retry_last_mutation()/retry_last_mutation() -- the generic
'duplicate-action recovery beyond proposals' and 'regional outage
retry' mechanism -- also already existed: any mutation (not just a
proposal response) becomes retryable on a transport failure or a
408/429/503 response, and matchmaking.gd's queue button already fell
back to it ('Retry Request'). Neither had any test coverage proving
the mechanism actually works for a non-proposal mutation --
is_retryable_mutation_response's pure classification was the only
thing tested.
Two new tests: test_generic_mutation_retry_recovers_after_a_transient_failure
proves can_retry_last_mutation() transitions from false (mutation
in flight) to true after a transport-level failure on an ordinary
queue_heartbeat, exactly the 'regional outage' case; test_generic_mutation_retry_is_not_offered_for_unsafe_failures
proves a 409 (revision conflict) is never offered as a blind retry
and that retry_last_mutation() fails closed with ERR_INVALID_DATA
rather than resending a stale mutation. retry_last_mutation's literal
network dispatch (HTTPRequest.request()) is not exercised -- it needs
a live SceneTree that test_runner.tscn's synchronous single-_ready()
execution model cannot provide mid-suite; the two tests cover the
can_retry_last_mutation() decision boundary and the fail-closed path
instead, which is what's actually new here.
Verified against the real Godot 4.7.1 binary now that headless
testing has resumed: test_runner.tscn 214/214 clean (no crash, no
engine-level error), full make verify-multiplayer-local re-run clean,
zero new crash reports.
Remaining in §8.43: version-mismatch-specific messaging (a protocol
rejection currently surfaces only as the server's generic error
string), failed-reconnect UX, and §8.16's arena selection/long-running
worker integration.
Godot testing had been paused since earlier this session after a run
of native engine crashes (macOS crash reporter, EXC_BAD_ACCESS/SIGBUS)
that the user had confirmed as caused by this session's headless
invocations, based on temporal correlation with the session's own
activity.
Read the actual crash reports at
~/Library/Logs/DiagnosticReports/Godot-*.ips instead of relying on
that correlation. Every one of the 25 reports on the machine names
ChatGPT/codex (17 directly, 6 via an already-exited process in that
same tree) or a manual iTerm2 session (1) as the responsible/parent
process in the crash's own process tree -- none name Claude Code.
Codex (via the ChatGPT desktop app) was apparently running headless
Godot invocations concurrently with this session that day; the
crashes were most likely misattributed to Claude Code on timing
alone, not on anything in the crash reports themselves.
Presented this finding to the user, who confirmed resuming Godot
testing. Re-verified clean with zero new crash reports: test_runner.tscn
(212/212), the full make verify-enet-integration suite (all five
cases including the 3-process match), scripts/verify_control_plane_proposal_integration.sh
(passed twice -- the real matcher forms the proposal and both real
headless clients accept it), and the complete make verify-multiplayer-local
gate end to end (Go tests/race/vet, all three fuzz targets, 212 Godot
tests, contracts, manifests).
This unblocks the Godot-side work multiplayer-next.md had been
holding open pending this question: the two-player proposal
integration script (already on disk, now proven to pass) and the
Phase 8 client-experience tasks (8.39-8.43) that were waiting on the
same answer.
Closes part of the 'live Redis failover' gap in §8.46, found by
reproducing a genuine Redis outage (not just an empty/partial cache)
against CandidateProjection.Snapshot with a killed miniredis instance.
CandidateProjection.Snapshot funnelled two different situations into
the same code path: the index erroring outright (Redis unreachable)
and the index coming back empty (ambiguous — a genuinely empty queue,
or a lost keyspace). Both went through Repair, which itself calls
Index.Rebuild — a second Redis round-trip that fails for exactly the
same reason the first one did. The result: a real Redis outage, or
the window during a failover, made Snapshot fail outright even though
PostgreSQL — the documented authoritative source everywhere
(RedisCandidateIndex's own comment, cmd/matcher, cmd/control-plane's
--redis-addr help text all call it a rebuildable/optional
acceleration layer) — was completely healthy. Matchmaking would stop
entirely on a Redis outage despite the architecture explicitly not
requiring that.
Snapshot now falls back to serving Source (PostgreSQL) directly
whenever the index errors OR comes back empty, and only best-effort
attempts to repopulate Redis afterward — that attempt's outcome is
deliberately ignored, since a caller must never be denied service
just because the opportunistic rebuild also hit the same down Redis.
Snapshot still fails when Source itself is unavailable; the fallback
is not unconditional.
Verified: reproduced the bug first (killed-miniredis Snapshot call
failed even though Source was healthy), then fixed it. go build/vet
clean; all pre-existing store-package tests pass unmodified,
including the two live-redis:7-alpine-container tests
(TestRealRedisCandidateIndexUpsertSnapshotRemove,
TestRealRedisCandidateProjectionRepairsAfterFlush, run against a real
container and torn down after). Two new tests cover the fallback
directly (killed miniredis, Source still served, exactly one Source
call) and that the fallback is not unconditional (both Redis and
Source down still fails). Full go test ./... -race clean across
every server package.
Remaining: live matcher-worker-under-load-during-failover integration,
i.e. running the actual matcher process against a real Redis that
goes down mid-run under concurrent load, not just this unit-level
reproduction.
Closes the 'innocent-ticket restoration' gap noted in §8.20 and found
by re-examining §8.16's matcher worker. domain.FormFromQueue's anchor
is always the single oldest candidate, deterministically. If
domain.PrepareProposal then rejected that exact formation for a
reason specific to those particular players — mismatched protocol,
incomplete ranked identity metadata, a duplicate-SteamID pair, ranked
admission generally — RunOnce returned immediately and the next
matcher interval reproduced the identical formation and failed again.
Forever: nothing in the queue ever changes, so the same doomed anchor
group would be retried every single pass, permanently head-of-line-
blocking every other waiting player behind it too, not just the
players actually at fault. This is worse than the already-fixed
no-common-region crash-loop (§8.16) — that one killed the process;
this one fails silently and just never matches anyone again.
Two changes, both required together:
1. RunOnce now excludes a failed formation's players and retries
with the remaining candidate pool, bounded to 8 attempts per pass.
A batch with no viable formation at all (the pre-existing
no-common-region case) still returns immediately, since retrying
that can't help.
2. That fix was inert without a second one: RunOnce was asking
Source for exactly w.Size candidates, so after excluding one
failed formation's players there was nothing left to retry
against. domain.SelectCandidates was always designed to search a
larger pool (anchor plus an arbitrary remainder, widening through
it) — the call site just never gave it one. RunOnce now requests
up to 10x w.Size, capped at 200.
Verified: go build/vet/test -race clean across every server package.
Three new matcher tests cover the exclusion retry (an 8-candidate
batch whose permanently-doomed oldest 4 still lets the remaining 4
form and claim, correctly excluding the doomed players from the
claimed ticket set), that exhausting every attempt surfaces the last
real error rather than a silent false/nil, and that Source is
actually asked for more than w.Size candidates — a regression guard
for exactly the companion bug above. All six pre-existing worker
tests still pass unmodified, confirming the fix preserves every prior
guarantee (mixed-playlist/duplicate-identity rejection, incomplete
batch handling, durable claim failure propagation, Run's existing
per-pass-error survival).
Closes the 'concurrent proposal-recovery expiry races' gap noted in
§8.46. GetProposal (read-side recovery) and RespondToProposal both
run the identical expiry-advance SQL in their own transaction, so any
number of them can observe the same past-expiry proposal at once —
this had never been exercised concurrently, only sequentially (the
existing late-response test drives one call at a time).
TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce
races 8 concurrent GetProposal/RespondToProposal calls, each with a
distinct 'now' past the proposal window, against one proposal and
asserts: EXPIRED lands on the proposal and both tickets exactly once,
a PROPOSAL_TIMEOUT penalty lands exactly once per offending player
(not once per racing transaction), and no idempotency row survives a
closed-proposal response. The design already defends against this —
ProposalParticipantExpireSQL only ever flips a still-PENDING row
once, so a losing racer's 'now' can't match
recordProposalTimeoutCooldowns' responded_at filter — this test is
what actually proves that holds under real concurrent load rather
than by inspection.
Verified: real postgres:17-alpine container, go test -tags
integration ./store/... -run
TestPostgreSQLConcurrentProposalExpiryRecoveryAppliesCooldownsExactlyOnce
-race -count=3 clean; full -tags integration ./store/... -race run
clean; full non-integration go build/vet/test -race clean across
every server package; container removed after the run.
Closes the 'live duplicate/conflict alerting also remains' gap noted
in §8.10: a durable domain.ErrConflict/ErrResultConflict rejection on
/v1/servers/{id}/{register,connect,disconnect,shutdown,result} was
already logged as a structured 'conflict' stage event, but had no
Prometheus signal distinct from the generic 4xx-class counter, which
also catches ordinary client noise (malformed bodies, expired
tokens). A real duplicate registration, raced reconnect, or replayed
result would have been invisible to alerting until someone went
looking through logs.
observability.Metrics gains ObserveServerConflict(kind), a bounded
counter keyed to serverMutation's own five routes (an unrecognized
kind folds into "other", so a caller mistake can't grow the label
set), exported as cosmic_clash_api_server_conflicts_total. Wired at
each of serverMutation's four conflict branches in server/api/service.go.
deploy/observability/prometheus-rules.yaml adds
CosmicClashControlPlaneServerConflicts, mirroring the existing
allocator quota-denial alert shape, firing on >3 conflicts of one
kind in 15 minutes.
Verified: go build/vet/test -race clean across every server package;
new unit tests cover per-kind counting, the bounded 'other' fallback,
the counter's absence until first observed, and a nil-receiver no-op;
a service-level test proves a real register conflict is exported
through the live /metrics endpoint. scripts/verify_observability_manifests.py
passes against the edited rules file.
Remaining, and explicitly out of scope here: this alert has only been
validated statically, never against a live Prometheus/Alertmanager
firing on real traffic — that requires the same live cluster this
sandbox has never had.
Josh Creek