mirror of
https://github.com/jcreek/CosmicClash.git
synced 2026-09-10 16:04:04 +00:00
52ee1810427ea2b14c3cbc780b83f8ccbb881243
865 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Josh Creek
|
52ee181042 |
fix(kind): validate the allocation response Agones actually returns
With the Fleet readiness wait corrected, the gate reached the allocation
check for the first time and failed with "allocation did not return a
GameServer" -- while the cluster dump shows the allocation plainly
succeeded: one GameServer Allocated, Fleet reporting ALLOCATED 1.
GameServerAllocationStatus is flat: state, gameServerName, address,
ports, nodeName. It does not embed the allocated GameServer. The
validator read status.gameServer.metadata.name and
status.gameServer.status.{address,ports}, a shape Agones never sends,
and its unit tests asserted that same invented shape -- so validator and
tests agreed with each other while both disagreed with Agones. Nothing
caught it because the gate had never once allocated anything.
Read the real fields, keeping every existing check: non-empty name,
address neither blank nor unspecified, exactly one named "game" port in
range.
Also print the response body when validation fails. work_dir is removed
by the EXIT trap, so a shape mismatch was otherwise invisible from CI --
which is how this survived. If the shape is still not what I expect, the
next run says so instead of costing another round trip.
|
||
|
Josh Creek
|
4f48f0a6a8 |
fix(kind): wait on the Fleet field that exists
The gate asserted `--for=jsonpath='{.status.ready}'=2`. An Agones Fleet's
status carries replicas, readyReplicas, reservedReplicas and
allocatedReplicas -- there is no `ready` -- so the wait could never match
however healthy the Fleet was.
It failed in the most misleading way available: as "the Fleet never
became ready", which sent three separate investigations after the game
server. Two of those found genuine bugs, but the gate would have stayed
red with both fixed.
The evidence is in the previous CI run's own dump, which the new
per-container diagnostics produced: both GameServers Ready and stable for
5m6s, and the Fleet reporting DESIRED 2 / CURRENT 2 / READY 2, while
kubectl wait timed out beside it. That same dump also confirms the health
fix in
|
||
|
Josh Creek
|
0de97381b7 |
fix(agones): stop a dead health loop from passing as a healthy server
Every allocated GameServer reached Ready and was recycled by Agones ~20s later. Health pings are the game process's job by design -- the supervisor has no health implementation at all -- so a server that stops pinging is exactly what Agones is built to reclaim. start_health() armed a Timer on a node that might not be inside the SceneTree. A Timer only ticks inside the tree, so the node reported itself configured, sent nothing, and said nothing about it. It now returns a bool, refuses loudly when unconfigured, and defers to _ready() when called before parenting, so the SDK arms its own timer and no caller has to get the ordering right. server_boot.gd defers the add like every sibling does (§9 gotcha 27) and logs when AGONES_SDK_HTTP_PORT is missing, which previously read identically to a healthy start. Also bounded the in-flight latch: it is set across an await, so a request that never completes would silence health permanently. Defence in depth rather than an observed fault. Tests target the contract rather than the mechanism: a test that parents the SDK correctly and asserts pings passes with the bug present, because the defect was in the wiring. The unit tests assert start_health() cannot claim success out of tree, and were confirmed to fail against the previous code. The smoke gains a counting sidecar and asserts a *repeating* ping -- it reports "health pings in 3.0s = 1, want at least 2" when the loop is broken, which is the production symptom exactly. It is also now actually run: nothing referenced it before. Two diagnostic fixes, both of which changed conclusions during this work: The kind gate only built the game-server image when the tag was absent, so a local rerun silently verified whatever was built last. That is why local runs and CI disagreed about the same commit. It now builds by default, with KIND_REUSE_GAME_SERVER_IMAGE=1 as the opt-in fast path. The failure dump logged only not-ready pods, and used --all-containers with a shared tail. A GameServer recycled after reaching Ready leaves no unready pod behind, and the Agones sidecar out-logs the game server, so the relevant output was never captured. It now dumps every pod, per container, current and previous, plus the GameServer and Fleet resources -- Agones' own state machine is what rejects these. |
||
|
Josh Creek
|
ca70568fad |
fix(agones): make the kind gate's Agones lifecycle actually work
Several independent causes, all of which had to be right before the Fleet could reach Ready. The supervisor pointed --sdk-base-url at 127.0.0.1:9357, which is the Agones sidecar's gRPC port; its HTTP surface is 9358, and that is what AGONES_SDK_HTTP_PORT carries and what agones_sdk.gd reads. An HTTP client against the gRPC port could never have worked, in kind or in production. The supervisor also treated the sidecar's first incomplete /gameserver response as fatal. The sidecar accepts requests before the controller populates status.address and status.ports, so this produced a restart loop precisely during normal Agones startup. It now polls until the endpoint is assigned or ReadyTimeout elapses. server_boot.gd started ServerControl and the Agones SDK only under --allocated-mode, but the kind smoke deliberately strips that flag, so nothing served the readiness probe and the GameServer could never become Ready. Lifecycle now keys on AGONES_SDK_HTTP_PORT, which Agones injects into every managed container, while allocation and roster semantics stay tied to --allocated-mode. The SDK node is added to the tree non-deferred, since start_health() creates a Timer immediately. Fleet: Agones assigns its own SDK service account and masks that token from the game container while keeping it for the injected sidecar, so the manifest must not pin serviceAccountName or automountServiceAccountToken. Godot stores user:// under HOME, so HOME points at the writable runtime volume to keep the root filesystem read-only, and fsGroup makes that volume writable for the non-root user. Namespace: Agones' Dynamic port policy injects a hostPort, which both the baseline and restricted Pod Security Standards forbid, so the workload namespace enforces privileged while continuing to audit and warn against restricted. NetworkPolicy: the injected sidecar reaches the Kubernetes API over HTTPS, and NetworkPolicy applies to the whole Pod rather than to the container whose token was masked. The kind runner creates the namespace before Helm so Agones can install its per-namespace SDK RBAC, scopes gameservers.namespaces to it, forces the allocator and ping Services to ClusterIP because LoadBalancer ingress never becomes ready in plain kind, and labels the node so the production Fleet's on-demand/zone constraints are exercised rather than edited out of the rendered manifest. |
||
|
Josh Creek
|
8aa4af3a3a |
test(kind): always dump on failure, and record the second Agones failure
The dump added in
|
||
|
Josh Creek
|
9ab1bec89a |
test(kind): dump cluster state before the Agones gate deletes its cluster
This gate fails with nothing but Helm's "context deadline exceeded" and three Deployments reporting Available: 0/1, then the EXIT trap deletes the cluster -- so there is no way to learn why the pods never became ready. Both CI runs and a local run are equally uninformative. Dump node capacity and conditions, pods and recent events for agones-system and cosmic-clash, and describe plus current/previous logs for every not-ready pod, on any failure and before deletion. Events matter as much as pod status here: FailedScheduling, ImagePullBackOff and probe failures are all invisible in a status column. KIND_KEEP_ON_FAILURE=1 retains the cluster for interactive inspection. Same approach that just found the allocated-Compose cause, where a silent assertion had hidden a real 422-instead-of-409 API bug across several CI runs. |
||
|
Josh Creek
|
14da286e11 |
fix(store): return 409 for idempotency key reuse, not 422
Both idempotency paths returned a bare fmt.Errorf, and writeDomainError maps anything it does not recognise to its 422 "invalid_request" default. So reusing a key with a different payload answered 422 where openapi.json declares 409 and state-transitions.json requires "reject_conflict_without_state_change". That is the difference between "your request was malformed" and "that key is taken". A client acting on 422 would rewrite a request that was never wrong, and the 409 branch of every generated client was unreachable. Wrap domain.ErrConflict on both the create and mutate paths, and add an integration test covering identical replay and conflicting reuse. Pre-existing: both bare errors are unchanged from |
||
|
Josh Creek
|
fc2f5c8669 |
test(compose): report the actual status when the idempotency conflict check fails
The ERR trap added in
|
||
|
Josh Creek
|
432e5a11e8 |
test(compose): make the allocated smoke explain its own CI failures
This suite fails on GitHub Actions while passing locally, and it failed
the same way at
|
||
|
Josh Creek
|
707aea5898 |
docs: mark the 8.48 Compose fixture item done
compose.allocated-smoke.yml and verify_allocated_compose.sh are already independent of compose.phase6-smoke.yml -- the script says so explicitly and reuses none of its ports -- so the allocated-mode flow no longer inherits that fixture's hardcoded port, first-come slots or --max-matches=2. Fifteenth stale backlog entry found this session, and the first in TODO.md rather than multiplayer-next.md §7. It was also the only remaining item marked agent-actionable. |
||
|
Josh Creek
|
8ba045063d |
fix(compose): give the allocated smoke's allocator its signing key
Making cmd/allocator refuse to start without join-signing material was right -- an allocator that binds allocations it can never publish rosters for strands every match silently -- but I updated the Kubernetes manifests and the kind fixture without updating the Compose one. The allocator container exited at startup, so no allocation was ever bound and verify-allocated-compose failed with "allocator did not bind a provider allocation". Mount the same join-signing-keys.json fixture the game server already uses and name the key it was written with. Caught by running the target locally rather than by CI after a push. |
||
|
Josh Creek
|
654f20e28f |
docs: record the backfill roster-delivery decision and remaining work
A backfilled player's join authorisation is issued after their server started, but the supervisor fetches the roster once before launching the game child and the game process has no reload path, so backfill cannot work end to end regardless of how good the selection rule is. Decided: the control plane marks the roster changed, the supervisor re-fetches and rewrites the roster file, then signals the game process to reload. Chosen because it reuses the authenticated channel and roster endpoint that already exist -- no inbound path into the game pod, no new trust boundary -- and keeps the roster an allowlist the server was told to expect rather than admitting anyone holding a valid signature. Signature verification is untouched and already binds match, server, slot and generation. Recorded in docs/MATCHMAKING.md, which the repo treats as the design source of truth, so the decision is not re-litigated from a task row. Remaining implementation is tracked in #32 and summarised in §7 8.19. |
||
|
Josh Creek
|
1becfb4f3f |
feat(domain): add casual backfill candidate selection
First slice of task 8.19. docs/MATCHMAKING.md specifies the choice precisely -- "the oldest ordinary casual ticket that meets the same build, region <=100 ms and current anchor-tolerance rules for the vacated human slot; ties use ticket ID" -- and that rule is needed whatever is decided about delivering a late authorisation to a running server, so it is worth landing on its own. Kept a pure function over an already-fetched candidate set: the choice is then reproducible and testable without a database, and claiming the ticket stays a durable transaction as it is for ordinary proposals. Ties break on ticket ID rather than scan order, so two replicas evaluating the same queue cannot offer one slot to different players. Region matching is stricter than ordinary formation: the server already exists in one region, so a candidate must have RTT evidence for that region specifically, not merely share some region with the others. Eligibility is re-checked here as well as at the durable boundary, so an ineligible mid-play or human-occupied slot never reaches selection at all. Ranked is refused outright. Corrected a comment I had written claiming the backfill window is shorter than an ordinary proposal's; both are 10 seconds. What makes a backfill offer separate is its payload and the absent decline penalty, not its timing. This does not yet make backfill work end to end -- see the roster delivery question raised alongside this commit. |
||
|
Josh Creek
|
61a073099d |
fix(store): widen the serializable retry budget, stop leaking test volumes
Two things that made the integration gate untrustworthy. The retry budget was too small for expected contention. TestPostgreSQLConcurrentIdenticalResultSubmission fires five identical concurrent submissions and requires all five to succeed; it failed 4 runs in 20. The error was retryable and retries did fire -- three attempts simply was not enough. Contention here is normal rather than exceptional: several game servers can submit results, and several matchers can claim candidates, against the same rows at once. Raised to five attempts, which is 0 failures in 40 runs. Also jittered the backoff, but measured rather than assumed: my first theory was a thundering herd, since the delay was exactly RetryBackoff*(attempt+1) and every loser of a race woke at the same instant. Isolating the two changes showed jitter alone moved 4/20 to 3/20, while the budget alone reached 0/20. The budget was the real constraint. Jitter is kept because it costs nothing and its benefit grows with the number of contending writers -- production is not capped at five -- but the comment now says plainly that it is the smaller half, so nobody inherits my wrong explanation. Second, the integration scripts leaked one throwaway database volume per run. --rm does reclaim anonymous volumes on a normal exit, but these scripts force-remove the container from a trap, and `docker rm -f` without -v keeps the volume. Sixty-four accumulated during this branch until PostgreSQL stopped starting, surfacing only as the scripts' own readiness timeout rather than as a disk error -- which is what the "Docker storage exhausted locally" notes were really describing. Measured at one volume per run before, zero after, across all five scripts. |
||
|
Josh Creek
|
a1f30f6af9 |
test(store): prove the audited claims instead of asserting them
The audit established which rows understated what was built by locating implementations and their call sites. That proves code exists, not that it works, so each corrected claim is now tied to an executable test. Seven of the nine were already covered and just needed naming: the allocated ServerConfig fields, signed-authorisation admission, endpoint wiring, casual lineup being reached through formation, three of the four penalty kinds, signed roster metadata, the season countdown, and the matcher deployments. Two had no proof at all: - INITIAL_CONNECT_NO_SHOW was the one penalty kind with no integration coverage, so "all four penalty kinds are written durably" rested entirely on reading the code. - Cross-replica revocation is a behavioural property. An in-memory cache in front of the session read would break it while leaving every call site looking correct, so no amount of reading establishes it. Writing the first one found my own error rather than a defect: casual deliberately waits past InitialConnectWindow to CasualBotStartAfter before deciding a no-show, giving a slow-loading player longer than the ranked deadline. Reconciling at the earlier window only yields WAIT. Both new tests were mutation-checked -- removing the penalty insert and removing the revoked_at check each make them fail -- so they assert something real rather than passing incidentally. |
||
|
Josh Creek
|
8033d52db3 |
docs: audit every Phase 7/8 task row against the code
Three tasks in a row this branch turned out to be partly built already:
8.20's allocation wiring was complete end to end, 8.22's client UI was
built, and 7.4/8.7 listed durable ban storage and the Steam adapter as
outstanding after both had landed. That is a systematic problem, not
three coincidences, so this checks all 56 rows rather than fixing them
one at a time as they are picked up.
Nine more rows understated what exists:
- 8.6 every allocated-mode ServerConfig field is present, signed
authorisation admission is in MatchNet, endpoint wiring is in
AssignmentState
- 8.8 distributed revocation needs no cross-replica protocol: sessions
are durable and read on every authenticated request
- 8.19 casual lineup formation is built and wired, and all four penalty
kinds are written durably; only the backfill proposal path is
genuinely missing
- 8.30 signed roster metadata landed with 8.31
- 8.42 the season countdown is implemented
- 8.16/8.43 the matcher is deployed; what remains is soak, not integration
- 8.13/8.52 cross-referenced to #31 rather than described loosely
The drift runs one way -- rows keep listing work that has since landed --
which inflates the apparent backlog and invites rebuilding what exists.
8.19 is the clearest case: it reads as four missing pieces and is one.
The §7 preamble claimed every row lists only what is still open. It
doesn't, so it now says so and points at the audit note, with the
standing instruction to verify a row against the code before planning
against it.
|
||
|
Josh Creek
|
2702e53068 |
feat(ranked): make tier thresholds durable instead of compiled in
Task 8.22. Tier bands lived in domain.DefaultTierPolicy(), compiled into every API binary, so retuning one meant building and rolling a new image -- least attractive exactly when it is most needed, as the rating distribution settles after launch. Bands now live in a tier_bands table, seeded by the migration with the exact policy the binaries hardcode, so this changes durable state without changing behaviour. Retuning is a rolling restart rather than a rebuild. Three properties the loader deliberately holds: - A malformed durable policy stops startup. Falling back on error would silently mis-tier every player, which is worse than not starting. - An empty table is supported and falls back to the compiled default, so an operator can truncate back to known-good without a deploy, and a fresh database works before the seed is reviewed. - PROVISIONAL is rejected as a band. It is derived from ranked game count, not rating, so a band claiming it would be unreachable at best and would shadow a real tier at worst. Bands stay backend-owned; clients still receive only the resulting label, per docs/MATCHMAKING.md. UNIQUE(min_rating) rejects two bands sharing a threshold, catching an ambiguous policy before NewTierPolicy does. testkit-api loads it too, so the control-plane integration scripts exercise the durable path rather than the compiled default. Integration tests cover the seeded policy matching the compiled one, retuning taking effect from the database alone, truncation falling back, and each invalid-policy shape being rejected. Verified they fail against a loader that ignores durable bands. The other two parts of 8.22 needed no work: the client UI already renders tier, provisional status, ranked games and the season countdown, and reconnect transport is 8.42's, dependent on live backend events. |
||
|
Josh Creek
|
8b9ae35b43 |
test(domain): guard the ranked arena list against Godot registry drift
Task 8.20. `arena_registry.gd` is the documented single source of truth for arenas, but `domain/ranked.go` keeps a hand-maintained mirror of its floor-goal entries and nothing checked the two against each other -- ranked_test.go asserts the same three paths the production code hardcodes, so both could drift together silently. Drift is not hypothetical in either direction. The registry's own comment anticipates flipping an elevated variant to random:true once a checkpoint trained on that geometry is promoted, which ranked would then keep excluding indefinitely. A rename or removal is worse: the allocator would hand out a scene path that no longer exists, and the ranked server fails to load its arena at match start -- after allocation, so it burns a real match and a real server. Keeping the two copies is deliberate rather than a wart: ranked arena selection is server-authoritative and happens before any Godot process exists. So this guards the relationship instead of removing it, the same way the golden join-authorisation token guards the signing format. It parses the registry and fails if the sets disagree either way, if rotation order diverges from declaration order, or if a ranked path has no scene behind it. The parser asserts it found both eligible and ineligible entries, so a format change cannot make everything pass vacuously. Verified against four drift scenarios. The allocation-wiring half of 8.20 turned out to be already complete end to end, with coverage at each hop; recorded in the task row rather than rebuilt. Add a Server Unit Tests workflow, because none of this would otherwise run: the only Go tests CI executed were multiplayer-load's two load tests, so ~24k lines of control plane gated nothing. Docker-free so it can gate every push, and it vets the integration-tagged files too, since those are excluded from the default build and could otherwise rot uncompiled. CLAUDE.md's CI section claimed two workflows and no unit-test job; there were seven and now eight. |
||
|
Josh Creek
|
a4b362cb01 |
fix(deploy): supply Steam credentials to the control plane, refresh stale status
The Steam adapter took --steam-publisher-key/--steam-app-id and the matching env vars, but no manifest supplied them, so a deployed control plane would have kept sign-in returning 503 even once the App ID from #15 arrived -- that issue would have unblocked nothing on landing. Mount them from a new cosmic-clash-steam Secret, into the control-plane Deployment alone: the publisher key is issued to us, never to a client, and no other workload (least of all a game server) has any use for it. A manifest test asserts both the wiring and that the Secret appears in no other manifest; verified it fails in both directions. Both keys are optional, so the Deployment still rolls out before the App ID exists and sign-in simply stays 503. Also correct task rows this branch made stale: 7.4 (durable ban storage landed), 8.7 (adapter, bans and secret store landed), 8.39 (cross-replica fan-out landed), and 8.5's migration range, which stopped at 0013. Move the branch review into docs/ with a header marking it a point-in-time artefact -- all thirteen findings are addressed, and its present tense would otherwise read as current behaviour. Record gotcha 52: the integration scripts use `docker run --rm`, which reclaims the container but not its anonymous volume. Sixty-four of them, ~4 GB, accumulated during this session until PostgreSQL stopped starting -- surfacing only as the script's own readiness timeout, not as a disk error. That is the real cause behind the "Docker storage exhausted locally" notes those rows carried. |
||
|
Josh Creek
|
0a8f3924d0 |
docs: cross-reference the new image-publishing issue in TODO.md
Issue #31 covers the gap that no workflow builds or pushes the images deploy/k8s/base references, and that every digest there is still an all-zero placeholder which the supply-chain gate accepts because it runs without --require-concrete. It blocks #17. |
||
|
Josh Creek
|
ccf7d0fbfe |
docs: record the closed root blocker and the new probe/rotation contract
docs/MATCHMAKING.md is the stated source of truth for this design, so it changes first: the probe challenge endpoint and why probing gates matching rather than merely improving it, and the key-ID rotation procedure that makes overlapping-key rotation concrete. multiplayer-next.md §0's root blocker is closed rather than deleted -- what it was, why it blocked everything, and how it was resolved, since the reasoning is what a future reader needs. Tasks 7.6, 8.15 and 8.31 updated to what actually remains, which in every case is now external rather than unbuilt. TODO.md #14 asked for a join-signing design decision; that decision is recorded with its rationale. CLAUDE.md no longer says a real deployment cannot complete a match end to end. |
||
|
Josh Creek
|
f628ccfd35 |
feat(auth): wire production Steam sign-in and the client login flow
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. |
||
|
Josh Creek
|
d40344a2c0 |
fix(deploy): ship runnable control-plane and matcher workloads
The Kubernetes base deployed a control-plane image the Dockerfile never built -- cmd/control-plane was absent from the Go build stage and no target existed -- while the Dockerfile built a matcher image no manifest ever deployed. Applying the checked-in base therefore could not produce the advertised topology: one required workload had no repository-defined artifact, and nothing consumed queued tickets. Tickets could be created but never became proposals. Add the production control-plane build and image target, explicitly not the testkit-api target, which injects a fake login accepting any ticket. Add casual and ranked matcher Deployments as separate workloads: they have different match sizes, and separating them keeps a ranked backlog from delaying casual formation. One replica each -- CreateProposal's SKIP LOCKED fences make more replicas safe, but they would halve the candidate pool each worker sees per poll and worsen formation for no throughput gain at this scale. Their PDB uses maxUnavailable, since minAvailable against a single replica blocks node drains outright. Also fix both blocked traffic directions. No ingress policy admitted UDP/7777 to game-server pods, so an allocated server was unreachable from the internet under the namespace-wide default deny. And control-plane ingress admitted only edge-gateway pods, so roster fetch, registration, connection receipts, shutdown and result submission from game servers were dropped even inside the cluster, despite their egress being permitted. The default deny stays. Manifest tests now assert every required role is deployed, both playlists are scheduled, every referenced image maps to a real Dockerfile target, and both traffic directions are permitted. Each was verified to fail against the defect it covers. The control-plane image was built and run to confirm the target works. |
||
|
Josh Creek
|
801fca7cb0 |
fix(matchmaking): make regional RTT evidence obtainable end to end
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.
|
||
|
Josh Creek
|
5765532409 |
fix(allocator): publish signed assignment rosters before servers start
The root blocker (issue #14). The worker bound the provider allocation and stopped. Service.PublishRoster and store.SaveVerifiedAssignmentRoster both existed, fully tested, with zero non-test callers, and the production allocator configured neither a roster store nor a signing key. Nothing ever wrote the assignments table. The allocated supervisor fetches a non-empty roster before it launches the game child, so every real allocation failed at that fetch: no match could reach ASSIGNMENT_READY or accept a player. Existing tests seeded assignments directly, which is exactly why the missing hand-off went unnoticed. The worker now builds one join authorisation per durable participant, signs each with the active key, and publishes them. Participants are read through the same query SaveVerifiedAssignmentRoster re-validates against, so the allocator cannot construct a roster the persistence boundary would reject. The manifest commits to a digest over the whole roster, so a server cannot be handed a truncated roster whose surviving entries are each individually valid. Persist the provider endpoint on the allocation: it arrived on the provider response and was never stored, so a worker crashing between allocating and publishing had no endpoint to recover and would have stranded the match permanently. Republishing is idempotent, so that crash now simply retries. cmd/allocator refuses to start without key material rather than running an allocator that binds allocations and silently strands every match. The k8s allocator Deployment mounts the same key set the Fleet does, and both now take the JSON key map so a rotation can publish several. New integration test drives the real worker through to the supervisor's own roster read path without seeding the assignments table. Verified it fails with "assignments = 0, want 2" when the publish step is removed. |
||
|
Josh Creek
|
b8bcc1f3c1 |
feat(join-auth): add key-ID rotation to signed join authorisations
Prerequisite for wiring the allocator to publish rosters. The signing key is a shared HMAC secret mounted into both the allocator and the allocated game server; without a key ID, rotating it would invalidate every authorisation already issued for an in-flight match, because a server holding only the new key cannot verify a token signed with the old one. Add KeyID to JoinAuthorisation and append it to the canonical claim bytes, so it is covered by the signature and cannot be repointed at a different key than the one that actually signed. Allocated servers now hold a set of currently-valid keys and select by ID: a rotation publishes the new key alongside the old, and the old is dropped once no live match can still reference it. The key file becomes a JSON map of key ID to base64 key. A file of raw key bytes is still accepted as a single key under the empty ID, which is what an unrotated deployment and the kind fixture use. Game/scripts/match_net.gd builds the canonical bytes independently, so it changes in lockstep; the cross-language golden token in test_match_net.gd is regenerated from the Go implementation and now carries a key ID. Added tests cover accepting either key mid-rotation, rejecting a retired key ID, and rejecting a token whose key ID was swapped to name a key the server does hold. Go suite and 223 Godot tests pass. |
||
|
Josh Creek
|
5453e19761 |
feat(server): add retention for idempotency, outbox and session records
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". |
||
|
Josh Creek
|
129b0c7ef0 |
fix(server): fan outbox events out to every control-plane replica
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. |
||
|
Josh Creek
|
4248e51c60 |
fix(server): enforce durable identity bans on session issuance and auth
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. |
||
|
Josh Creek
|
320ec46ba2 |
fix(server): partition and bound the Redis candidate projection
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. |
||
|
Josh Creek
|
f6a87463c5 |
fix(server): load authoritative ratings into ranked candidates
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. |
||
|
Josh Creek
|
1dd05c75f1 |
fix(server): repair the initial-connect outbox envelope and unblock dispatch
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.
|
||
|
Josh Creek
|
2c648514ba |
test(server): fix and wire up the two unrun Python suites
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.
|
||
|
Josh Creek
|
089c127cc3 |
docs: cross-reference the human-actionable backlog to GitHub issues
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. |
||
|
Josh Creek
|
b43ad207c1 |
docs(multiplayer): split spec into MULTIPLAYER_SPEC.md, trim task doc to outstanding work
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. |
||
|
Josh Creek
|
de263f30e8 |
docs: explain why the matchmaking control plane is Go, fix stale status
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. |
||
|
Josh Creek
|
ad9289fb21 |
docs(multiplayer): flag the real root blocker of the allocation pipeline
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). |
||
|
Josh Creek
|
ae4a6f937f |
fix(multiplayer): surface matchmaking connect failures to the player
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. |
||
|
Josh Creek
|
7d50612abb |
feat(multiplayer): reject outdated clients with distinct messaging
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. |
||
|
Josh Creek
|
24620be5c1 |
docs(multiplayer): correct two more stale 'remains' notes
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). |
||
|
Josh Creek
|
4c61b1e28d |
fix(multiplayer): actually connect once matchmaking assigns a match
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. |
||
|
Josh Creek
|
8810bf7d8f |
test(multiplayer): cover generic mutation retry recovery
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.
|
||
|
Josh Creek
|
91b3fc938c |
docs(multiplayer): resolve the Godot crash-attribution blocker
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. |
||
|
Josh Creek
|
79ab0d1404 |
fix(multiplayer): serve queue candidates when Redis is down, not just empty
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. |
||
|
Josh Creek
|
5190cded56 |
fix(multiplayer): stop a doomed formation from wedging the matcher
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). |
||
|
Josh Creek
|
f09ef7da8f |
test(multiplayer): cover concurrent proposal-expiry recovery race
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. |
||
|
Josh Creek
|
ce17a45afb |
feat(multiplayer): alert on workload server-mutation conflicts
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
|
817572a6ce | fix(multiplayer): size kind agones smoke resources | ||
|
Josh Creek
|
a5cbba8ac9 | fix(multiplayer): harden live registration verification | ||
|
Josh Creek
|
482d4b0985 | docs(multiplayer): record load gate verification |