SpaceCities / docs /adr /0006-command-wire-protocol.md
Almaatla's picture
docs: delivery plan, engine/command dossiers, and the ADRs they ground
ef7537c unverified
|
Raw History Blame
5.28 kB

ADR-0006: Id-based, server-stamped, tick-scheduled command protocol

Status: Accepted Date: 2026-08-30 Serves: PRD FR-8, FR-10, FR-19, NFR-8 Evidence: docs/analysis/02-command-wire-protocol.md

Context

ADR-0003 makes the server authoritative, so player intent must cross a wire as data. Today it does not: engine/commands.js takes live object references β€” issueMove(units, x, y, …) is handed unit objects, not ids. Object references do not serialize.

The audit also corrected a scope assumption this port was carrying. engine/commands.js is the whole unit-order surface, but not the whole intent surface: hudSelection.js:20-35 imports and calls roughly 20 further cost-bearing engine mutators directly β€” production queueing, research, market trades, diplomacy, colony and galaxy actions. Those are where resources are actually spent, so the protocol must cover them too. A protocol built only around issue* would have shipped with the economy wide open.

The anti-cheat surface is concrete. Without server-side ownership checks a modified client can command another player's army, spend another player's resources, and recycle their buildings β€” issueRecycle in particular performs no ownership check at all, despite a comment in engine/recycle.js:87-89 claiming it does.

Options considered

A. Change engine/commands.js to take ids

Pros. One representation everywhere; no adapter. Cons. Touches ~22 exported functions and every call site across the client, the AI and the inherited tests β€” a very large diff through the most heavily tested code in the repo, against 2,519 tests and a determinism guard, for no behavioural gain. It also maximizes divergence from upstream, directly raising the cost of every future merge (ADR-0002).

B. Keep object signatures; wrap with a codec that resolves ids server-side

Pros. The engine is untouched, so the inherited suite keeps testing exactly what it tested before, and the upstream diff stays small. All the new, risky logic β€” resolution, ownership, validation, rate limits β€” lands in one new module that can be tested adversarially in isolation. Cons. Two representations coexist (wire ids, engine objects), and the boundary between them must be the only way in, or validation can be bypassed.

Decision

Option B: wrap, do not rewrite.

net/commandCodec.js owns encode/decode/validate/apply and is the sole path from wire to engine. Its rules:

  1. Id-based envelope, versioned. Entities travel as id strings, resolved server-side.
  2. The server stamps the owner. Always. A client's claimed ownerId is advisory telemetry and is never trusted; the seat is determined by the authenticated connection. This single rule closes the command-another-player's-army and spend-another-player's-resources classes at once.
  3. Order of a selection is significant and must never be sorted. ids[0] is the formation leader (engine/commands.js:145, :195) and issueEscort derives ring slots from array index (:363). Canonicalizing the array β€” the obvious thing to do when de-duplicating input β€” would silently change formations.
  4. Deterministic application order: (applyTick, ownerIndex, clientSeq), where ownerIndex = state.owners.indexOf(owner). Commands apply immediately before tick(state, dt), never inside it.
  5. state.selection is UI-only and moves to the client session. The field remains on State as a permanently-empty array β€” removeEntity writes it (engine/state.js:347) β€” guarded by a test asserting no server module reads it.
  6. The economy surface is in scope, not just issue*.

Exactly one engine signature changes: issueSetRally(building, …) β†’ issueSetRally(state, buildingId, …), 3 call sites. It is the one command whose only handle on its target is an object reference with no id-based route, so wrapping it would mean inventing a lookup the engine already owns.

Consequences

Gains. The engine keeps its shape, so the inherited suite keeps its meaning and upstream merges stay cheap. Every new security-relevant decision lives in one file with one entry point. A match becomes replayable from {engineCommit, createGameStateOpts, dt, aiSeatConfigs, orderedCommandLog} (FR-19).

Costs. The codec is a chokepoint that must not be bypassed β€” any future code calling issue* directly on the server re-opens everything the codec closes. This needs a guard test in the style of test/engine-purity.test.js, which the repo already uses for exactly this kind of invariant.

Follow-on work. Fix the defects this audit surfaced before the codec ships β€” in particular issueRecycle's missing ownership check, the false comment documenting it, and friendly-fire on explicit attack orders (engine/combat.js:46). Fix the formation gate (see ADR-0008). Write the adversarial codec test suite: cross-seat commands, unaffordable commands, unknown ids, malformed envelopes, flooding.

Revisit if. The two-representation boundary proves leaky in practice β€” at which point option A becomes worth its cost, as a deliberate migration rather than a drift.