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.
This commit is contained in:
Josh Creek
2026-09-05 15:05:49 +01:00
parent a4b362cb01
commit 8b9ae35b43
4 changed files with 230 additions and 3 deletions
+47
View File
@@ -0,0 +1,47 @@
# The Go control plane is ~24k lines, and until this workflow existed the only
# Go tests CI ever ran were the two load tests in multiplayer-load.yml. Nothing
# else — domain policy, the wire/store boundaries, the allocator, the Steam
# adapter — gated a change. The Godot unit suite is covered (verify-phase6 runs
# test_runner.tscn as its first step); this closes the equivalent gap on the
# Go side.
#
# Deliberately Docker-free and cluster-free so it stays fast enough to gate
# every push. Tests that need a real PostgreSQL or Redis are behind the
# `integration` build tag and stay with their own scripts; `go vet` is still
# run over that tag so those files cannot rot uncompiled.
name: Server Unit Tests
on:
push:
pull_request:
permissions:
contents: read
jobs:
go-tests:
runs-on: ubuntu-latest
defaults:
run:
working-directory: server
steps:
- uses: actions/checkout@v4
- uses: actions/setup-go@v5
with:
go-version-file: server/go.mod
cache-dependency-path: server/go.sum
- name: Build
run: go build ./...
- name: Vet
run: go vet ./...
# Integration-tagged files are excluded from the default build, so
# without this a signature change could leave them broken until someone
# ran the integration scripts by hand.
- name: Vet integration-tagged tests
run: go vet -tags integration ./...
- name: Test
run: go test ./...
# The control plane is concurrent by design: outbox dispatchers, the
# event hub, the matcher worker and the allocator all run in parallel.
- name: Test with race detector
run: go test -race ./...
+17 -2
View File
@@ -132,7 +132,22 @@ To run one by hand, and for every config flag, see `SERVER.md`. `--smoke-force-g
### CI ### CI
`.github/workflows/` has exactly two jobs, both running the Make targets above: `dedicated-server-smoke.yml` (`make verify-phase6`) and `enet-integration.yml` (`make verify-enet-integration` inside the `enet-test` image). There is no unit-test-only workflow — `verify-phase6` runs `test_runner.tscn` as its first step. `.github/workflows/` has eight jobs, all but one running a Make target:
| Workflow | Runs | Needs |
|---|---|---|
| `server-unit-tests.yml` | `go build`/`vet`/`vet -tags integration`/`test`/`test -race` in `server/` | nothing (no Docker) |
| `dedicated-server-smoke.yml` | `make verify-phase6` | Docker, several GB |
| `enet-integration.yml` | `make verify-enet-integration` inside the `enet-test` image | Docker |
| `allocated-compose.yml` | `make verify-allocated-compose` | Docker Compose |
| `agones-integration.yml` | `make verify-kind-agones` | kind + Helm |
| `multiplayer-chaos.yml` | `make verify-chaos-recovery` | Docker |
| `multiplayer-load.yml` | `make verify-multiplayer-load` (two `-tags load` Go tests) | — |
| `supply-chain.yml` | `make verify-supply-chain` | — |
**The Godot unit suite runs via `verify-phase6`**, which invokes `test_runner.tscn` as its first step — there is no separate Godot workflow. The Go unit suite has its own workflow because until it existed the only Go tests CI ran were `multiplayer-load`'s two load tests, so ~24k lines of control plane gated nothing.
Note what is *not* in CI: `make verify-multiplayer-local` (the combined local gate, which also runs the Python manifest/contract suites) and the `integration`-tagged Go tests, which need a real PostgreSQL/Redis and live in `scripts/run_*_integration.sh`. Run those by hand before landing server changes.
### Other ### Other
@@ -148,7 +163,7 @@ The structure was deliberately chosen so an RL-trained AI opponent and, later, m
- **Scene flow**: `scenes/main_menu.tscn` (`main_menu.gd`, one handler per mode) → `free_play.tscn` (practice: no timer, R resets ball), `match.tscn` (150s timer, per-team score, kickoff resets), `spectate.tscn` (bot vs bot exhibition), `settings.tscn`, or — for online — `lobby.tscn``networked_match.tscn`. Esc returns to the menu. Canonical paths live in `scripts/scene_paths.gd`; use those constants rather than string literals. - **Scene flow**: `scenes/main_menu.tscn` (`main_menu.gd`, one handler per mode) → `free_play.tscn` (practice: no timer, R resets ball), `match.tscn` (150s timer, per-team score, kickoff resets), `spectate.tscn` (bot vs bot exhibition), `settings.tscn`, or — for online — `lobby.tscn``networked_match.tscn`. Esc returns to the menu. Canonical paths live in `scripts/scene_paths.gd`; use those constants rather than string literals.
- **Controller seam (do not bypass)**: `Ship` (`scripts/ship.gd`, `RigidBody3D`) never reads `Input`. Each physics tick, `_integrate_forces` pulls one `ShipAction` (`scripts/ship_action.gd`: thrust `Vector3`, rotation `Vector3`, turbo `bool`, each axis -1..1) from its `ShipController` child (`scripts/ship_controller.gd`, base returns a zero action). `PlayerShipController` reads input actions; `AIShipController` runs an RL policy; `RLShipController` is driven by the training bridge; `LocalNetShipController` wraps another controller to record inputs into the network timeline. A ship with no controller is inert but simulated. The ShipAction shape *is* the RL action space and *is* what the wire format quantises — change it deliberately and everywhere at once. - **Controller seam (do not bypass)**: `Ship` (`scripts/ship.gd`, `RigidBody3D`) never reads `Input`. Each physics tick, `_integrate_forces` pulls one `ShipAction` (`scripts/ship_action.gd`: thrust `Vector3`, rotation `Vector3`, turbo `bool`, each axis -1..1) from its `ShipController` child (`scripts/ship_controller.gd`, base returns a zero action). `PlayerShipController` reads input actions; `AIShipController` runs an RL policy; `RLShipController` is driven by the training bridge; `LocalNetShipController` wraps another controller to record inputs into the network timeline. A ship with no controller is inert but simulated. The ShipAction shape *is* the RL action space and *is* what the wire format quantises — change it deliberately and everywhere at once.
- **Arena vs game mode**: an arena (`scripts/arena.gd`, group `"arena"`) is a stateless stadium — a setting, an enclosing `Boundary` (instance of `objects/arena_boundary.tscn`), two `Goal` instances, `BallSpawn` and `SpawnsTeam0/1` Marker3Ds — queried via `get_ball_spawn()`/`get_ship_spawns(team)`/`get_goals()`. All arenas are a standard size: they instance the shared `arena_boundary.tscn`, and `scripts/arena_boundary.gd` (`ArenaBoundary`) holds the canonical play-volume constants (`INNER_HALF_X` 18, `INNER_HALF_Z` 27, `INNER_HEIGHT` 18, `GOAL_LINE_Z` = `INNER_HALF_Z`) that field-size logic must derive from instead of restating numbers. Game modes extend `GameMode` (`scripts/game_mode.gd`, group `"game"`): the mode's scene contains an Arena + HUD, and the mode spawns ball/ships/controllers/camera **in code** (`spawn_ship(team, index, controller)` etc.) so ship counts and controller mixes stay flexible. - **Arena vs game mode**: an arena (`scripts/arena.gd`, group `"arena"`) is a stateless stadium — a setting, an enclosing `Boundary` (instance of `objects/arena_boundary.tscn`), two `Goal` instances, `BallSpawn` and `SpawnsTeam0/1` Marker3Ds — queried via `get_ball_spawn()`/`get_ship_spawns(team)`/`get_goals()`. All arenas are a standard size: they instance the shared `arena_boundary.tscn`, and `scripts/arena_boundary.gd` (`ArenaBoundary`) holds the canonical play-volume constants (`INNER_HALF_X` 18, `INNER_HALF_Z` 27, `INNER_HEIGHT` 18, `GOAL_LINE_Z` = `INNER_HALF_Z`) that field-size logic must derive from instead of restating numbers. Game modes extend `GameMode` (`scripts/game_mode.gd`, group `"game"`): the mode's scene contains an Arena + HUD, and the mode spawns ball/ships/controllers/camera **in code** (`spawn_ship(team, index, controller)` etc.) so ship counts and controller mixes stay flexible.
- **Arena registry**: `scripts/arena_registry.gd` is the single source of truth for the arena list — three settings × floor/elevated goal variants. `"random": true` gates which arenas Match/Spectate/the dedicated server may pick; **elevated-goal variants are Free-Play-only** until a checkpoint trained on `training_elevated.tscn` is promoted, because the current bots cannot score on an elevated goal. `path_for_match(match_index, mode)` is deliberately pure arithmetic so "the server cycles arenas" is unit-testable. `arena_base.tscn` is the scenery-free physical layout the dedicated server loads (clients still render the variant `MatchSim` names). - **Arena registry**: `scripts/arena_registry.gd` is the single source of truth for the arena list — three settings × floor/elevated goal variants. `"random": true` gates which arenas Match/Spectate/the dedicated server may pick; **elevated-goal variants are Free-Play-only** until a checkpoint trained on `training_elevated.tscn` is promoted, because the current bots cannot score on an elevated goal. `path_for_match(match_index, mode)` is deliberately pure arithmetic so "the server cycles arenas" is unit-testable. **The Go control plane keeps its own copy of the ranked-eligible subset** (`server/domain/ranked.go`'s `rankedArenas`), because ranked arena selection is a server-authoritative decision made before any Godot process exists. That copy is not free to drift: `server/domain/arena_registry_sync_test.go` parses this file and fails if the two disagree in either direction, or if a ranked path has no scene behind it. Editing the arena list therefore means updating `ranked.go` too — the test says so when it fails. `arena_base.tscn` is the scenery-free physical layout the dedicated server loads (clients still render the variant `MatchSim` names).
- **Goals are dumb sensors**: `scripts/goal.gd` (`Area3D`, group `"goal"`, `@export team`) only emits `goal_scored(team)` when a body in group `"ball"` enters; `GameMode` debounces it (`_handle_goal_scored`) and modes decide consequences. Never put scoring/reset logic in the goal. - **Goals are dumb sensors**: `scripts/goal.gd` (`Area3D`, group `"goal"`, `@export team`) only emits `goal_scored(team)` when a body in group `"ball"` enters; `GameMode` debounces it (`_handle_goal_scored`) and modes decide consequences. Never put scoring/reset logic in the goal.
- **Ship physics**: all movement is force/torque-based (`_integrate_forces`), not kinematic — inputs become world-space forces/torques relative to ship orientation, with manual drag and speed clamps per tick. Physics properties (mass, inertia, friction material) live in `objects/ship.tscn`, not in `_ready` overrides — keep the scene truthful; RL tuning and the client/server parity trace depend on it. - **Ship physics**: all movement is force/torque-based (`_integrate_forces`), not kinematic — inputs become world-space forces/torques relative to ship orientation, with manual drag and speed clamps per tick. Physics properties (mass, inertia, friction material) live in `objects/ship.tscn`, not in `_ready` overrides — keep the scene truthful; RL tuning and the client/server parity trace depend on it.
- **Surface pull (wall/ceiling grav-plating)**: `ArenaBoundary.get_surface_pull()` is a wall+ceiling-only proximity force field (the floor stays plain default gravity) that `Ship` and `Ball` (`scripts/ball.gd`) each apply in their own `_integrate_forces` with independently-tuned strength/range, discovered via the `"arena_boundary"` group — enabling wall-rides and ceiling shots with no collision-shape changes. - **Surface pull (wall/ceiling grav-plating)**: `ArenaBoundary.get_surface_pull()` is a wall+ceiling-only proximity force field (the floor stays plain default gravity) that `Ship` and `Ball` (`scripts/ball.gd`) each apply in their own `_integrate_forces` with independently-tuned strength/range, discovered via the `"arena_boundary"` group — enabling wall-rides and ceiling shots with no collision-shape changes.
+1 -1
View File
@@ -215,7 +215,7 @@ are done; everything below is what's left on the tasks still open.
| 8.17 `[D:8.14,8.16]` | Proposal policy (response window, cooldowns, offender/innocent split) | Live PostgreSQL execution and allocation integration remain | | 8.17 `[D:8.14,8.16]` | Proposal policy (response window, cooldowns, offender/innocent split) | Live PostgreSQL execution and allocation integration remain |
| 8.18 `[D:8.5,8.14,8.17]` | Store layer (serializable retries, claim SQL, atomic promotion) | Allocation runtime integration remains | | 8.18 `[D:8.5,8.14,8.17]` | Store layer (serializable retries, claim SQL, atomic promotion) | Allocation runtime integration remains |
| 8.19 `[D:8.18]` | Casual lineup (26 humans, bot backfill) | Queue candidate selection, opt-in 10 s backfill proposals, reconnect/leave penalties, live integration remain | | 8.19 `[D:8.18]` | Casual lineup (26 humans, bot backfill) | Queue candidate selection, opt-in 10 s backfill proposals, reconnect/leave penalties, live integration remain |
| 8.20 `[D:8.18]` | Ranked admission (six unique verified humans) | `ArenaRegistry` integration and allocation wiring remain | | 8.20 `[D:8.18]` | Ranked admission (six unique verified humans) | Done. Allocation wiring was already complete end to end (allocator sets the `cosmic-clash.io/arena-path` annotation → `supervisor.withAllocatedCompatibility` maps it to `--arena-path``server_boot.gd``ServerMatchLoop.allocated_arena_path`), with coverage at each hop. `ArenaRegistry` integration is now a cross-language guard rather than a shared list: `server/domain/ranked.go` must keep its own ranked-eligible subset (the choice is server-authoritative and made before any Godot process exists), so `arena_registry_sync_test.go` parses `arena_registry.gd` and fails if the two disagree in either direction, if rotation order diverges, or if a ranked path has no scene behind it. Verified against four drift scenarios including promoting an elevated variant, which the registry's own comment anticipates. Live ranked admission against a real cluster remains ([#17](https://github.com/jcreek/CosmicClash/issues/17)) |
| 8.21 `[D:8.5,8.20]` | Rating core (Glicko-2, weights, transactional updates) | Live maintenance/DB execution remains | | 8.21 `[D:8.5,8.20]` | Rating core (Glicko-2, weights, transactional updates) | Live maintenance/DB execution remains |
| 8.22 `[D:8.21]` | Ranked profile (provisional games, tiers) | Persisted tier policy, client UI, reconnect transport remain | | 8.22 `[D:8.21]` | Ranked profile (provisional games, tiers) | Persisted tier policy, client UI, reconnect transport remain |
| 8.23 `[D:8.21]` | Ranked season policy (compression, rollover) | Live maintenance/DB execution remains | | 8.23 `[D:8.21]` | Ranked season policy (compression, rollover) | Live maintenance/DB execution remains |
+165
View File
@@ -0,0 +1,165 @@
package domain
import (
"os"
"path/filepath"
"regexp"
"testing"
)
// arenaRegistryPath is the Godot-side single source of truth for the arena
// list (CLAUDE.md says so explicitly). rankedArenas in ranked.go is a
// hand-maintained mirror of its floor-goal entries, and nothing has ever
// 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's
// `random` flag to true once a checkpoint trained on that geometry is
// promoted. Ranked would keep excluding it indefinitely.
// - Adding an arena leaves ranked never selecting it.
// - Renaming or removing one leaves the allocator handing out a scene path
// that no longer exists, and an allocated ranked server fails to load its
// arena at match start -- after allocation, so it burns a real match.
const arenaRegistryPath = "../../Game/scripts/arena_registry.gd"
// gameScenesDir resolves a res:// path to the checked-out scene file.
const gameScenesDir = "../../Game"
var arenaEntryPattern = regexp.MustCompile(`\{"name":\s*"([^"]*)",\s*"path":\s*"([^"]*)",\s*"random":\s*(true|false)\}`)
type registryArena struct {
Name string
Path string
Random bool
}
func parseArenaRegistry(t *testing.T) []registryArena {
t.Helper()
source, err := os.ReadFile(arenaRegistryPath)
if err != nil {
t.Fatalf("read the Godot arena registry: %v", err)
}
matches := arenaEntryPattern.FindAllStringSubmatch(string(source), -1)
arenas := make([]registryArena, 0, len(matches))
for _, match := range matches {
arenas = append(arenas, registryArena{Name: match[1], Path: match[2], Random: match[3] == "true"})
}
// Guard the guard. If the literal format changes and the pattern stops
// matching, every assertion below would pass vacuously against an empty
// list -- which is the exact failure mode this test exists to prevent.
if len(arenas) < 2 {
t.Fatalf("parsed %d arenas from %s; the entry format probably changed and this parser needs updating", len(arenas), arenaRegistryPath)
}
var eligible, ineligible int
for _, arena := range arenas {
if arena.Random {
eligible++
} else {
ineligible++
}
}
if eligible == 0 || ineligible == 0 {
t.Fatalf("parsed %d eligible and %d ineligible arenas; expected both kinds, so the `random` flag is probably not being read correctly", eligible, ineligible)
}
return arenas
}
// TestRankedArenasMatchTheGodotRegistry is the cross-language contract. It is
// the arena equivalent of the golden join-authorisation token in
// Game/tests/cases/test_match_net.gd: one side owns the truth, and this fails
// loudly when the other stops agreeing.
func TestRankedArenasMatchTheGodotRegistry(t *testing.T) {
registry := parseArenaRegistry(t)
expected := map[string]string{}
var expectedOrder []string
for _, arena := range registry {
if !arena.Random {
continue
}
expected[arena.Path] = arena.Name
expectedOrder = append(expectedOrder, arena.Path)
}
actual := map[string]string{}
for id, arena := range rankedArenas {
actual[arena.Path] = id
}
for path, name := range expected {
if _, present := actual[path]; !present {
t.Errorf("registry arena %q (%s) is ranked-eligible in Godot but missing from rankedArenas.\n"+
"If a checkpoint trained on this geometry was promoted, add it to rankedArenas and rankedArenaOrder in ranked.go.", path, name)
}
}
for path, id := range actual {
if _, present := expected[path]; !present {
t.Errorf("rankedArenas contains %q (id %q), which is not a random:true entry in %s.\n"+
"Ranked would allocate a scene the Godot registry no longer offers.", path, id, arenaRegistryPath)
}
}
// Rotation order must follow the registry's declaration order, since
// RankedArenaForProposal indexes rankedArenaOrder and callers reason about
// "the arenas, in order" across both languages.
if len(rankedArenaOrder) != len(expectedOrder) {
t.Fatalf("rankedArenaOrder has %d entries, registry has %d eligible", len(rankedArenaOrder), len(expectedOrder))
}
for index, id := range rankedArenaOrder {
arena, known := rankedArenas[id]
if !known {
t.Fatalf("rankedArenaOrder[%d] = %q, which is not a key of rankedArenas", index, id)
}
if arena.Path != expectedOrder[index] {
t.Errorf("rotation position %d is %q, registry declares %q there", index, arena.Path, expectedOrder[index])
}
}
}
// A ranked arena path is handed to an allocated server after allocation, so a
// path with no scene behind it fails at match start rather than at selection --
// burning a real match and a real server. Cheap to catch here instead.
func TestRankedArenaPathsResolveToRealScenes(t *testing.T) {
for id, arena := range rankedArenas {
relative, ok := scenePathFromRes(arena.Path)
if !ok {
t.Errorf("ranked arena %q has path %q, which is not a res:// path", id, arena.Path)
continue
}
if _, err := os.Stat(filepath.Join(gameScenesDir, relative)); err != nil {
t.Errorf("ranked arena %q points at %q, which does not exist: %v", id, arena.Path, err)
}
}
}
// Elevated-goal variants stay ranked-ineligible until a policy trained on that
// geometry is promoted; the current bots cannot score on one. Assert this
// against the registry's own flag rather than a second hardcoded list, so the
// exclusion tracks the registry instead of drifting alongside it.
func TestIneligibleRegistryArenasAreRejectedForRanked(t *testing.T) {
registry := parseArenaRegistry(t)
checked := 0
for _, arena := range registry {
if arena.Random {
continue
}
checked++
if IsRankedArenaPath(arena.Path) {
t.Errorf("%q (%s) is random:false in the Godot registry but accepted for ranked", arena.Path, arena.Name)
}
}
if checked == 0 {
t.Fatal("no ineligible arenas were checked")
}
}
func scenePathFromRes(path string) (string, bool) {
const prefix = "res://"
if len(path) <= len(prefix) || path[:len(prefix)] != prefix {
return "", false
}
return path[len(prefix):], true
}