fix(game): #59 — per-command rejection on PUT /api/v1/order #71

Merged
developer merged 4 commits from feature/issue-59-invalid-order-per-command into development 2026-05-29 10:18:15 +00:00
Owner

Fixes #59.

Summary

  • Controller.ValidateOrder no longer short-circuits on the first per-command *GenericError. It applies every command in the order against a transient game-state snapshot and records the outcome on each command's meta (cmdApplied, cmdErrorCode); the order is persisted with per-command verdicts and the engine returns 202 + UserGamesOrder so the UI can surface partial-rejection instead of sync failed: [unavailable] downstream service is unavailable.
  • Engine errorResponse maps *GenericError by code shelf rather than blanket-500 (1xxx → 500/501, 2xxx → 400, 3xxx → 400). Order-level structural rejections (quit not last command) now correctly answer 400.
  • pkg/error constants are reorganised onto three documented shelves with helper predicates IsInternalCode/IsInputCode/IsGameStateCode. Two pre-existing typos fixed (ErrBeakGroupNumberNotEnoughErrBreakGroupNumberNotEnough, ErrRaceExinctErrRaceExtinct) along with all callsites.
  • The Quit-not-last type assertion was silently broken (value-typed assertion against a pointer-typed command); fixed.
  • Docs aligned: game/openapi.yaml preamble + PUT /api/v1/order description + Command schema; docs/FUNCTIONAL.md §6.2 + Russian mirror.
  • Backend / gateway / UI untouched — they were already correct on the 202 path.

Test plan

  • go test ./... green in game, backend, gateway, pkg/error, pkg/transcoder, pkg/model
  • go vet ./... clean across all workspace modules
  • New unit tests in game/internal/controller/order_test.go cover the issue scenario, no-short-circuit, simulated state, and structural rejection
  • New handler tests in game/internal/router/order_test.go confirm 202 + body for per-command rejection and 400 for ErrInputQuitCommandFollowedByCommand
  • CI on feature/issue-59-invalid-order-per-command (go-unit, ui-test, integration) — pending
  • dev-deploy smoke: build ship A → produce A on planet (both applied); delete A → produce-A keeps cmdErrorCode != 0, rest of the order stays applied, no blanket sync failed
Fixes #59. ## Summary * `Controller.ValidateOrder` no longer short-circuits on the first per-command `*GenericError`. It applies every command in the order against a transient game-state snapshot and records the outcome on each command's meta (`cmdApplied`, `cmdErrorCode`); the order is persisted with per-command verdicts and the engine returns `202 + UserGamesOrder` so the UI can surface partial-rejection instead of `sync failed: [unavailable] downstream service is unavailable`. * Engine `errorResponse` maps `*GenericError` by code shelf rather than blanket-500 (1xxx → 500/501, 2xxx → 400, 3xxx → 400). Order-level structural rejections (`quit` not last command) now correctly answer 400. * `pkg/error` constants are reorganised onto three documented shelves with helper predicates `IsInternalCode`/`IsInputCode`/`IsGameStateCode`. Two pre-existing typos fixed (`ErrBeakGroupNumberNotEnough` → `ErrBreakGroupNumberNotEnough`, `ErrRaceExinct` → `ErrRaceExtinct`) along with all callsites. * The Quit-not-last type assertion was silently broken (value-typed assertion against a pointer-typed command); fixed. * Docs aligned: `game/openapi.yaml` preamble + `PUT /api/v1/order` description + `Command` schema; `docs/FUNCTIONAL.md` §6.2 + Russian mirror. * Backend / gateway / UI untouched — they were already correct on the 202 path. ## Test plan - [x] `go test ./...` green in `game`, `backend`, `gateway`, `pkg/error`, `pkg/transcoder`, `pkg/model` - [x] `go vet ./...` clean across all workspace modules - [x] New unit tests in `game/internal/controller/order_test.go` cover the issue scenario, no-short-circuit, simulated state, and structural rejection - [x] New handler tests in `game/internal/router/order_test.go` confirm 202 + body for per-command rejection and 400 for `ErrInputQuitCommandFollowedByCommand` - [ ] CI on `feature/issue-59-invalid-order-per-command` (`go-unit`, `ui-test`, `integration`) — pending - [ ] dev-deploy smoke: build ship A → produce A on planet (both `applied`); delete A → produce-A keeps `cmdErrorCode != 0`, rest of the order stays `applied`, no blanket `sync failed`
developer added 1 commit 2026-05-29 07:36:55 +00:00
fix(game): #59 — per-command rejection on PUT /api/v1/order
Tests · Go / test (push) Successful in 2m2s
Tests · Go / test (pull_request) Successful in 3m3s
Tests · Integration / integration (pull_request) Successful in 1m40s
af30846091
Validation of a player's order now applies every command against a
transient game-state snapshot and records the per-command outcome
(cmdApplied, cmdErrorCode) in each command's meta. The order is
persisted even when some commands are rejected, and the response is
202 + UserGamesOrder so clients can surface the partial failure
without the chain collapsing into "downstream service is unavailable".

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

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

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

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
developer added 1 commit 2026-05-29 08:47:26 +00:00
fix(dev-deploy): recycle engine containers on galaxy-engine:dev SHA drift
Tests · Integration / integration (pull_request) Successful in 1m48s
Tests · Go / test (pull_request) Successful in 2m1s
e038ea6154
`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) <noreply@anthropic.com>
developer added 1 commit 2026-05-29 09:42:29 +00:00
fix(order): surface rejection reason, keep sync green, hydrate verdicts
Tests · UI / test (push) Has been cancelled
Tests · Go / test (push) Successful in 2m3s
Tests · Go / test (pull_request) Successful in 2m5s
Tests · Integration / integration (pull_request) Successful in 1m44s
Tests · UI / test (pull_request) Failing after 4m28s
723885e74e
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/<id>.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) <noreply@anthropic.com>
developer added 1 commit 2026-05-29 10:11:50 +00:00
test(ui): align rejected-submit e2e specs with per-command sync semantics
Tests · UI / test (push) Has been cancelled
Tests · Integration / integration (pull_request) Successful in 1m46s
Tests · Go / test (pull_request) Successful in 1m59s
Tests · UI / test (pull_request) Successful in 3m8s
2ffd7527a6
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) <noreply@anthropic.com>
owner approved these changes 2026-05-29 10:17:19 +00:00
developer merged commit 4a7bf0be61 into development 2026-05-29 10:18:15 +00:00
developer deleted branch feature/issue-59-invalid-order-per-command 2026-05-29 10:18:15 +00:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: developer/galaxy-game#71