newAPIService never supplied SteamLogin, so POST /v1/session/steam
always returned 503 auth_unavailable in production. The only
implementation was cmd/testkit-api's fake, which derives an identity
from the ticket string itself and accepts anything -- so the passing
integration path was neither deployable nor secure. On the client side
the game started with an empty token and a loopback base URL, and no
production code called configure() or login_steam(); the menu entered
matchmaking directly, so every request failed ERR_UNAUTHORIZED before
reaching the network.
Add a real ISteamUserAuth/AuthenticateUserTicket adapter behind an
interface, so the production login path is testable with only the Valve
call stubbed. It rejects family-shared copies (the account playing does
not own the app) and, by default, VAC- or publisher-banned accounts, and
refuses malformed tickets locally rather than forwarding them.
Crucially it separates our faults from the player's: a Valve outage or a
revoked publisher key returns 503, not 401. Answering 401 would tell a
legitimate player their login failed and send them to fix an account
that is fine while the real fault went unnoticed. A banned identity now
returns 403 rather than a misleading 503.
Sign-in is configuration-gated on the publisher key and App ID: without
them the endpoint keeps returning 503, since silently accepting an
unverified ticket would be worse than refusing to authenticate. A
returning player keeps the player ID they already had, so ratings,
penalties and bans follow the account rather than the session.
Client side: acquire a web-API ticket through GodotSteam's async
signal -- requesting one returns a handle, not a ticket -- using the
existing dynamic-call pattern so stock Godot still parses the project.
The endpoint is configurable for release builds, and matchmaking
completes sign-in before it will queue.
Verified against real PostgreSQL; 232 Godot tests pass.
domain.validCandidate hard-requires a non-empty PredictedRTT map, but
CreateQueueTicket persisted an empty one and the only endpoint that
could fill it returned 503 in every real binary, because Service.Probe
was assigned nowhere outside api tests. No client-created ticket could
ever be selected by the matcher. The Godot client had no probe method at
all, so even a wired backend was unreachable from the game.
Four distinct defects had to be fixed for this path to work:
Nothing issued the nonce ProbeProvider was meant to compare against, so
the contract could not be satisfied even in principle. Add
POST /v1/probes/{region}/challenge, backed by a durable single-use
challenge -- durable because any replica may serve the answer for a
challenge another replica issued. RTT is the interval between issuing
and receiving, so no client-reported latency reaches placement.
CreateQueueTicket marshalled a nil map to JSON `null`, a JSONB scalar
rather than an object, and jsonb_set rejects that with "cannot set path
in scalar". RecordProbe would have failed at runtime even once wired.
Persist an object, and normalise non-object values in the update for
rows already written.
A nil ProbeRecorder made the handler report success while persisting
nothing, which silently leaves the ticket unmatchable. That is a
misconfiguration, not a successful probe; it now returns 503.
A successful probe updated PostgreSQL only. The candidate inserted at
enqueue time carries an empty RTT map, and the Redis keyspace has its
TTL continually refreshed, so the stale entry need never repair itself.
Refresh that player's projection after the probe commits.
Client side: add the challenge/answer round trip and have the
matchmaking screen collect evidence before creating a ticket, since
queueing first produces a search that can never match. Probing every
region fully is not required -- placement uses whichever regions
answered -- but queueing with none is refused rather than silently
stalling.
New integration test drives the real enqueue and probe paths and then
asks the actual matcher predicate, rather than hand-building a candidate
the way the unit tests do -- which is exactly why they missed this.
Also make the integration schema reset drop the whole public schema: the
enumerated table list silently broke with each new migration.
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.
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.
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.
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.
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.
The just-landed signed workload token embedded (allocation_id, match_id,
server_id) as claims. That doesn't actually work for its intended delivery
channel: the token is meant to be requested as a GameServerAllocation
annotation in the SAME request that asks Agones to pick a server, so at mint
time the allocator knows allocation_id (it generates it) but not yet which
server_id Agones will return -- server_id only exists in Agones's response,
after the annotation request has already been sent. Embedding it was simply
not possible for the real caller this was built for; only the (allocator ->
signed_token) unit tests and hand-constructed integration tests happened to
supply it directly, masking the gap.
Fixes it by having the token bind only allocation_id (the one identifier
actually known at mint time) plus expiry. match_id/server_id are resolved at
verify time from the durable allocations table via the new
store.AllocationBindingByAllocationID, keyed by allocation_id -- which the
allocator already records immediately after Agones responds. This is
strictly stronger, not just a workaround: a caller can no longer claim any
match/server pairing at all, even one that happens to be internally
consistent -- the binding returned is entirely durable-record-derived.
Verified: server/workload's unit tests updated for the new two-field claim
shape; server/api's Postgres integration suite gains
TestWorkloadVerifierFromSignedTokenNeverTrustsCallerSuppliedBinding (two
distinct real allocations each resolve to their own, and only their own,
match/server pairing) replacing the now-inapplicable mismatched-triple test.
Full `go build ./... && go vet ./... && gofmt -l . && go test ./... -race`
and `go test -tags integration ./... -race` both clean; the api integration
suite re-run 3x clean against a live postgres:17-alpine container.
WorkloadVerify (api.Service.WorkloadVerify) was permanently unwired: both
/v1/servers/{id}/register and /v1/servers/{id}/result always 503, because
the only design considered so far was verifying a Kubernetes-projected
service-account JWT (server/workload/jwt.go), which needs a live cluster's
TokenReview/JWKS endpoint to validate against safely -- something this
sandbox cannot do without guessing at a trust boundary.
The API layer doesn't actually require that specific mechanism. serverMutation
only compares WorkloadBinding.ServerID and .MatchID (server/api/service.go);
AdvanceServerRegistration only uses .MatchID/.ServerID/.AllocationID. Nothing
downstream needs Namespace/ServiceAcct/PodUID/GameServerUID populated.
This adds a self-contained alternative: a short-lived, HMAC-signed token the
control plane mints and verifies with a secret only it holds (server/workload/
signed_token.go), the same trust model domain.SessionStore already uses for
player sessions elsewhere in this codebase. It needs no cluster to verify --
signature + expiry is fully self-contained and unit-testable.
The design's soundness rests on the delivery channel, not the crypto: the
token is meant to reach the allocated GameServer via the same Agones
GameServerAllocation annotation channel allocation.go already uses for
match-id/allocation-id, readable only by that pod's own local SDK sidecar. A
caller presenting this token has already proven, via that channel, that it is
the pod Agones allocated. (Wiring the actual annotation delivery -- extending
agones.Client.Allocate and the supervisor's token source -- is a separate,
follow-up change; this commit lands the verification core it depends on.)
store.AllocationBindingStillValid adds defense-in-depth on top of signature
and expiry: it cross-checks the token's claims against the durable
allocations table (append-only, never leaves 'ALLOCATED'), so a validly-signed
token naming an allocation that was never recorded -- or a real allocation id
paired with a mismatched match/server -- is still rejected.
api.WorkloadVerifierFromSignedToken wires the two together and is now plugged
into cmd/control-plane (new --workload-secret / COSMIC_CLASH_WORKLOAD_SECRET
flag; a startup warning is logged if it's left unset, since the route then
stays 503 exactly as before) and cmd/testkit-api (fixed test secret, since
that binary is test-only already).
Verified: new unit tests in server/workload (signature tamper, wrong secret,
expiry boundary, malformed input) and a new Postgres integration suite in
server/api (real allocation row, real signed token, acceptance / unknown-
allocation rejection / mismatched-triple rejection / the previously-503
Service.WorkloadVerify field itself) -- both run clean with -race across
multiple passes against a live postgres:17-alpine container. Full
`go build ./... && go vet ./... && gofmt -l . && go test ./... -race` and
`go test -tags integration ./... -race` both clean.
Same discovery pattern as ResultSubmitter/SessionIssuer, one level
deeper: api.Service.rankedProfile and .profile both only ever read
from an in-memory RankedProfiles map with no durable-store equivalent
at all -- not "adapter exists but unwired" this time, there was no
adapter. Every real request to GET /v1/profile/ranked or /api/v1/profile
always 404'd regardless of a player's actual rating.
Add RankedProfileProvider (an interface, not a struct-literal adapter
this time) and store.PostgresRankedProfiles reading the ratings table;
Service.rankedProfileFor prefers it when set and falls back to the
map otherwise, so every existing test/direct Service literal keeps
compiling and passing unchanged. A missing ratings row maps to the
exact same (zero value, false, nil) the map lookup already produced,
preserving existing not-found semantics rather than reinterpreting
them. LastSeasonID/SeasonHistory are deliberately left unset -- the
ratings table has no season pointer, and reconstructing history needs
its own query and display semantics, not bundled in here speculatively.
Wired into both cmd/control-plane and cmd/testkit-api. Verified
against real PostgreSQL via curl: a fresh identity's ranked profile
correctly 404s through the real adapter (same behavior as before,
now for a real reason instead of an empty map).
Wire the same Service.Log hook added for the server register/result
routes into queue create/heartbeat/cancel and proposal accept/decline:
log the resulting state on success (queue_create, queue_heartbeat,
queue_cancel, proposal_response) or 'rejected' on a domain error,
using only the ticket/proposal ID and outcome -- never the domain
error text itself, which isn't documented as credential-free.
Read-only routes (queue GET, proposal GET, assignment fetch) and the
early availability/not-found rejections that return before reaching
the domain call are deliberately not logged in this pass.
Covered by a new end-to-end test driving real create/heartbeat/cancel
and an accept followed by a stale-revision accept (fenced for real by
the domain layer behind proposalBackendSpy, unlike the dumb queue
spy), asserting the exact sequence of events logged.
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.
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.
Josh Creek