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
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
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>
This commit is contained in:
@@ -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<string, FakeFetchStatus>,
|
||||
): { 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<string, FakeFetchStatus>,
|
||||
): 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);
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user