fix(game): #59 — per-command rejection on PUT /api/v1/order
Tests · Go / test (push) Successful in 2m2s
Tests · Go / test (pull_request) Successful in 3m3s
Tests · Integration / integration (pull_request) Successful in 1m40s

Validation of a player's order now applies every command against a
transient game-state snapshot and records the per-command outcome
(cmdApplied, cmdErrorCode) in each command's meta. The order is
persisted even when some commands are rejected, and the response is
202 + UserGamesOrder so clients can surface the partial failure
without the chain collapsing into "downstream service is unavailable".

Pkg/error consts are reshelved onto three explicit ranges with a
package doc and helpers (IsInternalCode/IsInputCode/IsGameStateCode):
1xxx internal/server (500/501), 2xxx structural input (400), 3xxx
game-state per-command rejection (400 when escaping HTTP, otherwise
recorded as cmdErrorCode). Two pre-existing typos fixed mechanically
(ErrBeakGroupNumberNotEnough -> ErrBreakGroupNumberNotEnough,
ErrRaceExinct -> ErrRaceExtinct) along with all callsites.

Engine errorResponse maps *GenericError by shelf rather than mapping
everything to 500. The Quit-not-last structural check in
Controller.ValidateOrder is preserved and its type assertion fixed
(was a value assertion against a pointer-typed command, so the check
silently never fired).

Backend, gateway and UI are unchanged — they were already correct on
the 202 path; only the engine collapsing per-command rejection into
500 was needed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ilia Denisov
2026-05-29 09:36:29 +02:00
parent ce1dc19a29
commit af30846091
22 changed files with 517 additions and 110 deletions
+1 -1
View File
@@ -57,7 +57,7 @@ func TestFleetSend(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.FleetSend(Race_Extinct.Name, fleetSending, 2),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.FleetSend(Race_0.Name, "UnknownFleet", 2),
e.GenericErrorText(e.ErrInputEntityNotExists))
+3 -3
View File
@@ -37,7 +37,7 @@ func TestShipGroupJoinFleet(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupJoinFleet(Race_Extinct.Name, fleetOne, groupIndex),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
// ensure race has no Fleets
assert.Len(t, slices.Collect(c.ListFleets(Race_0_idx)), 0)
@@ -124,14 +124,14 @@ func TestFleetMerge(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.FleetMerge(Race_Extinct.Name, fleetSourceOne, fleetTargetTwo),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipGroupJoinFleet(UnknownRace, fleetSourceOne, c.ShipGroup(0).ID),
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupJoinFleet(Race_Extinct.Name, fleetSourceOne, c.ShipGroup(0).ID),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.NoError(t, g.ShipGroupJoinFleet(Race_0.Name, fleetSourceOne, c.ShipGroup(0).ID))
+15 -8
View File
@@ -13,17 +13,24 @@ import (
"github.com/google/uuid"
)
func (c *Controller) ValidateOrder(actor string, commands ...order.DecodableCommand) (err error) {
// ValidateOrder applies every command in the order against a transient
// view of the engine state, records the per-command outcome in each
// command's CommandMeta via applyCommand, and reports only order-level
// structural errors as the function return. Per-command rejections are
// surfaced through CommandMeta.Result so the caller can persist and
// forward them as `cmdApplied`/`cmdErrorCode` in the response body.
func (c *Controller) ValidateOrder(actor string, commands ...order.DecodableCommand) error {
for i := range commands {
if _, ok := commands[i].(order.CommandRaceQuit); ok && i != len(commands)-1 {
err = e.NewQuitCommandFollowedByCommandError()
if _, ok := commands[i].(*order.CommandRaceQuit); ok && i != len(commands)-1 {
return e.NewQuitCommandFollowedByCommandError()
}
if err != nil {
return err
}
err = errors.Join(err, c.applyCommand(actor, commands[i]))
// applyCommand never returns a non-GenericError outside of
// programmer-error panics; the per-command code, if any, is
// already recorded on the command's meta and must not abort
// validation of the remaining commands in this order.
_ = c.applyCommand(actor, commands[i])
}
return
return nil
}
func (c *Controller) applyCommand(actor string, cmd order.DecodableCommand) (err error) {
+145
View File
@@ -0,0 +1,145 @@
package controller_test
import (
"errors"
"testing"
e "galaxy/error"
"galaxy/model/order"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// TestValidateOrderRejectsCommandReferencingMissingShipClass mirrors
// the scenario reported in issue #59: an order whose only command
// builds a ship of a class that does not exist must not turn into a
// generic engine failure. The engine records the rejection in the
// command's meta and reports no order-level error so the caller can
// persist the partial result and forward it as a per-command status.
func TestValidateOrderRejectsCommandReferencingMissingShipClass(t *testing.T) {
_, ctl := newCache()
cmd := &order.CommandPlanetProduce{
CommandMeta: order.CommandMeta{
CmdID: uuid.NewString(),
CmdType: order.CommandTypePlanetProduce,
},
Number: int(R0_Planet_0_num),
Production: "SHIP",
Subject: "Nonexistent",
}
err := ctl.ValidateOrder(Race_0.Name, cmd)
assert.NoError(t, err, "per-command rejection must not become an order-level error")
require.NotNil(t, cmd.CmdApplied, "cmdApplied must be set on every processed command")
assert.False(t, *cmd.CmdApplied)
require.NotNil(t, cmd.CmdErrCode, "cmdErrorCode must be set when the command is rejected")
assert.Equal(t, e.ErrInputEntityNotExists, *cmd.CmdErrCode)
}
// TestValidateOrderContinuesAfterRejection — when one command in an
// order is rejected, every remaining command is still validated and
// receives its own per-command status. Without this property, the
// engine would silently drop the tail of an order on the first
// failure, which is exactly what produced the issue #59 symptom.
func TestValidateOrderContinuesAfterRejection(t *testing.T) {
_, ctl := newCache()
rejected := &order.CommandPlanetProduce{
CommandMeta: order.CommandMeta{
CmdID: uuid.NewString(),
CmdType: order.CommandTypePlanetProduce,
},
Number: int(R0_Planet_0_num),
Production: "SHIP",
Subject: "Nonexistent",
}
succeeding := &order.CommandPlanetRename{
CommandMeta: order.CommandMeta{
CmdID: uuid.NewString(),
CmdType: order.CommandTypePlanetRename,
},
Number: int(R0_Planet_0_num),
Name: "Homeworld",
}
err := ctl.ValidateOrder(Race_0.Name, rejected, succeeding)
assert.NoError(t, err)
require.NotNil(t, rejected.CmdApplied)
assert.False(t, *rejected.CmdApplied)
require.NotNil(t, rejected.CmdErrCode)
assert.Equal(t, e.ErrInputEntityNotExists, *rejected.CmdErrCode)
require.NotNil(t, succeeding.CmdApplied)
assert.True(t, *succeeding.CmdApplied)
require.NotNil(t, succeeding.CmdErrCode)
assert.Equal(t, 0, *succeeding.CmdErrCode)
}
// TestValidateOrderSimulatesPriorCommands — a later command may
// depend on the in-memory state mutation performed by an earlier
// command in the same order. Creating a ship class and producing a
// ship of that class in the same batch should both succeed because
// validation runs the commands against the transient state in
// submission order.
func TestValidateOrderSimulatesPriorCommands(t *testing.T) {
_, ctl := newCache()
create := &order.CommandShipClassCreate{
CommandMeta: order.CommandMeta{
CmdID: uuid.NewString(),
CmdType: order.CommandTypeShipClassCreate,
},
Name: "Drone",
Drive: 1,
}
produce := &order.CommandPlanetProduce{
CommandMeta: order.CommandMeta{
CmdID: uuid.NewString(),
CmdType: order.CommandTypePlanetProduce,
},
Number: int(R0_Planet_0_num),
Production: "SHIP",
Subject: "Drone",
}
err := ctl.ValidateOrder(Race_0.Name, create, produce)
assert.NoError(t, err)
require.NotNil(t, create.CmdApplied)
assert.True(t, *create.CmdApplied)
require.NotNil(t, produce.CmdApplied)
assert.True(t, *produce.CmdApplied)
}
// TestValidateOrderRejectsQuitFollowedByCommand — quit must be the
// last command in the order; if it is followed by another command,
// validation aborts at the order level with a structural error so
// the caller can surface HTTP 400.
func TestValidateOrderRejectsQuitFollowedByCommand(t *testing.T) {
_, ctl := newCache()
quit := &order.CommandRaceQuit{
CommandMeta: order.CommandMeta{
CmdID: uuid.NewString(),
CmdType: order.CommandTypeRaceQuit,
},
}
follow := &order.CommandRaceVote{
CommandMeta: order.CommandMeta{
CmdID: uuid.NewString(),
CmdType: order.CommandTypeRaceVote,
},
Acceptor: Race_1.Name,
}
err := ctl.ValidateOrder(Race_0.Name, quit, follow)
require.Error(t, err)
var ge *e.GenericError
require.True(t, errors.As(err, &ge), "expected GenericError")
assert.Equal(t, e.ErrInputQuitCommandFollowedByCommand, ge.Code)
}
+2 -2
View File
@@ -27,7 +27,7 @@ func TestPlanetRename(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.PlanetRename(Race_Extinct.Name, int(R0_Planet_0_num), "Home_World"),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.PlanetRename(Race_0.Name, -1, "Home_World"),
e.GenericErrorText(e.ErrInputPlanetNumber))
@@ -107,7 +107,7 @@ func TestPlanetProduce(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.PlanetProduce(Race_Extinct.Name, pn, "DRIVE", ""),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.PlanetProduce(Race_0.Name, pn, "Hyperdrive", ""),
e.GenericErrorText(e.ErrInputProductionInvalid))
+1 -1
View File
@@ -98,7 +98,7 @@ func (c *Cache) validRace(name string) (int, error) {
return -1, err
}
if c.g.Race[i].Extinct {
return -1, e.NewRaceExinctError(name)
return -1, e.NewRaceExtinctError(name)
}
return i, nil
}
+6 -6
View File
@@ -28,10 +28,10 @@ func TestRaceVote(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.RaceVote(Race_0.Name, Race_Extinct.Name),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.RaceVote(Race_Extinct.Name, Race_1.Name),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
}
func TestRaceRelation(t *testing.T) {
@@ -54,10 +54,10 @@ func TestRaceRelation(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.RaceRelation(Race_0.Name, Race_Extinct.Name, "War"),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.RaceRelation(Race_Extinct.Name, Race_0.Name, "War"),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
}
func TestRaceQuit(t *testing.T) {
@@ -69,7 +69,7 @@ func TestRaceQuit(t *testing.T) {
assert.ErrorContains(t,
g.RaceQuit(Race_Extinct.Name),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.NoError(t, g.RaceQuit(Race_0.Name))
assert.Equal(t, 3, int(c.Race(Race_0_idx).TTL))
@@ -84,7 +84,7 @@ func TestRaceID(t *testing.T) {
assert.ErrorContains(t, err, e.GenericErrorText(e.ErrInputUnknownRace))
_, err = g.RaceID(Race_Extinct.Name)
assert.ErrorContains(t, err, e.GenericErrorText(e.ErrRaceExinct))
assert.ErrorContains(t, err, e.GenericErrorText(e.ErrRaceExtinct))
id, err := g.RaceID(Race_0.Name)
assert.NoError(t, err)
+2 -2
View File
@@ -49,7 +49,7 @@ func TestPlanetRouteSet(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.PlanetRouteSet(Race_Extinct.Name, "COL", 0, 2),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.PlanetRouteSet(Race_0.Name, "IND", 0, 2),
e.GenericErrorText(e.ErrInputCargoTypeInvalid))
@@ -87,7 +87,7 @@ func TestPlanetRouteRemove(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.PlanetRouteRemove(Race_Extinct.Name, "COL", 0),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.PlanetRouteRemove(Race_0.Name, "IND", 0),
e.GenericErrorText(e.ErrInputCargoTypeInvalid))
+2 -2
View File
@@ -33,7 +33,7 @@ func TestScienceCreate(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ScienceCreate(Race_Extinct.Name, second, 0.4, 0, 0.6, 0),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ScienceCreate(Race_0.Name, BadEntityName, 0.4, 0, 0.6, 0),
e.GenericErrorText(e.ErrInputEntityTypeNameInvalid))
@@ -95,7 +95,7 @@ func TestScienceRemove(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ScienceRemove(Race_Extinct.Name, second),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ScienceRemove(Race_0.Name, first),
e.GenericErrorText(e.ErrInputEntityNotExists))
+3 -3
View File
@@ -31,7 +31,7 @@ func TestShipClassCreate(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipClassCreate(Race_Extinct.Name, "Drone", 1, 0, 0, 0, 0),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipClassCreate(Race_0.Name, BadEntityName, 1, 0, 0, 0, 0),
e.GenericErrorText(e.ErrInputEntityTypeNameInvalid))
@@ -109,7 +109,7 @@ func TestShipClassMerge(t *testing.T) {
e.GenericErrorText(e.ErrInputEntityNotExists))
assert.ErrorContains(t,
g.ShipClassMerge(Race_Extinct.Name, "Spy", "Drone"),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipClassMerge(Race_0.Name, "Spy", "Spy"),
e.GenericErrorText(e.ErrInputEntityTypeNameEquality))
@@ -134,7 +134,7 @@ func TestShipClassRemove(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipClassRemove(Race_Extinct.Name, Race_0_Freighter),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipClassRemove(Race_0.Name, "Elephant"),
e.GenericErrorText(e.ErrInputEntityNotExists))
+1 -1
View File
@@ -408,7 +408,7 @@ func (c *Cache) ShipGroupBreak(ri int, groupID, newID uuid.UUID, quantity uint)
}
if c.ShipGroup(sgi).Number < quantity {
return e.NewBeakGroupNumberNotEnoughError("%d<%d", c.ShipGroup(sgi).Number, quantity)
return e.NewBreakGroupNumberNotEnoughError("%d<%d", c.ShipGroup(sgi).Number, quantity)
}
if quantity > 0 && quantity < c.ShipGroup(sgi).Number {
@@ -33,7 +33,7 @@ func TestShipGroupSend(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupSend(Race_Extinct.Name, c.ShipGroup(0).ID, 2),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipGroupSend(Race_0.Name, uuid.New(), 2),
e.GenericErrorText(e.ErrInputEntityNotExists))
+8 -8
View File
@@ -89,7 +89,7 @@ func TestShipGroupMerge(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupMerge(Race_Extinct.Name),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.NoError(t, g.ShipGroupMerge(Race_0.Name))
@@ -139,7 +139,7 @@ func TestShipGroupBreak(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupBreak(Race_Extinct.Name, c.ShipGroup(0).ID, uuid.New(), 1),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipGroupBreak(Race_0.Name, uuid.New(), uuid.New(), 1),
e.GenericErrorText(e.ErrInputEntityNotExists))
@@ -148,7 +148,7 @@ func TestShipGroupBreak(t *testing.T) {
e.GenericErrorText(e.ErrInputNewEntityDuplicateIdentifier))
assert.ErrorContains(t,
g.ShipGroupBreak(Race_0.Name, c.ShipGroup(0).ID, uuid.New(), 17),
e.GenericErrorText(e.ErrBeakGroupNumberNotEnough))
e.GenericErrorText(e.ErrBreakGroupNumberNotEnough))
assert.ErrorContains(t,
g.ShipGroupBreak(Race_0.Name, c.ShipGroup(1).ID, uuid.New(), 1),
e.GenericErrorText(e.ErrShipsBusy))
@@ -220,10 +220,10 @@ func TestShipGroupTransfer(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupTransfer(Race_0.Name, Race_Extinct.Name, c.ShipGroup(1).ID),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipGroupTransfer(Race_Extinct.Name, Race_1.Name, c.ShipGroup(1).ID),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipGroupTransfer(Race_0.Name, Race_0.Name, c.ShipGroup(1).ID),
e.GenericErrorText(e.ErrInputSameRace))
@@ -320,7 +320,7 @@ func TestShipGroupLoad(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupLoad(Race_Extinct.Name, c.ShipGroup(0).ID, game.CargoMaterial.String(), 0),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipGroupLoad(Race_0.Name, c.ShipGroup(0).ID, "GOLD", 0),
e.GenericErrorText(e.ErrInputCargoTypeInvalid))
@@ -434,7 +434,7 @@ func TestShipGroupUnload(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupUnload(Race_Extinct.Name, c.ShipGroup(0).ID, 0),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipGroupUnload(Race_0.Name, uuid.New(), 0),
e.GenericErrorText(e.ErrInputEntityNotExists))
@@ -509,7 +509,7 @@ func TestShipGroupDismantle(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupDismantle(Race_Extinct.Name, c.ShipGroup(0).ID),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipGroupDismantle(Race_0.Name, uuid.New()),
e.GenericErrorText(e.ErrInputEntityNotExists))
@@ -125,7 +125,7 @@ func TestShipGroupUpgrade(t *testing.T) {
e.GenericErrorText(e.ErrInputUnknownRace))
assert.ErrorContains(t,
g.ShipGroupUpgrade(Race_Extinct.Name, c.ShipGroup(0).ID, "DRIVE", 0),
e.GenericErrorText(e.ErrRaceExinct))
e.GenericErrorText(e.ErrRaceExtinct))
assert.ErrorContains(t,
g.ShipGroupUpgrade(Race_0.Name, uuid.New(), "DRIVE", 0),
e.GenericErrorText(e.ErrInputEntityNotExists))