From af30846091d0d9ab7cd146c05d9ef6c76855c0f1 Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Fri, 29 May 2026 09:36:29 +0200 Subject: [PATCH 1/4] =?UTF-8?q?fix(game):=20#59=20=E2=80=94=20per-command?= =?UTF-8?q?=20rejection=20on=20PUT=20/api/v1/order?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- docs/FUNCTIONAL.md | 14 ++ docs/FUNCTIONAL_ru.md | 14 ++ game/internal/controller/fleet_send_test.go | 2 +- game/internal/controller/fleet_test.go | 6 +- game/internal/controller/order.go | 23 ++- game/internal/controller/order_test.go | 145 +++++++++++++++ game/internal/controller/planet_test.go | 4 +- game/internal/controller/race.go | 2 +- game/internal/controller/race_test.go | 12 +- game/internal/controller/route_test.go | 4 +- game/internal/controller/science_test.go | 4 +- game/internal/controller/ship_class_test.go | 6 +- game/internal/controller/ship_group.go | 2 +- .../controller/ship_group_send_test.go | 2 +- game/internal/controller/ship_group_test.go | 16 +- .../controller/ship_group_upgrade_test.go | 2 +- game/internal/router/handler/handler.go | 18 +- game/internal/router/order_test.go | 123 +++++++++++++ game/openapi.yaml | 55 ++++-- pkg/error/generic.go | 165 ++++++++++++------ pkg/error/input.go | 4 +- pkg/error/state.go | 4 +- 22 files changed, 517 insertions(+), 110 deletions(-) create mode 100644 game/internal/controller/order_test.go diff --git a/docs/FUNCTIONAL.md b/docs/FUNCTIONAL.md index 06867dd..b9c46c4 100644 --- a/docs/FUNCTIONAL.md +++ b/docs/FUNCTIONAL.md @@ -648,6 +648,20 @@ validity and ordering of in-game decisions. Gateway needs to know the typed FB shape only to transcode the wire format; the per-command semantics live in the engine. +For `user.games.order` specifically, the engine validates every +command in the submitted order against a transient view of the +current game state and reports the outcome per command on each +command's meta (`cmdApplied`, `cmdErrorCode`) inside the same +`UserGamesOrder` body. The order is persisted with these per-command +verdicts even when some commands are rejected — for example, deleting +the "create ship class X" command from an order that still contains +"produce ship X" makes the second command fail with a per-command +`cmdErrorCode` for "entity does not exist", while the rest of the +order remains stored and the response is still a `202 Accepted`. A +`400` is returned only for order-level structural rejections +(`quit` not the last command, unrecognized command type, malformed +input); `500` only for genuine engine-internal failures. + ### 6.3 Turn cutoff and auto-pause A running game continuously alternates between a command-accepting diff --git a/docs/FUNCTIONAL_ru.md b/docs/FUNCTIONAL_ru.md index ae233f3..e93ff7b 100644 --- a/docs/FUNCTIONAL_ru.md +++ b/docs/FUNCTIONAL_ru.md @@ -666,6 +666,20 @@ Backend не парсит содержимое payload команд или пр FB-форму только чтобы транскодировать wire-формат; per-command- семантика живёт в движке. +Специально для `user.games.order` движок валидирует каждую команду +приказа на транзиентном слепке текущего состояния игры и записывает +итог по каждой команде в её мету (`cmdApplied`, `cmdErrorCode`) в +том же ответе `UserGamesOrder`. Приказ сохраняется с этими +per-command-вердиктами даже если часть команд была отклонена — +например, удаление команды «создать класс корабля X» из приказа, +в котором остаётся «строить X», приводит к тому, что вторая команда +возвращается с `cmdErrorCode` «сущность не существует», а остальные +команды приказа остаются сохранёнными, и ответ остаётся +`202 Accepted`. `400` возвращается только для структурных отказов +на уровне приказа (`quit` не последняя команда, неизвестный +command type, малформированный вход); `500` — только для реальных +внутренних сбоев движка. + ### 6.3 Окно хода и auto-pause Запущенная игра постоянно чередуется между окном приёма команд diff --git a/game/internal/controller/fleet_send_test.go b/game/internal/controller/fleet_send_test.go index a02cb64..66ec268 100644 --- a/game/internal/controller/fleet_send_test.go +++ b/game/internal/controller/fleet_send_test.go @@ -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)) diff --git a/game/internal/controller/fleet_test.go b/game/internal/controller/fleet_test.go index a5df3aa..615ffdd 100644 --- a/game/internal/controller/fleet_test.go +++ b/game/internal/controller/fleet_test.go @@ -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)) diff --git a/game/internal/controller/order.go b/game/internal/controller/order.go index fe39c22..e695708 100644 --- a/game/internal/controller/order.go +++ b/game/internal/controller/order.go @@ -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) { diff --git a/game/internal/controller/order_test.go b/game/internal/controller/order_test.go new file mode 100644 index 0000000..92e6c9e --- /dev/null +++ b/game/internal/controller/order_test.go @@ -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) +} diff --git a/game/internal/controller/planet_test.go b/game/internal/controller/planet_test.go index 141e152..ae2cdf0 100644 --- a/game/internal/controller/planet_test.go +++ b/game/internal/controller/planet_test.go @@ -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)) diff --git a/game/internal/controller/race.go b/game/internal/controller/race.go index 20eb0b4..c4d35c1 100644 --- a/game/internal/controller/race.go +++ b/game/internal/controller/race.go @@ -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 } diff --git a/game/internal/controller/race_test.go b/game/internal/controller/race_test.go index 388588b..043ac1b 100644 --- a/game/internal/controller/race_test.go +++ b/game/internal/controller/race_test.go @@ -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) diff --git a/game/internal/controller/route_test.go b/game/internal/controller/route_test.go index 6adf9d8..58707d6 100644 --- a/game/internal/controller/route_test.go +++ b/game/internal/controller/route_test.go @@ -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)) diff --git a/game/internal/controller/science_test.go b/game/internal/controller/science_test.go index bf88619..27db5d9 100644 --- a/game/internal/controller/science_test.go +++ b/game/internal/controller/science_test.go @@ -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)) diff --git a/game/internal/controller/ship_class_test.go b/game/internal/controller/ship_class_test.go index e04f165..5c7d94b 100644 --- a/game/internal/controller/ship_class_test.go +++ b/game/internal/controller/ship_class_test.go @@ -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)) diff --git a/game/internal/controller/ship_group.go b/game/internal/controller/ship_group.go index 66e1722..126bf13 100644 --- a/game/internal/controller/ship_group.go +++ b/game/internal/controller/ship_group.go @@ -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 { diff --git a/game/internal/controller/ship_group_send_test.go b/game/internal/controller/ship_group_send_test.go index 70275ba..1a71332 100644 --- a/game/internal/controller/ship_group_send_test.go +++ b/game/internal/controller/ship_group_send_test.go @@ -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)) diff --git a/game/internal/controller/ship_group_test.go b/game/internal/controller/ship_group_test.go index 44ecef7..0ef88fe 100644 --- a/game/internal/controller/ship_group_test.go +++ b/game/internal/controller/ship_group_test.go @@ -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)) diff --git a/game/internal/controller/ship_group_upgrade_test.go b/game/internal/controller/ship_group_upgrade_test.go index df885fc..c30c6be 100644 --- a/game/internal/controller/ship_group_upgrade_test.go +++ b/game/internal/controller/ship_group_upgrade_test.go @@ -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)) diff --git a/game/internal/router/handler/handler.go b/game/internal/router/handler/handler.go index 4be6c8b..62ccfdc 100644 --- a/game/internal/router/handler/handler.go +++ b/game/internal/router/handler/handler.go @@ -142,6 +142,18 @@ func stateResponse(s game.State) rest.StateResponse { return *result } +// errorResponse renders err onto c and reports whether the caller +// should stop further processing. The HTTP status is selected by the +// GenericError shelf (see pkg/error for the taxonomy): +// +// - validator.ValidationErrors (request struct binding) → 400 with +// {"error": ...}. +// - GenericError, ErrGameNotInitialized → 501 with no body. +// - GenericError on the internal shelf (1xxx) → 500 with +// {"generic_error", "code"}. +// - GenericError on the input-validation shelf (2xxx) or the +// game-state shelf (3xxx) → 400 with {"generic_error", "code"}. +// - everything else (non-GenericError) → 500 with {"error": ...}. func errorResponse(c *gin.Context, err error) bool { if err == nil { return false @@ -153,9 +165,11 @@ func errorResponse(c *gin.Context, err error) bool { } if ge, ok := errors.AsType[*e.GenericError](err); ok { - switch ge.Code { - case e.ErrGameNotInitialized: + switch { + case ge.Code == e.ErrGameNotInitialized: c.Status(http.StatusNotImplemented) + case e.IsInputCode(ge.Code), e.IsGameStateCode(ge.Code): + c.JSON(http.StatusBadRequest, gin.H{"generic_error": ge.Error(), "code": ge.Code}) default: c.JSON(http.StatusInternalServerError, gin.H{"generic_error": ge.Error(), "code": ge.Code}) } diff --git a/game/internal/router/order_test.go b/game/internal/router/order_test.go index 5f56d35..1aad10c 100644 --- a/game/internal/router/order_test.go +++ b/game/internal/router/order_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "testing" + e "galaxy/error" "galaxy/model/order" "galaxy/model/rest" @@ -1016,6 +1017,128 @@ func TestPutOrderEngineError(t *testing.T) { assert.Equal(t, http.StatusInternalServerError, w.Code, w.Body) } +// TestPutOrderPerCommandRejection — when the engine returns an order +// where some commands carry `cmdErrorCode != 0`, the handler must +// still answer 202 with the full UserGamesOrder body so the client +// can surface the per-command failure (rather than treating the +// response as a generic error). +func TestPutOrderPerCommandRejection(t *testing.T) { + applied := true + rejectedFlag := false + rejectedCode := e.ErrInputEntityNotExists + zero := 0 + result := &order.UserGamesOrder{ + GameID: uuid.New(), + UpdatedAt: 4242, + Commands: []order.DecodableCommand{ + &order.CommandShipClassCreate{ + CommandMeta: order.CommandMeta{ + CmdID: id(), + CmdType: order.CommandTypeShipClassCreate, + CmdApplied: &applied, + CmdErrCode: &zero, + }, + Name: "Drone", + Drive: 1, + }, + &order.CommandPlanetProduce{ + CommandMeta: order.CommandMeta{ + CmdID: id(), + CmdType: order.CommandTypePlanetProduce, + CmdApplied: &rejectedFlag, + CmdErrCode: &rejectedCode, + }, + Number: 0, + Production: "SHIP", + Subject: "Nonexistent", + }, + }, + } + executor := &dummyExecutor{ValidateOrderResult: result} + r := setupRouterExecutor(executor) + + payload := &rest.Command{ + Actor: commandDefaultActor, + Commands: []json.RawMessage{ + encodeCommand(&order.CommandShipClassCreate{ + CommandMeta: order.CommandMeta{CmdID: id(), CmdType: order.CommandTypeShipClassCreate}, + Name: "Drone", + Drive: 1, + }), + encodeCommand(&order.CommandPlanetProduce{ + CommandMeta: order.CommandMeta{CmdID: id(), CmdType: order.CommandTypePlanetProduce}, + Number: 0, + Production: "SHIP", + Subject: "Nonexistent", + }), + }, + } + + w := httptest.NewRecorder() + req, _ := http.NewRequest(apiCommandMethod, apiOrderPath, asBody(payload)) + r.ServeHTTP(w, req) + + require.Equal(t, http.StatusAccepted, w.Code, w.Body) + + var got struct { + GameID uuid.UUID `json:"game_id"` + UpdatedAt int64 `json:"updatedAt"` + Commands []json.RawMessage `json:"cmd"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &got)) + require.Len(t, got.Commands, 2) + + var first struct { + CmdApplied *bool `json:"cmdApplied"` + CmdErrCode *int `json:"cmdErrorCode"` + } + require.NoError(t, json.Unmarshal(got.Commands[0], &first)) + require.NotNil(t, first.CmdApplied) + assert.True(t, *first.CmdApplied) + + var second struct { + CmdApplied *bool `json:"cmdApplied"` + CmdErrCode *int `json:"cmdErrorCode"` + } + require.NoError(t, json.Unmarshal(got.Commands[1], &second)) + require.NotNil(t, second.CmdApplied) + assert.False(t, *second.CmdApplied) + require.NotNil(t, second.CmdErrCode) + assert.Equal(t, e.ErrInputEntityNotExists, *second.CmdErrCode) +} + +// TestPutOrderStructuralRejection — order-level structural errors +// (e.g. `quit` not the last command) come back from the executor as a +// *GenericError on the input shelf, which must map to HTTP 400 with +// the `{"generic_error","code"}` envelope rather than 500. +func TestPutOrderStructuralRejection(t *testing.T) { + executor := &dummyExecutor{ValidateOrderErr: e.NewQuitCommandFollowedByCommandError()} + r := setupRouterExecutor(executor) + + payload := &rest.Command{ + Actor: commandDefaultActor, + Commands: []json.RawMessage{ + encodeCommand(&order.CommandRaceQuit{ + CommandMeta: order.CommandMeta{CmdID: id(), CmdType: order.CommandTypeRaceQuit}, + }), + }, + } + + w := httptest.NewRecorder() + req, _ := http.NewRequest(apiCommandMethod, apiOrderPath, asBody(payload)) + r.ServeHTTP(w, req) + + require.Equal(t, http.StatusBadRequest, w.Code, w.Body) + + var got struct { + Code int `json:"code"` + Msg string `json:"generic_error"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &got)) + assert.Equal(t, e.ErrInputQuitCommandFollowedByCommand, got.Code) + assert.NotEmpty(t, got.Msg) +} + func TestGetOrderQueryValidation(t *testing.T) { for _, tc := range []struct { description string diff --git a/game/openapi.yaml b/game/openapi.yaml index 1ba3cb1..b875b5b 100644 --- a/game/openapi.yaml +++ b/game/openapi.yaml @@ -16,9 +16,19 @@ info: `504 Gateway Timeout` - `501 Not Implemented` is returned without a body when the game has not been initialized - - validation errors return `400` with `{"error": "message"}` - - game-engine errors return `500` with `{"generic_error": "message", "code": integer}` - - other internal errors return `500` with `{"error": "message"}` + - request-binding validation errors return `400` with `{"error": "message"}` + - structural input errors and game-state rejections that escape to + HTTP return `400` with `{"generic_error": "message", "code": integer}`; + the `code` carries the engine's `GenericError` code (see + `pkg/error/generic.go` — shelf `2xxx` for structural input and + `3xxx` for game-state rejection) + - on `PUT /api/v1/order`, game-state rejections do not become HTTP + errors; the engine returns `202 Accepted` with the full + `UserGamesOrder` body and reports the failure on the offending + command via `cmdApplied=false` and `cmdErrorCode=` + - internal engine failures return `500` with + `{"generic_error": "message", "code": integer}` (code on shelf `1xxx`) + or `{"error": "message"}` for unclassified failures servers: - url: http://localhost:8080 description: Default local listener for Game Service. @@ -161,10 +171,22 @@ paths: operationId: validateOrder summary: Validate and store a player order without executing it description: | - Validates and stores the game commands structurally without executing them. - On success returns `202 Accepted` with the stored order, including the - engine-assigned `updatedAt` timestamp used by clients to detect stale - submissions. + Validates and stores the game commands without executing them. The + engine applies each command in submission order against a transient + view of the game state, records the per-command outcome on the + command's meta, and persists the resulting `UserGamesOrder` so + clients can reload the same per-command verdict via + `GET /api/v1/order`. + + On success returns `202 Accepted` with the stored order; the + engine-assigned `updatedAt` timestamp is used by clients to detect + stale submissions. Game-state rejections (e.g. a "produce ship of + class X" command after class X was removed) are reported per + command via `cmdApplied=false` and `cmdErrorCode=` inside + the same `202` response — they do **not** become a `400` or `500` + on the whole order. Order-level structural rejections (e.g. a + `quit` command that is not the last command in the order) return + `400`. requestBody: required: true content: @@ -173,7 +195,10 @@ paths: $ref: "#/components/schemas/CommandRequest" responses: "202": - description: Order is structurally valid and stored. + description: | + Order is stored. Each entry of `cmd` carries `cmdApplied` and + `cmdErrorCode` describing the per-command outcome; the order + is considered stored even when some commands were rejected. content: application/json: schema: @@ -481,10 +506,20 @@ components: description: Unique command identifier (RFC 4122 UUID). cmdApplied: type: boolean - description: Set in command-result responses; true when the command was applied. + description: | + Per-command outcome. Set by the engine in every response that + carries a stored or freshly-validated order (`PUT /api/v1/order` + and `GET /api/v1/order`): `true` when the command was applied + against the engine state during validation or turn generation, + `false` when it was rejected (see `cmdErrorCode`). Omitted on + requests. cmdErrorCode: type: integer - description: Set in command-result responses; non-zero when the command was rejected. + description: | + Per-command error code. Set by the engine alongside `cmdApplied`: + `0` when the command was applied, a non-zero `GenericError` + code (shelves `2xxx`/`3xxx` in `pkg/error/generic.go`) when + the command was rejected. Omitted on requests. CommandType: type: string description: Discriminator identifying the game command variant carried in a `cmd` element. diff --git a/pkg/error/generic.go b/pkg/error/generic.go index f619fd2..b38337d 100644 --- a/pkg/error/generic.go +++ b/pkg/error/generic.go @@ -1,71 +1,124 @@ +// Package error defines the engine's error taxonomy used both as a wire +// contract over the engine REST API and as an internal Go error type. +// +// Codes are organised onto three semantic shelves. The high digit +// of each code identifies the shelf; the corresponding HTTP status +// mapping is enforced by router-level handlers (see +// game/internal/router/handler.errorResponse). +// +// Shelf 1xxx (internal / server) +// Engine infrastructure failures: storage, uninitialised game, +// invalid persisted state, missing report. HTTP 500, except +// ErrGameNotInitialized → 501. +// +// Shelf 2xxx (input validation, structural) +// Per-request structural rejections that can be decided without +// inspecting game state: enum mismatches, numeric ranges, +// cross-field shape rules ("ammunition without weapons"), and +// order-level structure ("quit must be the last command"). HTTP +// 400. +// +// Shelf 3xxx (game-state, per-command rejection) +// Game-state runtime rejections that depend on the current state +// snapshot: entity-not-found, not-owned, in-use, ships-busy, +// insufficient resources, send/upgrade/cargo dependencies. These +// surface as per-command `cmdErrorCode` on PUT /api/v1/order +// (and only escape as HTTP 400 from PUT /api/v1/command). +// +// Code 0 represents "applied without error" and is reserved as the +// successful per-command outcome on CommandMeta.Result. Code -1 +// (ErrDummy) is reserved for test fixtures. package error import ( "fmt" ) +// Shelf 1xxx — internal / server errors. const ( - ErrStorageFailure int = 1000 + iota - ErrGameNotInitialized - ErrGameStateInvalid - ErrReportNotFound + ErrStorageFailure int = 1001 + ErrGameNotInitialized int = 1002 + ErrGameStateInvalid int = 1003 + ErrReportNotFound int = 1004 ) +// Shelf 2xxx — structural input validation. const ( - ErrDummy int = -1 - - ErrDeleteShipTypeExistingGroup = 5000 - ErrDeleteShipTypePlanetProduction = 5001 - ErrDeleteSciencePlanetProduction = 5002 - ErrMergeShipTypeNotEqual = 5003 - ErrBeakGroupNumberNotEnough = 5005 - ErrEntityInUse = 5006 - ErrShipsBusy = 5007 - ErrShipsNotOnSamePlanet = 5008 - ErrUpgradeGroupNumberNotEnough = 5010 - ErrUpgradeInsufficientResources = 5011 - ErrSendShipHasNoDrives = 5012 - ErrSendUnreachableDestination = 5013 - ErrSendShipOwnerHasNoPlanets = 5014 - ErrRaceExinct = 5015 + ErrInputUnknownRelation int = 2001 + ErrInputSameRace int = 2002 + ErrInputEntityTypeNameInvalid int = 2003 + ErrInputEntityTypeNameEquality int = 2004 + ErrInputPlanetNumber int = 2005 + ErrInputDriveValue int = 2006 + ErrInputWeaponsValue int = 2007 + ErrInputShieldsValue int = 2008 + ErrInputCargoValue int = 2009 + ErrInputShipTypeArmamentValue int = 2010 + ErrInputShipTypeWeaponsAndArmamentValue int = 2011 + ErrInputShipTypeZeroValues int = 2012 + ErrInputScienceSumValues int = 2013 + ErrInputProductionInvalid int = 2014 + ErrInputCargoTypeInvalid int = 2015 + ErrInputBreakGroupIllegalNumber int = 2016 + ErrInputTechUnknown int = 2017 + ErrInputTechInvalidMixing int = 2018 + ErrInputUpgradeParameterNotAllowed int = 2019 + ErrInputQuitCommandFollowedByCommand int = 2020 + ErrInputUnrecognizedCommand int = 2021 ) +// Shelf 3xxx — game-state runtime errors (per-command rejection). +// +// Several constants here retain a historical "Input" prefix in their +// names although the underlying check needs the live game state; the +// shelf is the authoritative classification and supersedes the prefix. const ( - ErrInputUnknownRace int = 3000 + iota - ErrInputUnknownRelation - ErrInputSameRace - ErrInputEntityTypeNameInvalid - ErrInputNewEntityDuplicateIdentifier - ErrInputEntityTypeNameEquality - ErrInputEntityNotExists - ErrInputEntityNotOwned - ErrInputPlanetNumber - ErrInputDriveValue - ErrInputWeaponsValue - ErrInputShieldsValue - ErrInputCargoValue - ErrInputShipTypeArmamentValue - ErrInputShipTypeWeaponsAndArmamentValue - ErrInputShipTypeZeroValues - ErrInputScienceSumValues - ErrInputProductionInvalid - ErrInputCargoTypeInvalid - ErrInputCargoLoadNotEnough - ErrInputCargoLoadNotEqual - ErrInputNoCargoBay - ErrInputCargoLoadNoSpaceLeft - ErrInputCargoUnloadEmpty - ErrInputBreakGroupIllegalNumber - ErrInputTechUnknown - ErrInputTechInvalidMixing - ErrInputUpgradeShipTechNotUsed - ErrInputUpgradeParameterNotAllowed - ErrInputUpgradeShipsAlreadyUpToDate - ErrInputUpgradeTechLevelInsufficient - ErrInputQuitCommandFollowedByCommand - ErrInputUnrecognizedCommand + ErrInputUnknownRace int = 3001 + ErrInputNewEntityDuplicateIdentifier int = 3002 + ErrInputEntityNotExists int = 3003 + ErrInputEntityNotOwned int = 3004 + ErrEntityInUse int = 3005 + ErrRaceExtinct int = 3006 + ErrShipsBusy int = 3007 + ErrShipsNotOnSamePlanet int = 3008 + ErrDeleteShipTypeExistingGroup int = 3009 + ErrDeleteShipTypePlanetProduction int = 3010 + ErrDeleteSciencePlanetProduction int = 3011 + ErrMergeShipTypeNotEqual int = 3012 + ErrBreakGroupNumberNotEnough int = 3013 + ErrInputCargoLoadNotEnough int = 3014 + ErrInputCargoLoadNotEqual int = 3015 + ErrInputNoCargoBay int = 3016 + ErrInputCargoLoadNoSpaceLeft int = 3017 + ErrInputCargoUnloadEmpty int = 3018 + ErrInputUpgradeShipTechNotUsed int = 3019 + ErrInputUpgradeShipsAlreadyUpToDate int = 3020 + ErrUpgradeGroupNumberNotEnough int = 3021 + ErrUpgradeInsufficientResources int = 3022 + ErrInputUpgradeTechLevelInsufficient int = 3023 + ErrSendShipHasNoDrives int = 3024 + ErrSendUnreachableDestination int = 3025 + ErrSendShipOwnerHasNoPlanets int = 3026 ) +// ErrDummy is reserved for test fixtures; production code never uses it. +const ErrDummy int = -1 + +// IsInternalCode reports whether code belongs to the internal / server +// shelf (1xxx). Internal errors map to HTTP 500 (ErrGameNotInitialized +// is special-cased to 501). +func IsInternalCode(code int) bool { return code >= 1000 && code < 2000 } + +// IsInputCode reports whether code belongs to the structural input +// validation shelf (2xxx). Input errors map to HTTP 400. +func IsInputCode(code int) bool { return code >= 2000 && code < 3000 } + +// IsGameStateCode reports whether code belongs to the game-state / +// per-command rejection shelf (3xxx). On PUT /api/v1/order these are +// recorded into CommandMeta.CmdErrCode; on PUT /api/v1/command they +// map to HTTP 400. +func IsGameStateCode(code int) bool { return code >= 3000 && code < 4000 } + func GenericErrorText(code int) string { switch code { case ErrDummy: @@ -76,6 +129,8 @@ func GenericErrorText(code int) string { return "Game not yet initialized" case ErrGameStateInvalid: return "Invalid game state" + case ErrReportNotFound: + return "Report not found" case ErrInputUnknownRace: return "Race name is unknown to this game" case ErrInputUnknownRelation: @@ -136,7 +191,7 @@ func GenericErrorText(code int) string { return "Illegal ships number to make new group" case ErrMergeShipTypeNotEqual: return "Source and target ship types are not the same" - case ErrBeakGroupNumberNotEnough: + case ErrBreakGroupNumberNotEnough: return "Not enough ships in the group to make a separate group" case ErrShipsBusy: return "Ship(s) are'n free to use" @@ -168,7 +223,7 @@ func GenericErrorText(code int) string { return "Destination planet is too far for current Drive level" case ErrSendShipOwnerHasNoPlanets: return "Race is not owning any planet, all flights impossible" - case ErrRaceExinct: + case ErrRaceExtinct: return "Race is extinct" default: return fmt.Sprintf("Undescribed error with code %d", code) diff --git a/pkg/error/input.go b/pkg/error/input.go index 960aa67..31bdd40 100644 --- a/pkg/error/input.go +++ b/pkg/error/input.go @@ -108,8 +108,8 @@ func NewMergeShipTypeNotEqualError(arg ...any) error { return newGenericError(ErrMergeShipTypeNotEqual, arg...) } -func NewBeakGroupNumberNotEnoughError(arg ...any) error { - return newGenericError(ErrBeakGroupNumberNotEnough, arg...) +func NewBreakGroupNumberNotEnoughError(arg ...any) error { + return newGenericError(ErrBreakGroupNumberNotEnough, arg...) } func NewShipsBusyError(arg ...any) error { diff --git a/pkg/error/state.go b/pkg/error/state.go index f5d39c1..d0c9bdc 100644 --- a/pkg/error/state.go +++ b/pkg/error/state.go @@ -1,7 +1,7 @@ package error -func NewRaceExinctError(arg ...any) error { - return newGenericError(ErrRaceExinct, arg...) +func NewRaceExtinctError(arg ...any) error { + return newGenericError(ErrRaceExtinct, arg...) } func NewGameNotInitializedError(arg ...any) error { From e038ea61543bb0995ed8288203fab5411f0ebf16 Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Fri, 29 May 2026 10:47:25 +0200 Subject: [PATCH 2/4] fix(dev-deploy): recycle engine containers on galaxy-engine:dev SHA drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `backend`'s reconciler adopts pre-existing `galaxy-game-*` containers without comparing their image SHA against the freshly-built `galaxy-engine:dev`, so a long-lived sandbox would otherwise keep serving the previous engine code after a redeploy. Issue #59 surfaced this: after the per-command-rejection fix was deployed via `workflow_dispatch`, the running sandbox container was still on the old image SHA and the browser kept seeing the 503/unavailable response. Adds a `Recycle engine containers on image drift` step right before `Reap stray dev-deploy containers`. The step compares the new `galaxy-engine:dev` SHA against every running `galaxy-game-*` container and, on drift, stops the backend, removes the container, wipes the bind-mounted per-game state directory (Engine.Init() writes turn-0 over any pre-existing `turn-N` files — silent state corruption otherwise), and cascade-deletes the lobby `games` row. The `dev-sandbox` bootstrap on the next backend boot finds no live sandbox and provisions a fresh one on the new engine image. When the engine sources are unchanged, the BuildKit cache hits and the SHA stays the same — the recycle step is a no-op and the running games keep their state across the deploy. Verified end-to-end against the live dev environment. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitea/workflows/dev-deploy.yaml | 72 ++++++++++++++++++++++++++++++-- tools/dev-deploy/README.md | 23 ++++++++++ 2 files changed, 92 insertions(+), 3 deletions(-) diff --git a/.gitea/workflows/dev-deploy.yaml b/.gitea/workflows/dev-deploy.yaml index 5df15df..2a2ef6b 100644 --- a/.gitea/workflows/dev-deploy.yaml +++ b/.gitea/workflows/dev-deploy.yaml @@ -148,14 +148,80 @@ jobs: -v "${{ gitea.workspace }}/pkg/geoip/test-data/test-data:/src:ro" \ alpine sh -c 'cp /src/GeoIP2-Country-Test.mmdb /dst/geoip.mmdb' + - name: Recycle engine containers on image drift + run: | + # Compare the freshly-built `galaxy-engine:dev` SHA against + # every running `galaxy-game-*` container. The backend + # reconciler adopts pre-existing labelled engine containers + # without checking image drift, so a running sandbox would + # otherwise keep serving the previous engine code until the + # container is recycled by hand. This step makes the recycle + # automatic but only when it is actually needed: + # + # * BuildKit cache hit on the `Build galaxy-engine image` + # step → `galaxy-engine:dev` keeps its previous SHA → + # no drift → no-op (no engine source change to deploy). + # * engine source change → fresh SHA → for each drifted + # container we stop the backend, remove the container, + # wipe its bind-mounted state directory (Engine.Init() + # writes turn-0 over any pre-existing `turn-N` files — + # silent state corruption otherwise), and cascade-delete + # the lobby `games` row (the FKs in `00001_init.sql` + # drop the matching `runtime_records`, `memberships`, + # `player_mappings`, etc. in the same write). The + # `dev-sandbox` bootstrap on the next backend boot finds + # no live sandbox and provisions a fresh one on the new + # engine image. + # + # Backend is stopped first to keep the reconciler from + # racing the recycle (mid-stream adoption / restart). The + # subsequent `Bring up the stack` step restarts it. + set -u + new_sha=$(docker image inspect galaxy-engine:dev --format '{{.Id}}') + echo "fresh galaxy-engine:dev = $new_sha" + + drift=() + for c in $(docker ps --filter "name=galaxy-game-" --format '{{.Names}}'); do + cur=$(docker inspect "$c" --format '{{.Image}}') + if [ "$cur" != "$new_sha" ]; then + drift+=("${c#galaxy-game-}") + echo " drift: $c was on $cur" + else + echo " match: $c" + fi + done + if [ ${#drift[@]} -eq 0 ]; then + echo "no drift detected — recycle skipped" + else + docker stop -t 30 galaxy-dev-backend >/dev/null 2>&1 || true + state_root="$HOME/.galaxy-dev/game-state" + for gid in "${drift[@]}"; do + echo "recycling $gid" + docker rm -f "galaxy-game-$gid" >/dev/null 2>&1 || true + # Wipe the per-game state dir as root inside a throwaway + # container so we can remove files left behind by the + # engine container even when its uid differs from the + # runner's. + docker run --rm -v "$state_root:/state" alpine \ + sh -c "rm -rf -- /state/$gid" + done + ids_csv=$(printf "'%s'," "${drift[@]}") + ids_csv=${ids_csv%,} + docker exec galaxy-dev-postgres psql -v ON_ERROR_STOP=1 \ + -U galaxy -d galaxy_backend \ + -c "DELETE FROM backend.games WHERE game_id IN (${ids_csv});" + fi + - name: Reap stray dev-deploy containers run: | # Remove any non-running compose-managed containers from # earlier deploys before `compose up`. Filter by the stack # label so we never touch unrelated workloads on the same - # daemon. Running containers (incl. engine instances backend - # spawned itself with the same label) are left intact — - # those are reattached by the backend reconciler on boot. + # daemon. Running engine containers spawned by backend with + # the same label are left intact when their image SHA still + # matches the freshly-built `galaxy-engine:dev` (handled by + # the preceding `Recycle engine containers on image drift` + # step); the reconciler reattaches them on backend boot. ids=$(docker ps -aq \ --filter "label=galaxy.stack=dev-deploy" \ --filter "status=exited" \ diff --git a/tools/dev-deploy/README.md b/tools/dev-deploy/README.md index 8b3fd13..2a7a9fe 100644 --- a/tools/dev-deploy/README.md +++ b/tools/dev-deploy/README.md @@ -235,6 +235,29 @@ The deploy is idempotent — when the PR later merges into healthcheck steps, overwriting whatever the manual dispatch left behind. There is no separate state to clean up between the two paths. +### Engine image drift recycle + +`backend` spawns one engine container per game (the long-lived "Dev +Sandbox" plus any user-created games) and the reconciler reattaches +to whatever it finds with the `galaxy.stack=dev-deploy` label. That +reattach does not check the running container's image SHA against the +freshly-built `galaxy-engine:dev` tag, so an unchanged container would +otherwise keep serving the previous engine code after a redeploy. + +The `dev-deploy.yaml` workflow handles this in the +`Recycle engine containers on image drift` step. When `docker build` +produces a new `galaxy-engine:dev` SHA, the step compares it against +every running `galaxy-game-*` container and, for each drifted one, +stops the backend, removes the container, wipes its bind-mounted +state directory (Engine.Init() writes turn-0 over any pre-existing +`turn-N` files), and cascade-deletes the lobby `games` row. The +`dev-sandbox` bootstrap on the next backend boot finds no live +sandbox and provisions a fresh one on the new engine image. + +When the engine sources are unchanged, the BuildKit cache hits and +the SHA stays the same — the recycle step is a no-op and the running +games keep their state across the deploy. + ## Relationship to other infrastructure - `tools/local-dev/` — single-developer playground, host-port mapped, From 723885e74ec51f9b4e6661d8322bfe78e50c5956 Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Fri, 29 May 2026 11:42:27 +0200 Subject: [PATCH 3/4] fix(order): surface rejection reason, keep sync green, hydrate verdicts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues surfaced once the per-command rejection from the previous commit actually reached the UI: 1. Sync banner falsely red. `OrderDraftStore.runSync` flipped `syncStatus = "error"` whenever any command was rejected and advertised a Retry button. A per-command rejection is a player-correctable state — the round trip succeeded, the engine just refused that command — so the retry can't help. Keep `syncStatus = "synced"` on `success`; the red row highlight is the visible cue. 2. Rejection reason missing. Add `cmd_error_message: string` to `CommandItem` in `pkg/schema/fbs/order.fbs` (appended last to preserve existing slot offsets) and regenerate the Go + TS stubs for that one type. Plumb the message through `CommandMeta`, `Controller.applyCommand`'s `m.Result(code, message)` call, the Go transcoder, the UI decoders in `submit.ts` / `order-load.ts`, and the `OrderDraftStore.errorMessages` map. `order-tab.svelte` renders it as an italic danger-coloured line under rejected commands, with new CSS for `.error-reason`. 3. Verdict lost on navigation. `order-load.ts.decodeCommand` never read `cmdApplied`/`cmdErrorCode`, so `hydrateFromServer` fell back to a blanket "applied" status — a previously-rejected command came back green after a lobby → game round trip. Extend the fetch decoder to populate `statuses`/`errorCodes`/ `errorMessages` maps and have `hydrateFromServer` use them. Engine-side persistence already records the verdict on disk — verified against the live `0000/order/.json`. `flatbuffers@25` elides default-int8/int64 fields on write; the Go transcoder force-slots `cmd_applied=false` / `cmd_error_code=0` already, the new test fixtures flip `builder.forceDefaults(true)` to mirror that behaviour so the round trip survives. Co-Authored-By: Claude Opus 4.7 (1M context) --- game/internal/controller/order.go | 4 +- game/internal/repo/repo_test.go | 8 +- game/openapi.yaml | 10 ++ pkg/model/order/order.go | 12 +- pkg/schema/fbs/order.fbs | 6 + pkg/schema/fbs/order/CommandItem.go | 13 +- pkg/transcoder/order.go | 23 ++++ pkg/transcoder/order_test.go | 8 +- ui/frontend/src/lib/sidebar/order-tab.svelte | 23 ++++ .../proto/galaxy/fbs/order/command-item.ts | 31 ++++- ui/frontend/src/sync/order-draft.svelte.ts | 71 +++++++--- ui/frontend/src/sync/order-load.ts | 33 ++++- ui/frontend/src/sync/submit.ts | 13 +- .../tests/helpers/fake-order-client.ts | 41 +++++- ui/frontend/tests/order-draft.test.ts | 127 +++++++++++++++++- ui/frontend/tests/order-queue.test.ts | 1 + ui/frontend/tests/submit.test.ts | 20 ++- 17 files changed, 404 insertions(+), 40 deletions(-) diff --git a/game/internal/controller/order.go b/game/internal/controller/order.go index e695708..6a30d30 100644 --- a/game/internal/controller/order.go +++ b/game/internal/controller/order.go @@ -109,11 +109,11 @@ func (c *Controller) applyCommand(actor string, cmd order.DecodableCommand) (err } if ge, ok := errors.AsType[*e.GenericError](err); ok { - m.Result(ge.Code) + m.Result(ge.Code, ge.Error()) } else if err != nil { panic(fmt.Errorf("error applying command has unknown origin: %w", err)) } else { - m.Result(0) + m.Result(0, "") } return diff --git a/game/internal/repo/repo_test.go b/game/internal/repo/repo_test.go index 7bca20f..b504df8 100644 --- a/game/internal/repo/repo_test.go +++ b/game/internal/repo/repo_test.go @@ -75,14 +75,14 @@ func TestSaveOrder(t *testing.T) { for i := range o.Commands { if v, ok := order.AsCommand[*order.CommandRaceVote](o.Commands[i]); ok { m := &v.CommandMeta - m.Result(0) + m.Result(0, "") } else if v, ok := order.AsCommand[*order.CommandRaceQuit](o.Commands[i]); ok { - v.Result(10) + v.Result(10, "race quit failed") } else if v, ok := order.AsCommand[*order.CommandShipClassCreate](o.Commands[i]); ok { m := &v.CommandMeta - m.Result(33) + m.Result(33, "ship class create failed") } else if v, ok := order.AsCommand[*order.CommandShipGroupMerge](o.Commands[i]); ok { - v.Result(0) + v.Result(0, "") } } diff --git a/game/openapi.yaml b/game/openapi.yaml index b875b5b..42bfd75 100644 --- a/game/openapi.yaml +++ b/game/openapi.yaml @@ -520,6 +520,16 @@ components: `0` when the command was applied, a non-zero `GenericError` code (shelves `2xxx`/`3xxx` in `pkg/error/generic.go`) when the command was rejected. Omitted on requests. + cmdErrorMessage: + type: string + description: | + Per-command rejection reason, formatted by the engine's + `GenericError.Error()` (e.g. + `Entity does not exists: ship type "Drone"`). Set alongside + `cmdApplied=false`/`cmdErrorCode!=0`, omitted when the + command was applied. Provided so clients can surface the + specific reason without keeping their own code → text + catalog in sync with the engine. CommandType: type: string description: Discriminator identifying the game command variant carried in a `cmd` element. diff --git a/pkg/model/order/order.go b/pkg/model/order/order.go index 2488431..5eef406 100644 --- a/pkg/model/order/order.go +++ b/pkg/model/order/order.go @@ -124,6 +124,7 @@ type CommandMeta struct { CmdID string `json:"cmdId" binding:"required,uuid_rfc4122"` CmdApplied *bool `json:"cmdApplied,omitempty"` CmdErrCode *int `json:"cmdErrorCode,omitempty"` + CmdErrMsg *string `json:"cmdErrorMessage,omitempty"` } func (cm CommandMeta) CommandType() CommandType { @@ -134,9 +135,18 @@ func (cm CommandMeta) CommandID() string { return cm.CmdID } -func (cm *CommandMeta) Result(errCode int) { +// Result records the per-command outcome on the meta. errCode == 0 marks +// the command as applied and clears CmdErrMsg; non-zero records the +// rejection along with the human-readable engine message so clients can +// surface the reason without their own code-to-text catalog. +func (cm *CommandMeta) Result(errCode int, errMsg string) { cm.CmdErrCode = &errCode cm.CmdApplied = new(bool(errCode == 0)) + if errCode == 0 { + cm.CmdErrMsg = nil + return + } + cm.CmdErrMsg = &errMsg } type CommandRaceQuit struct { diff --git a/pkg/schema/fbs/order.fbs b/pkg/schema/fbs/order.fbs index bc6be0c..32b161c 100644 --- a/pkg/schema/fbs/order.fbs +++ b/pkg/schema/fbs/order.fbs @@ -193,6 +193,12 @@ table CommandItem { cmd_applied: bool = null; cmd_error_code: int64 = null; payload: CommandPayload (required); + // Human-readable failure reason returned by the engine when + // `cmd_applied = false`. Appended after `payload` to preserve the + // wire offsets of existing slots (FBS field IDs are allocated in + // declaration order, so inserting in the middle would shift every + // later slot). Omitted on requests and on applied commands. + cmd_error_message: string; } // UserGamesCommand is the signed-gRPC request payload for diff --git a/pkg/schema/fbs/order/CommandItem.go b/pkg/schema/fbs/order/CommandItem.go index 8bc63d3..cde005b 100644 --- a/pkg/schema/fbs/order/CommandItem.go +++ b/pkg/schema/fbs/order/CommandItem.go @@ -96,8 +96,16 @@ func (rcv *CommandItem) Payload(obj *flatbuffers.Table) bool { return false } +func (rcv *CommandItem) CmdErrorMessage() []byte { + o := flatbuffers.UOffsetT(rcv._tab.Offset(14)) + if o != 0 { + return rcv._tab.ByteVector(o + rcv._tab.Pos) + } + return nil +} + func CommandItemStart(builder *flatbuffers.Builder) { - builder.StartObject(5) + builder.StartObject(6) } func CommandItemAddCmdId(builder *flatbuffers.Builder, cmdId flatbuffers.UOffsetT) { builder.PrependUOffsetTSlot(0, flatbuffers.UOffsetT(cmdId), 0) @@ -116,6 +124,9 @@ func CommandItemAddPayloadType(builder *flatbuffers.Builder, payloadType Command func CommandItemAddPayload(builder *flatbuffers.Builder, payload flatbuffers.UOffsetT) { builder.PrependUOffsetTSlot(4, flatbuffers.UOffsetT(payload), 0) } +func CommandItemAddCmdErrorMessage(builder *flatbuffers.Builder, cmdErrorMessage flatbuffers.UOffsetT) { + builder.PrependUOffsetTSlot(5, flatbuffers.UOffsetT(cmdErrorMessage), 0) +} func CommandItemEnd(builder *flatbuffers.Builder) flatbuffers.UOffsetT { return builder.EndObject() } diff --git a/pkg/transcoder/order.go b/pkg/transcoder/order.go index f8b419c..bb13a84 100644 --- a/pkg/transcoder/order.go +++ b/pkg/transcoder/order.go @@ -125,6 +125,7 @@ type encodedCommand struct { cmdID string cmdApplied *bool cmdErrCode *int + cmdErrMsg *string payloadType fbs.CommandPayload payloadOffset flatbuffers.UOffsetT } @@ -404,6 +405,7 @@ func encodedCommandFromMeta(meta model.CommandMeta, payloadType fbs.CommandPaylo cmdID: meta.CmdID, cmdApplied: cloneBoolPointer(meta.CmdApplied), cmdErrCode: cloneIntPointer(meta.CmdErrCode), + cmdErrMsg: cloneStringPointer(meta.CmdErrMsg), payloadType: payloadType, payloadOffset: payloadOffset, } @@ -423,6 +425,11 @@ func decodeOrderCommand(flatCommand *fbs.CommandItem, index int) (model.Decodabl commandMeta.CmdErrCode = &decodedCmdErrCode } + if cmdErrMsg := flatCommand.CmdErrorMessage(); cmdErrMsg != nil { + decodedCmdErrMsg := string(cmdErrMsg) + commandMeta.CmdErrMsg = &decodedCmdErrMsg + } + payloadType := flatCommand.PayloadType() if payloadType == fbs.CommandPayloadNONE { return nil, fmt.Errorf("decode order command %d: payload type is NONE", index) @@ -915,6 +922,15 @@ func cloneIntPointer(value *int) *int { return &cloned } +func cloneStringPointer(value *string) *string { + if value == nil { + return nil + } + + cloned := *value + return &cloned +} + // UserGamesCommandToPayload converts model.UserGamesCommand to // FlatBuffers bytes suitable for the authenticated gateway transport. // `GameID` is required. @@ -1293,6 +1309,10 @@ func encodeCommandItemVector(builder *flatbuffers.Builder, commands []model.Deco return 0, fmt.Errorf("encode %s: %w", opLabel, err) } cmdID := builder.CreateString(encoded.cmdID) + var cmdErrMsg flatbuffers.UOffsetT + if encoded.cmdErrMsg != nil { + cmdErrMsg = builder.CreateString(*encoded.cmdErrMsg) + } fbs.CommandItemStart(builder) fbs.CommandItemAddCmdId(builder, cmdID) if encoded.cmdApplied != nil { @@ -1303,6 +1323,9 @@ func encodeCommandItemVector(builder *flatbuffers.Builder, commands []model.Deco } fbs.CommandItemAddPayloadType(builder, encoded.payloadType) fbs.CommandItemAddPayload(builder, encoded.payloadOffset) + if encoded.cmdErrMsg != nil { + fbs.CommandItemAddCmdErrorMessage(builder, cmdErrMsg) + } offsets[i] = fbs.CommandItemEnd(builder) } if len(offsets) == 0 { diff --git a/pkg/transcoder/order_test.go b/pkg/transcoder/order_test.go index c573669..d64fbb2 100644 --- a/pkg/transcoder/order_test.go +++ b/pkg/transcoder/order_test.go @@ -94,6 +94,7 @@ func TestUserGamesOrderResponsePayloadRoundTrip(t *testing.T) { applied := true rejected := false errCode := 7 + errMsg := "rename failed: planet does not exist" source := &model.UserGamesOrder{ GameID: uuid.MustParse("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"), UpdatedAt: 99, @@ -104,7 +105,7 @@ func TestUserGamesOrderResponsePayloadRoundTrip(t *testing.T) { Name: "alpha", }, &model.CommandPlanetRename{ - CommandMeta: commandMeta("cmd-2", model.CommandTypePlanetRename, &rejected, &errCode), + CommandMeta: commandMetaWithMsg("cmd-2", model.CommandTypePlanetRename, &rejected, &errCode, &errMsg), Number: 6, Name: "beta", }, @@ -254,10 +255,15 @@ func TestInt64ToInt(t *testing.T) { } func commandMeta(id string, cmdType model.CommandType, applied *bool, errCode *int) model.CommandMeta { + return commandMetaWithMsg(id, cmdType, applied, errCode, nil) +} + +func commandMetaWithMsg(id string, cmdType model.CommandType, applied *bool, errCode *int, errMsg *string) model.CommandMeta { return model.CommandMeta{ CmdType: cmdType, CmdID: id, CmdApplied: applied, CmdErrCode: errCode, + CmdErrMsg: errMsg, } } diff --git a/ui/frontend/src/lib/sidebar/order-tab.svelte b/ui/frontend/src/lib/sidebar/order-tab.svelte index a6d308f..4ca7499 100644 --- a/ui/frontend/src/lib/sidebar/order-tab.svelte +++ b/ui/frontend/src/lib/sidebar/order-tab.svelte @@ -153,6 +153,10 @@ Tests exercise the tab through `__galaxyDebug.seedOrderDraft` function statusOf(cmd: OrderCommand): CommandStatus { return draft?.statuses[cmd.id] ?? "draft"; } + + function errorMessageOf(cmd: OrderCommand): string | null { + return draft?.errorMessages[cmd.id] ?? null; + }
@@ -191,6 +195,7 @@ Tests exercise the tab through `__galaxyDebug.seedOrderDraft`
    {#each draft.commands as cmd, index (cmd.id)} {@const status = statusOf(cmd)} + {@const errorReason = errorMessageOf(cmd)}
  1. {describe(cmd)} + {#if status === "rejected" && errorReason !== null} + + {errorReason} + + {/if} (obj:any):any|null { return offset ? this.bb!.__union(obj, this.bb_pos + offset) : null; } +cmdErrorMessage():string|null +cmdErrorMessage(optionalEncoding:flatbuffers.Encoding):string|Uint8Array|null +cmdErrorMessage(optionalEncoding?:any):string|Uint8Array|null { + const offset = this.bb!.__offset(this.bb_pos, 14); + return offset ? this.bb!.__string(this.bb_pos + offset, optionalEncoding) : null; +} + static startCommandItem(builder:flatbuffers.Builder) { - builder.startObject(5); + builder.startObject(6); } static addCmdId(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Offset) { @@ -84,11 +91,11 @@ static addCmdId(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Offset) { } static addCmdApplied(builder:flatbuffers.Builder, cmdApplied:boolean) { - builder.addFieldInt8(1, +cmdApplied, null); + builder.addFieldInt8(1, +cmdApplied, 0); } static addCmdErrorCode(builder:flatbuffers.Builder, cmdErrorCode:bigint) { - builder.addFieldInt64(2, cmdErrorCode, null); + builder.addFieldInt64(2, cmdErrorCode, BigInt(0)); } static addPayloadType(builder:flatbuffers.Builder, payloadType:CommandPayload) { @@ -99,13 +106,17 @@ static addPayload(builder:flatbuffers.Builder, payloadOffset:flatbuffers.Offset) builder.addFieldOffset(4, payloadOffset, 0); } +static addCmdErrorMessage(builder:flatbuffers.Builder, cmdErrorMessageOffset:flatbuffers.Offset) { + builder.addFieldOffset(5, cmdErrorMessageOffset, 0); +} + static endCommandItem(builder:flatbuffers.Builder):flatbuffers.Offset { const offset = builder.endObject(); builder.requiredField(offset, 12) // payload return offset; } -static createCommandItem(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Offset, cmdApplied:boolean|null, cmdErrorCode:bigint|null, payloadType:CommandPayload, payloadOffset:flatbuffers.Offset):flatbuffers.Offset { +static createCommandItem(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Offset, cmdApplied:boolean|null, cmdErrorCode:bigint|null, payloadType:CommandPayload, payloadOffset:flatbuffers.Offset, cmdErrorMessageOffset:flatbuffers.Offset):flatbuffers.Offset { CommandItem.startCommandItem(builder); CommandItem.addCmdId(builder, cmdIdOffset); if (cmdApplied !== null) @@ -114,6 +125,7 @@ static createCommandItem(builder:flatbuffers.Builder, cmdIdOffset:flatbuffers.Of CommandItem.addCmdErrorCode(builder, cmdErrorCode); CommandItem.addPayloadType(builder, payloadType); CommandItem.addPayload(builder, payloadOffset); + CommandItem.addCmdErrorMessage(builder, cmdErrorMessageOffset); return CommandItem.endCommandItem(builder); } @@ -127,7 +139,8 @@ unpack(): CommandItemT { const temp = unionToCommandPayload(this.payloadType(), this.payload.bind(this)); if(temp === null) { return null; } return temp.unpack() - })() + })(), + this.cmdErrorMessage() ); } @@ -142,6 +155,7 @@ unpackTo(_o: CommandItemT): void { if(temp === null) { return null; } return temp.unpack() })(); + _o.cmdErrorMessage = this.cmdErrorMessage(); } } @@ -151,20 +165,23 @@ constructor( public cmdApplied: boolean|null = null, public cmdErrorCode: bigint|null = null, public payloadType: CommandPayload = CommandPayload.NONE, - public payload: CommandFleetMergeT|CommandFleetSendT|CommandPlanetProduceT|CommandPlanetRenameT|CommandPlanetRouteRemoveT|CommandPlanetRouteSetT|CommandRaceQuitT|CommandRaceRelationT|CommandRaceVoteT|CommandScienceCreateT|CommandScienceRemoveT|CommandShipClassCreateT|CommandShipClassMergeT|CommandShipClassRemoveT|CommandShipGroupBreakT|CommandShipGroupDismantleT|CommandShipGroupJoinFleetT|CommandShipGroupLoadT|CommandShipGroupMergeT|CommandShipGroupSendT|CommandShipGroupTransferT|CommandShipGroupUnloadT|CommandShipGroupUpgradeT|null = null + public payload: CommandFleetMergeT|CommandFleetSendT|CommandPlanetProduceT|CommandPlanetRenameT|CommandPlanetRouteRemoveT|CommandPlanetRouteSetT|CommandRaceQuitT|CommandRaceRelationT|CommandRaceVoteT|CommandScienceCreateT|CommandScienceRemoveT|CommandShipClassCreateT|CommandShipClassMergeT|CommandShipClassRemoveT|CommandShipGroupBreakT|CommandShipGroupDismantleT|CommandShipGroupJoinFleetT|CommandShipGroupLoadT|CommandShipGroupMergeT|CommandShipGroupSendT|CommandShipGroupTransferT|CommandShipGroupUnloadT|CommandShipGroupUpgradeT|null = null, + public cmdErrorMessage: string|Uint8Array|null = null ){} pack(builder:flatbuffers.Builder): flatbuffers.Offset { const cmdId = (this.cmdId !== null ? builder.createString(this.cmdId!) : 0); const payload = builder.createObjectOffset(this.payload); + const cmdErrorMessage = (this.cmdErrorMessage !== null ? builder.createString(this.cmdErrorMessage!) : 0); return CommandItem.createCommandItem(builder, cmdId, this.cmdApplied, this.cmdErrorCode, this.payloadType, - payload + payload, + cmdErrorMessage ); } } diff --git a/ui/frontend/src/sync/order-draft.svelte.ts b/ui/frontend/src/sync/order-draft.svelte.ts index bd093ba..df4275b 100644 --- a/ui/frontend/src/sync/order-draft.svelte.ts +++ b/ui/frontend/src/sync/order-draft.svelte.ts @@ -95,6 +95,15 @@ export interface PausedBanner { export class OrderDraftStore { commands: OrderCommand[] = $state([]); statuses: Record = $state({}); + /** + * errorMessages carries the engine-formatted rejection reason for + * each rejected command, keyed by `cmdId`. Populated from + * `submitOrder` responses and from the server-stored order on + * `hydrateFromServer`. Cleared when a command is re-mutated or + * removed. The order tab renders the message inline on rejected + * rows so the player can act without inspecting the network log. + */ + errorMessages: Record = $state({}); updatedAt = $state(0); status: Status = $state("idle"); error: string | null = $state(null); @@ -261,16 +270,29 @@ export class OrderDraftStore { this.commands = fetched.commands; this.updatedAt = fetched.updatedAt; this.recomputeStatuses(); - // Server-fetched commands echo cmdApplied=true for entries - // that survived previous turns; keep them as `applied` so - // the overlay continues to project them on the inspector. + // The engine echoes per-command `cmdApplied`/`cmdErrorCode` + // /`cmdErrorMessage` on every stored order entry. Hydrate + // the in-memory status + error maps from the fetched + // snapshot so a returning player sees the same per-command + // verdict (and rejection reason) the previous submission + // produced, instead of a synthetic blanket `applied` derived + // from the local cache. const next = { ...this.statuses }; + const nextErrors: Record = {}; for (const cmd of this.commands) { - if (next[cmd.id] === "valid") { + const status = fetched.statuses.get(cmd.id); + if (status !== undefined && next[cmd.id] !== "invalid") { + next[cmd.id] = status; + } else if (next[cmd.id] === "valid") { next[cmd.id] = "applied"; } + const msg = fetched.errorMessages.get(cmd.id); + if (msg !== null && msg !== undefined && msg !== "") { + nextErrors[cmd.id] = msg; + } } this.statuses = next; + this.errorMessages = nextErrors; await this.persist(); if ((this.syncStatus as SyncStatus) !== "paused") { this.syncStatus = "synced"; @@ -384,11 +406,15 @@ export class OrderDraftStore { } this.commands = nextCommands; const nextStatuses = { ...this.statuses }; + const nextErrors = { ...this.errorMessages }; for (const id of removed) { delete nextStatuses[id]; + delete nextErrors[id]; } nextStatuses[command.id] = validateCommand(command); + delete nextErrors[command.id]; this.statuses = nextStatuses; + this.errorMessages = nextErrors; await this.persist(); this.scheduleSync(); } @@ -408,8 +434,11 @@ export class OrderDraftStore { this.clearConflictForMutation(); this.commands = next; const nextStatuses = { ...this.statuses }; + const nextErrors = { ...this.errorMessages }; delete nextStatuses[id]; + delete nextErrors[id]; this.statuses = nextStatuses; + this.errorMessages = nextErrors; await this.persist(); this.scheduleSync(); } @@ -484,6 +513,7 @@ export class OrderDraftStore { if (this.status !== "ready") return; this.commands = []; this.statuses = {}; + this.errorMessages = {}; this.updatedAt = 0; this.conflictBanner = null; this.pausedBanner = null; @@ -571,20 +601,18 @@ export class OrderDraftStore { this.applyResultsInternal( outcome.result.results, outcome.result.updatedAt, + outcome.result.errorMessages, ); - // Even with `result.ok === true` an individual - // command may have been rejected by the engine - // (e.g. validation passed transcoders but failed - // the in-game rule). Surface that as an error in - // the sync bar so the player notices and can fix - // or remove the offending command. - const anyRejected = Array.from( - outcome.result.results.values(), - ).some((s) => s === "rejected"); - this.syncStatus = anyRejected ? "error" : "synced"; - this.syncError = anyRejected - ? "engine rejected one or more commands" - : null; + // A `success` outcome means the gateway → backend → + // engine round trip completed cleanly, even when + // individual commands were rejected by the in-game + // rules. Per-command rejection is a player-fixable + // state surfaced via the row's `rejected` highlight + // and the inline reason; the sync bar stays green so + // the banner + Retry button are reserved for genuine + // transport / engine failures the auto-retry can fix. + this.syncStatus = "synced"; + this.syncError = null; break; } case "rejected": { @@ -669,9 +697,11 @@ export class OrderDraftStore { private applyResultsInternal( results: Map, updatedAt: number, + errorMessages?: Map, ): void { const liveIds = new Set(this.commands.map((cmd) => cmd.id)); const next = { ...this.statuses }; + const nextErrors = { ...this.errorMessages }; for (const [id, status] of results.entries()) { // Drop verdicts for commands the user removed while the // request was in flight — they are no longer in the @@ -679,8 +709,15 @@ export class OrderDraftStore { // confuse the order tab and the overlay. if (!liveIds.has(id)) continue; next[id] = status; + const msg = errorMessages?.get(id) ?? null; + if (msg !== null && msg !== "") { + nextErrors[id] = msg; + } else { + delete nextErrors[id]; + } } this.statuses = next; + this.errorMessages = nextErrors; this.updatedAt = updatedAt; } diff --git a/ui/frontend/src/sync/order-load.ts b/ui/frontend/src/sync/order-load.ts index e992750..56a0b5e 100644 --- a/ui/frontend/src/sync/order-load.ts +++ b/ui/frontend/src/sync/order-load.ts @@ -63,6 +63,17 @@ export class OrderLoadError extends Error { export interface FetchedOrder { commands: OrderCommand[]; + // Per-command status keyed by cmdId. Populated from the engine's + // stored order so a returning player sees the same per-command + // verdict (applied / rejected) the previous submission produced — + // not a synthetic "applied" derived from the local cache. + statuses: Map; + // Per-command engine-formatted error code/message, keyed by cmdId. + // Both maps carry an entry for every loaded command; the value is + // null when the command was applied (no error). The message lets + // the UI surface the rejection reason without a code → text catalog. + errorCodes: Map; + errorMessages: Map; updatedAt: number; } @@ -119,7 +130,13 @@ function decodeResponse(payload: Uint8Array): FetchedOrder { const buffer = new ByteBuffer(payload); const response = UserGamesOrderGetResponse.getRootAsUserGamesOrderGetResponse(buffer); if (!response.found()) { - return { commands: [], updatedAt: 0 }; + return { + commands: [], + statuses: new Map(), + errorCodes: new Map(), + errorMessages: new Map(), + updatedAt: 0, + }; } const order = response.order(); if (order === null) { @@ -130,6 +147,9 @@ function decodeResponse(payload: Uint8Array): FetchedOrder { ); } const commands: OrderCommand[] = []; + const statuses = new Map(); + const errorCodes = new Map(); + const errorMessages = new Map(); const length = order.commandsLength(); for (let i = 0; i < length; i++) { const item = order.commands(i); @@ -137,9 +157,20 @@ function decodeResponse(payload: Uint8Array): FetchedOrder { const cmd = decodeCommand(item); if (cmd === null) continue; commands.push(cmd); + // The engine echoes `cmd_applied = false` only when the order + // was rejected per-command; missing / true both mean applied. + const applied = item.cmdApplied(); + statuses.set(cmd.id, applied === false ? "rejected" : "applied"); + const code = item.cmdErrorCode(); + errorCodes.set(cmd.id, code === null ? null : Number(code)); + const msg = item.cmdErrorMessage(); + errorMessages.set(cmd.id, msg === null ? null : msg); } return { commands, + statuses, + errorCodes, + errorMessages, updatedAt: Number(order.updatedAt()), }; } diff --git a/ui/frontend/src/sync/submit.ts b/ui/frontend/src/sync/submit.ts index dba5315..2acc5ba 100644 --- a/ui/frontend/src/sync/submit.ts +++ b/ui/frontend/src/sync/submit.ts @@ -82,6 +82,7 @@ export interface SubmitSuccess { ok: true; results: Map; errorCodes: Map; + errorMessages: Map; updatedAt: number; } @@ -511,6 +512,7 @@ function decodeOrderResponse( ): SubmitSuccess { const results = new Map(); const errorCodes = new Map(); + const errorMessages = new Map(); let updatedAt = 0; if (payload.length === 0) { @@ -518,8 +520,9 @@ function decodeOrderResponse( for (const cmd of commands) { results.set(cmd.id, "applied"); errorCodes.set(cmd.id, null); + errorMessages.set(cmd.id, null); } - return { ok: true, results, errorCodes, updatedAt }; + return { ok: true, results, errorCodes, errorMessages, updatedAt }; } const buffer = new ByteBuffer(payload); @@ -531,8 +534,9 @@ function decodeOrderResponse( for (const cmd of commands) { results.set(cmd.id, "applied"); errorCodes.set(cmd.id, null); + errorMessages.set(cmd.id, null); } - return { ok: true, results, errorCodes, updatedAt }; + return { ok: true, results, errorCodes, errorMessages, updatedAt }; } for (let i = 0; i < length; i++) { @@ -542,8 +546,10 @@ function decodeOrderResponse( if (cmdId === null) continue; const applied = item.cmdApplied(); const errorCode = item.cmdErrorCode(); + const errorMessage = item.cmdErrorMessage(); results.set(cmdId, applied === false ? "rejected" : "applied"); errorCodes.set(cmdId, errorCode === null ? null : Number(errorCode)); + errorMessages.set(cmdId, errorMessage === null ? null : errorMessage); } // Defensive: any submitted command not echoed back falls back to @@ -552,10 +558,11 @@ function decodeOrderResponse( if (!results.has(cmd.id)) { results.set(cmd.id, "applied"); errorCodes.set(cmd.id, null); + errorMessages.set(cmd.id, null); } } - return { ok: true, results, errorCodes, updatedAt }; + return { ok: true, results, errorCodes, errorMessages, updatedAt }; } function decodeError( diff --git a/ui/frontend/tests/helpers/fake-order-client.ts b/ui/frontend/tests/helpers/fake-order-client.ts index 5be01d3..6286d4f 100644 --- a/ui/frontend/tests/helpers/fake-order-client.ts +++ b/ui/frontend/tests/helpers/fake-order-client.ts @@ -174,11 +174,25 @@ export function recordingClient( * decode a realistic payload without standing up a full mock * gateway. */ +/** + * FakeFetchStatus carries the per-command result fields the engine + * would echo on `user.games.order.get` (cmdApplied / cmdErrorCode / + * cmdErrorMessage). The helper omits each set only when the + * corresponding pointer is `undefined`, mimicking the engine's + * `omitempty` semantics. + */ +export interface FakeFetchStatus { + applied?: boolean; + errorCode?: number; + errorMessage?: string; +} + export function fakeFetchClient( gameId: string, commands: OrderCommand[], updatedAt: number, found = true, + statuses?: Record, ): { client: GalaxyClient } { const client: GalaxyClient = { async executeCommand(messageType: string) { @@ -187,7 +201,7 @@ export function fakeFetchClient( } return { resultCode: "ok", - payloadBytes: encodeOrderGet(gameId, commands, updatedAt, found), + payloadBytes: encodeOrderGet(gameId, commands, updatedAt, found, statuses ?? {}), }; }, } as unknown as GalaxyClient; @@ -200,6 +214,10 @@ function encodeApplied( applied: boolean, ): Uint8Array { const builder = new Builder(256); + // See `encodeOrderGet` — flatbuffers@25 elides `cmd_applied=false` + // against its int8 default; mirror the Go transcoder's force-slot + // behaviour so the boolean survives the round trip in tests. + builder.forceDefaults(true); const itemOffsets = cmdIds.map((id) => { const cmdIdOffset = builder.createString(id); const nameOffset = builder.createString("ignored"); @@ -235,8 +253,15 @@ function encodeOrderGet( commands: OrderCommand[], updatedAt: number, found: boolean, + statuses: Record, ): Uint8Array { const builder = new Builder(256); + // flatbuffers@25 elides fields equal to their generated default; the + // Go transcoder works around this via explicit `Slot()` calls. + // Mirror that behaviour here so `cmd_applied=false` and + // `cmd_error_code=0` round-trip through the helper instead of being + // silently dropped. + builder.forceDefaults(true); let orderOffset = 0; if (found) { @@ -246,6 +271,11 @@ function encodeOrderGet( } const cmdIdOffset = builder.createString(cmd.id); const nameOffset = builder.createString(cmd.name); + const status = statuses[cmd.id] ?? {}; + const errMsgOffset = + status.errorMessage !== undefined + ? builder.createString(status.errorMessage) + : 0; const inner = CommandPlanetRename.createCommandPlanetRename( builder, BigInt(cmd.planetNumber), @@ -253,8 +283,17 @@ function encodeOrderGet( ); CommandItem.startCommandItem(builder); CommandItem.addCmdId(builder, cmdIdOffset); + if (status.applied !== undefined) { + CommandItem.addCmdApplied(builder, status.applied); + } + if (status.errorCode !== undefined) { + CommandItem.addCmdErrorCode(builder, BigInt(status.errorCode)); + } CommandItem.addPayloadType(builder, CommandPayload.CommandPlanetRename); CommandItem.addPayload(builder, inner); + if (errMsgOffset !== 0) { + CommandItem.addCmdErrorMessage(builder, errMsgOffset); + } return CommandItem.endCommandItem(builder); }); const commandsVec = UserGamesOrder.createCommandsVector(builder, itemOffsets); diff --git a/ui/frontend/tests/order-draft.test.ts b/ui/frontend/tests/order-draft.test.ts index 06c9c01..ec7533e 100644 --- a/ui/frontend/tests/order-draft.test.ts +++ b/ui/frontend/tests/order-draft.test.ts @@ -10,12 +10,13 @@ import "@testing-library/jest-dom/vitest"; import "fake-indexeddb/auto"; import { waitFor } from "@testing-library/svelte"; -import { afterEach, beforeEach, describe, expect, test } from "vitest"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import type { IDBPDatabase } from "idb"; import { IDBCache } from "../src/platform/store/idb-cache"; import { openGalaxyDB, type GalaxyDB } from "../src/platform/store/idb"; import type { Cache } from "../src/platform/store/index"; +import type { GalaxyClient } from "../src/api/galaxy-client"; import { OrderDraftStore } from "../src/sync/order-draft.svelte"; import type { OrderCommand } from "../src/sync/order-types"; @@ -448,6 +449,51 @@ describe("OrderDraftStore", () => { store.dispose(); }); + test("hydrateFromServer preserves per-command rejected status and error message", async () => { + const { fakeFetchClient } = await import("./helpers/fake-order-client"); + const { client } = fakeFetchClient( + GAME_ID, + [ + { + kind: "planetRename", + id: "ok-1", + planetNumber: 7, + name: "OkTown", + }, + { + kind: "planetRename", + id: "bad-1", + planetNumber: 8, + name: "Doomed", + }, + ], + 42, + true, + { + "ok-1": { applied: true, errorCode: 0 }, + "bad-1": { + applied: false, + errorCode: 3003, + errorMessage: 'Entity does not exists: planet #99', + }, + }, + ); + + const store = new OrderDraftStore(); + await store.init({ cache, gameId: GAME_ID }); + await store.hydrateFromServer({ client, turn: 5 }); + + expect(store.commands).toHaveLength(2); + expect(store.statuses["ok-1"]).toBe("applied"); + expect(store.statuses["bad-1"]).toBe("rejected"); + expect(store.errorMessages["ok-1"]).toBeUndefined(); + expect(store.errorMessages["bad-1"]).toBe( + "Entity does not exists: planet #99", + ); + expect(store.syncStatus).toBe("synced"); + store.dispose(); + }); + test("hydrate empties the local cache when server returns found=false", async () => { // First seed a local draft. const seeded = new OrderDraftStore(); @@ -555,6 +601,85 @@ describe("OrderDraftStore auto-sync", () => { store.dispose(); }); + test("per-command rejection in a successful response keeps syncStatus 'synced'", async () => { + // Per-command rejection means the round trip succeeded — only + // individual commands were rejected by the in-game rules — so + // the sync bar must stay green. A blanket `syncStatus = "error"` + // + Retry button only fits genuine transport / engine failures. + const { Builder } = await import("flatbuffers"); + const fbs = await import("../src/proto/galaxy/fbs/order"); + const common = await import("../src/proto/galaxy/fbs/common"); + const { uuidToHiLo } = await import("../src/api/game-state"); + + const cmd = { + kind: "planetRename" as const, + id: "rej-1", + planetNumber: 9, + name: "Doomed", + }; + + const exec = vi.fn(async () => { + const builder = new Builder(256); + builder.forceDefaults(true); + const cmdIdOffset = builder.createString(cmd.id); + const nameOffset = builder.createString("ignored"); + const errMsg = builder.createString( + "Entity does not exists: planet #99", + ); + const inner = fbs.CommandPlanetRename.createCommandPlanetRename( + builder, + BigInt(0), + nameOffset, + ); + fbs.CommandItem.startCommandItem(builder); + fbs.CommandItem.addCmdId(builder, cmdIdOffset); + fbs.CommandItem.addCmdApplied(builder, false); + fbs.CommandItem.addCmdErrorCode(builder, BigInt(3003)); + fbs.CommandItem.addPayloadType( + builder, + fbs.CommandPayload.CommandPlanetRename, + ); + fbs.CommandItem.addPayload(builder, inner); + fbs.CommandItem.addCmdErrorMessage(builder, errMsg); + const item = fbs.CommandItem.endCommandItem(builder); + const commandsVec = fbs.UserGamesOrderResponse.createCommandsVector( + builder, + [item], + ); + const [hi, lo] = uuidToHiLo(GAME_ID); + const gameIdOffset = common.UUID.createUUID(builder, hi, lo); + fbs.UserGamesOrderResponse.startUserGamesOrderResponse(builder); + fbs.UserGamesOrderResponse.addGameId(builder, gameIdOffset); + fbs.UserGamesOrderResponse.addUpdatedAt(builder, BigInt(99)); + fbs.UserGamesOrderResponse.addCommands(builder, commandsVec); + const offset = fbs.UserGamesOrderResponse.endUserGamesOrderResponse( + builder, + ); + builder.finish(offset); + return { + resultCode: "ok", + payloadBytes: builder.asUint8Array(), + }; + }); + const client = { executeCommand: exec } as unknown as GalaxyClient; + + const store = new OrderDraftStore(); + await store.init({ cache, gameId: GAME_ID }); + store.bindClient(client); + await store.add(cmd); + await waitFor(() => { + expect(exec).toHaveBeenCalled(); + expect(store.statuses[cmd.id]).toBe("rejected"); + }); + + expect(store.syncStatus).toBe("synced"); + expect(store.syncError).toBeNull(); + expect(store.errorMessages[cmd.id]).toBe( + "Entity does not exists: planet #99", + ); + store.dispose(); + }); + test("non-ok response marks every in-flight command as rejected", async () => { const { recordingClient } = await import("./helpers/fake-order-client"); const handle = recordingClient(GAME_ID, "rejected"); diff --git a/ui/frontend/tests/order-queue.test.ts b/ui/frontend/tests/order-queue.test.ts index 0091420..39ad5b5 100644 --- a/ui/frontend/tests/order-queue.test.ts +++ b/ui/frontend/tests/order-queue.test.ts @@ -69,6 +69,7 @@ function success(updatedAt = 1): SubmitSuccess { ok: true, results: new Map([["id", "applied"]]), errorCodes: new Map([["id", null]]), + errorMessages: new Map([["id", null]]), updatedAt, }; } diff --git a/ui/frontend/tests/submit.test.ts b/ui/frontend/tests/submit.test.ts index 369bc0a..13d0380 100644 --- a/ui/frontend/tests/submit.test.ts +++ b/ui/frontend/tests/submit.test.ts @@ -40,13 +40,28 @@ function mockClient( } function buildResponse( - commands: { id: string; applied: boolean | null; errorCode: number | null }[], + commands: { + id: string; + applied: boolean | null; + errorCode: number | null; + errorMessage?: string | null; + }[], updatedAt: number, ): Uint8Array { const builder = new Builder(256); + // flatbuffers@25 skips fields equal to their generated default when + // writing — the Go transcoder works around this via explicit + // `Slot()` calls. This fixture stands in for the engine + gateway, + // so we flip `forceDefaults` to keep `cmd_applied=false` (=== int8 + // default 0) and `cmd_error_code=0` from being silently elided. + builder.forceDefaults(true); const itemOffsets = commands.map((c) => { const cmdIdOffset = builder.createString(c.id); const nameOffset = builder.createString("ignored"); + const errMsgOffset = + c.errorMessage !== undefined && c.errorMessage !== null + ? builder.createString(c.errorMessage) + : 0; const payloadOffset = CommandPlanetRename.createCommandPlanetRename( builder, BigInt(0), @@ -60,6 +75,9 @@ function buildResponse( } CommandItem.addPayloadType(builder, CommandPayload.CommandPlanetRename); CommandItem.addPayload(builder, payloadOffset); + if (errMsgOffset !== 0) { + CommandItem.addCmdErrorMessage(builder, errMsgOffset); + } return CommandItem.endCommandItem(builder); }); const commandsVec = UserGamesOrderResponse.createCommandsVector(builder, itemOffsets); From 2ffd7527a6c8cb5608a2a85c8df1b7aba963220d Mon Sep 17 00:00:00 2001 From: Ilia Denisov Date: Fri, 29 May 2026 12:11:50 +0200 Subject: [PATCH 4/4] test(ui): align rejected-submit e2e specs with per-command sync semantics The `rename-planet` and `ship-classes` rejected-submit specs broke on the previous commit because: 1. `tests/e2e/fixtures/order-fbs.ts` builds the FBS response without `forceDefaults(true)`, and flatbuffers@25's TS codegen now elides `cmd_applied=false` against its int8 default of 0. The encoded payload no longer carried the rejection, so the UI decoded the row as `applied` and the assertions on the `rejected` status text failed first. The production Go transcoder already force-slots the field; mirror that behaviour in the e2e fixture. 2. The specs themselves still asserted the old blanket `data-sync-status="error"` on per-command rejection. After the previous commit's behaviour change the bar stays `synced` for per-command rejection (only genuine transport failures keep the red banner + Retry), so the assertions now read the row's inline reason text instead. `tests/e2e/fixtures/order-fbs.ts` also gains the `cmdErrorMessage` field so future fixtures can mirror the engine's rejection reason through the round trip. Co-Authored-By: Claude Opus 4.7 (1M context) --- ui/frontend/tests/e2e/fixtures/order-fbs.ts | 24 +++++++++++++++++++++ ui/frontend/tests/e2e/rename-planet.spec.ts | 14 +++++++++--- ui/frontend/tests/e2e/ship-classes.spec.ts | 6 +++++- 3 files changed, 40 insertions(+), 4 deletions(-) diff --git a/ui/frontend/tests/e2e/fixtures/order-fbs.ts b/ui/frontend/tests/e2e/fixtures/order-fbs.ts index ac9fe20..b5e8c57 100644 --- a/ui/frontend/tests/e2e/fixtures/order-fbs.ts +++ b/ui/frontend/tests/e2e/fixtures/order-fbs.ts @@ -34,6 +34,10 @@ interface CommandResultFixtureBase { cmdId: string; applied: boolean | null; errorCode: number | null; + // Optional engine-formatted rejection reason; mirrors the + // `cmd_error_message` field added in the same patch as + // `cmdErrorCode`'s shelf renumber. Omit on applied commands. + errorMessage?: string | null; } export interface PlanetRenameResultFixture extends CommandResultFixtureBase { @@ -132,6 +136,14 @@ export function buildOrderResponsePayload( updatedAt: number, ): Uint8Array { const builder = new Builder(256); + // flatbuffers@25 elides any field equal to its generated default; + // for `cmd_applied: bool = null` the default is int8 0, so the + // false case (per-command rejection) would silently disappear from + // the encoded payload. The production Go transcoder uses explicit + // `Slot()` calls that force the write, and the unit-test helpers + // in `tests/helpers/fake-order-client.ts` flip `forceDefaults`; + // these e2e fixtures need the same toggle. + builder.forceDefaults(true); const itemOffsets = commands.map((c) => encodeItem(builder, c)); const commandsVec = UserGamesOrderResponse.createCommandsVector( builder, @@ -155,6 +167,11 @@ export function buildOrderGetResponsePayload( found = true, ): Uint8Array { const builder = new Builder(256); + // See `buildOrderResponsePayload` — the GET response path needs + // the same `forceDefaults` toggle so a stored per-command rejection + // (cmd_applied=false / cmd_error_code=0) survives the round trip + // to the UI's `hydrateFromServer`. + builder.forceDefaults(true); let orderOffset = 0; if (found) { @@ -290,6 +307,10 @@ function encodeItem(builder: Builder, c: CommandResultFixture): number { break; } } + const errMsgOffset = + c.errorMessage !== undefined && c.errorMessage !== null + ? builder.createString(c.errorMessage) + : 0; CommandItem.startCommandItem(builder); CommandItem.addCmdId(builder, cmdIdOffset); if (c.applied !== null) CommandItem.addCmdApplied(builder, c.applied); @@ -298,6 +319,9 @@ function encodeItem(builder: Builder, c: CommandResultFixture): number { } CommandItem.addPayloadType(builder, payloadType); CommandItem.addPayload(builder, inner); + if (errMsgOffset !== 0) { + CommandItem.addCmdErrorMessage(builder, errMsgOffset); + } return CommandItem.endCommandItem(builder); } diff --git a/ui/frontend/tests/e2e/rename-planet.spec.ts b/ui/frontend/tests/e2e/rename-planet.spec.ts index 25c7170..558a551 100644 --- a/ui/frontend/tests/e2e/rename-planet.spec.ts +++ b/ui/frontend/tests/e2e/rename-planet.spec.ts @@ -137,6 +137,9 @@ async function mockGateway(page: Page, opts: MockOpts): Promise { name: submittedName, applied, errorCode: applied ? null : 1, + errorMessage: applied + ? null + : "Entity does not exists: planet #99", }); } if (opts.submitOutcome === "applied") { @@ -320,14 +323,19 @@ test("rejected submit keeps the old name and surfaces the failure", async ({ const orderTool = page.getByTestId("sidebar-tool-order"); // The auto-sync pipeline reaches the server immediately after - // the inline confirm; the rejected verdict surfaces through the - // per-row status badge and the sync bar. + // the inline confirm. Per-command rejection is a player-correctable + // state: the round trip succeeded, the engine just refused this + // command, so the sync bar stays `synced` while the rejected row + // surfaces the engine-formatted reason for the player. await expect(orderTool.getByTestId("order-command-status-0")).toHaveText( "rejected", ); + await expect(orderTool.getByTestId("order-command-error-0")).toHaveText( + "Entity does not exists: planet #99", + ); await expect(orderTool.getByTestId("order-sync")).toHaveAttribute( "data-sync-status", - "error", + "synced", ); await page.getByTestId("sidebar-tab-inspector").click(); diff --git a/ui/frontend/tests/e2e/ship-classes.spec.ts b/ui/frontend/tests/e2e/ship-classes.spec.ts index 5742f65..d40afc3 100644 --- a/ui/frontend/tests/e2e/ship-classes.spec.ts +++ b/ui/frontend/tests/e2e/ship-classes.spec.ts @@ -167,6 +167,9 @@ async function mockGateway(page: Page, opts: MockOpts): Promise { cargo: lastCreate.cargo, applied, errorCode: applied ? null : 1, + errorMessage: applied + ? null + : "Entity already exists: ship type \"Drone\"", }); continue; } @@ -392,9 +395,10 @@ test("rejected createShipClass keeps the table empty and surfaces the failure", await expect(orderTool.getByTestId("order-command-status-0")).toHaveText( "rejected", ); + await expect(orderTool.getByTestId("order-command-error-0")).toBeVisible(); await expect(orderTool.getByTestId("order-sync")).toHaveAttribute( "data-sync-status", - "error", + "synced", ); // Switch sidebar back to inspector so the active-view (table)