fix(core): gate spawn intents on when they were issued, not when they land

[Celant]

Follow-up to #4657, which a player reported was still exploitable. They were right about the hole, though not about the impact.

The hole

if (!this.mg.inSpawnPhase() && player.hasSpawned()) return;   // #4657

A player who never spawned fails the second condition, so the guard never fires for them. They can sit out the spawn phase, then send a spawn intent at any later point and land on any unowned tile, with the whole map visible.

I confirmed the chain rather than just the execution:

• GameImpl's constructor calls addPlayers(), so every lobby human is in game state from tick 0 — playerByClientID resolves and a real SpawnExecution is built.
• ExecutionManager.createExec gates only on the player existing.
• spawnPlayers() runs only under isRandomSpawn(), and at init — so in a normal game nothing ever auto-spawns someone who didn't pick. They stay unspawned indefinitely.

Random-spawn games were unaffected: everyone spawns at init, so hasSpawned() is true and the old guard held.

The "multiple times" part of the report does not reproduce. Firing four intents in a single tick lands only the first — setSpawnTile() runs at the end of tick(), so the rest hit the guard. It's one late drop, not repeated teleporting. Severity is lower than reported, but the hole is real. Also worth noting getSpawnTiles filters out owned tiles, so a late spawner claims only unowned land — no territory theft.

Why not just gate on the phase

Checking inSpawnPhase() at tick time breaks legitimate spawns. New executions are initialised at the end of the tick they're queued in and first tick on the next one. I probed the boundary:

LAST-TICK  -> atInit=true   atTick=false
POST-PHASE -> atInit=false  atTick=false

So an intent sent on the final spawn-phase tick executes once the phase has ended. A blanket gate would silently drop it and the player would never spawn — which is exactly the state that enables this exploit. It would also break NationExecution, which queues in-phase and explicitly waits for a spawn that lands after the boundary (NationExecution.ts:165).

The fix

Capture the phase in init(), which still sees it for a last-tick pick but not for anything sent afterwards, and apply the gate only to executions built from a client intent:

if (this.fromIntent && !this.queuedDuringSpawnPhase) return;

Internal callers (PlayerSpawner, NationExecution) place players deliberately and legitimately land a queued spawn just past the boundary; a client may not. init() runs identically in every client's simulation, so a rejected intent stays a deterministic no-op rather than a desync — the same property #4657 relied on.

This subsumes the hasSpawned() check, which is removed: an already-spawned player's mid-game intent is now rejected because it was issued out of phase, not because of what they own. The random-spawn re-roll guard is unchanged.

Why intent-scoped rather than all human spawns

Gating every human SpawnExecution on init-time phase also works, but breaks 11 test files / 35 tests that use SpawnExecution simply to place a player after setup() has ended the phase. The vulnerability is specifically untrusted client input, and createExec is the only place an intent becomes a SpawnExecution, so scoping the gate to that trust boundary is both tighter and non-destructive. The trade-off is that a future call site forwarding an intent must set the flag — there's a test locking the current wiring.

Tests

Four added:

• spawn intent after the phase, from a player who never spawned → ignored
• repeated intents (same-tick burst + across turns) → all no-ops
• intent issued on the final spawn-phase tick → still lands on the chosen tile
• Executor marks spawn intents as client-sourced (locks the wiring)

The existing anti-teleport test now goes through the intent path, which is what it always meant to model.

I verified the tests aren't vacuous by stashing the source fix: three of the four fail against unfixed main. The last-tick test passes either way by design — it's a regression guard for the new gate, not a bug-catcher.

Full suite: 225 files, 2598 tests, all passing. tsc --noEmit, eslint and prettier clean.

To be cherry-picked into v33.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3b286226-83cf-492f-9346-46a63760579c

📥 Commits

Reviewing files that changed from the base of the PR and between ad02955 and d745651.

📒 Files selected for processing (3)

• src/core/execution/ExecutionManager.ts
• src/core/execution/SpawnExecution.ts
• tests/core/execution/SpawnExecution.test.ts

---

Walkthrough

Client-originated spawn intents now carry provenance into SpawnExecution. The execution records the queue-time spawn phase and ignores late intents while accepting intents queued on the final spawn-phase tick. Tests cover these cases and executor wiring.

Changes

Spawn phase gating

Layer / File(s)	Summary
Execution provenance and validation src/core/execution/SpawnExecution.ts, src/core/execution/ExecutionManager.ts	SpawnExecution records client intent provenance and the queue-time spawn phase. ExecutionManager marks wire-originated spawns. Late client intents are ignored.
Spawn intent behavior coverage tests/core/execution/SpawnExecution.test.ts	Tests cover late, repeated, anti-teleport, final-tick, and executor-wiring scenarios.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• openfrontio/OpenFrontIO#4428: Modifies SpawnExecution and its tests for client-originated spawn behavior.
• openfrontio/OpenFrontIO#4657: Rejects client-originated spawn intents after the spawn phase.
• openfrontio/OpenFrontIO#4660: Wires spawn-phase gating through ExecutionManager and SpawnExecution.

Suggested labels: Bugfix

Suggested reviewers: flopinguin, evanpelle

Poem

Spawn intents wait for the phase,
Late requests leave no trace.
Final ticks still pass the gate,
Tests confirm the guarded state.
Wire and execution now align.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main fix: validating spawn intents by issue time instead of execution time.
Description check	✅ Passed	The description directly explains the spawn-intent vulnerability, fix, scope, tests, and validation results.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}