refactor(mapgen-studio): centralize ORPCError data cast, drop dead success fallbacks, and narrow router to contract-derived type

apps/mapgen-studio/src/features/civ7Setup/api.ts

@@ -12,7 +12,7 @@
 // router maps them onto `ORPCError.status`, which we read back below.
 import { ORPCError } from "@orpc/client";
 
-import { orpcClient } from "../../lib/orpc";
+import { orpcClient, readErrorData } from "../../lib/orpc";
 import type { Civ7SavedSetupConfigFile, Civ7SetupSnapshotLike } from "./setupConfig";
 
 export type Civ7SetupCatalogOption = Readonly<{
@@ -44,7 +44,9 @@ function orpcFailure(
   fallback: string,
 ): { error: string; statusCode?: number; observedAt?: string } {
   if (err instanceof ORPCError) {
-    const data = (err.data ?? undefined) as { observedAt?: string } | undefined;
+    // `observedAt` rides in the error body for setupConfig (503) and savedConfigs
+    // (500) — the router attaches it via `orpcError(…, { observedAt })`.
+    const data = readErrorData<{ observedAt: string }>(err);
     return {
       error: err.message || `HTTP ${err.status}`,
       statusCode: err.status,
@@ -65,7 +67,8 @@ export async function fetchCiv7SetupConfig(options: { signal?: AbortSignal } = {
     );
     return {
       ok: true,
-      observedAt: body.observedAt ?? new Date().toISOString(),
+      // Contract-required on the success body (civ7.setupConfig output: isoTimestamp).
+      observedAt: body.observedAt,
       setup: body.setup as Civ7SetupSnapshotLike,
     };
   } catch (err) {
@@ -81,8 +84,10 @@ export async function fetchCiv7SavedSetupConfigs(): Promise<
     const body = await orpcClient.civ7.savedConfigs({});
     return {
       ok: true,
-      observedAt: body.observedAt ?? new Date().toISOString(),
-      directory: body.directory ?? "",
+      // Both contract-required on the success body (civ7.savedConfigs output:
+      // `observedAt: isoTimestamp`, `directory: z.string()`).
+      observedAt: body.observedAt,
+      directory: body.directory,
       configurations: body.configurations as unknown as ReadonlyArray<Civ7SavedSetupConfigFile>,
     };
   } catch (err) {

apps/mapgen-studio/src/features/runInGame/api.ts

@@ -12,7 +12,7 @@
 // `ORPCError.data`, which we read back below.
 import { ORPCError } from "@orpc/client";
 
-import { orpcClient } from "../../lib/orpc";
+import { orpcClient, readErrorData } from "../../lib/orpc";
 import { normalizeStudioSetupConfig, type Civ7StudioSetupConfig } from "../civ7Setup/setupConfig";
 import type { RunInGameFailureDetails, RunInGameOperationStatus } from "./status";
 
@@ -28,7 +28,9 @@ function runInGameFailure(
   fallback: string,
 ): { error: string; details?: RunInGameFailureDetails; statusCode?: number } {
   if (err instanceof ORPCError) {
-    const data = (err.data ?? undefined) as { details?: RunInGameFailureDetails } | undefined;
+    // `details` (the `RunInGameFailureDetails` the engine attached) rides in the
+    // error body; the router carries it via `ORPCError.data`.
+    const data = readErrorData<{ details: RunInGameFailureDetails }>(err);
     return {
       error: err.message || `HTTP ${err.status}`,
       statusCode: err.status,

apps/mapgen-studio/src/lib/orpc.ts

@@ -1,4 +1,4 @@
-import { createORPCClient } from "@orpc/client";
+import { createORPCClient, ORPCError } from "@orpc/client";
 import { RPCLink } from "@orpc/client/fetch";
 import { createTanstackQueryUtils } from "@orpc/tanstack-query";
 import type { ContractRouterClient } from "@orpc/contract";
@@ -38,5 +38,29 @@ export const orpcClient: ContractRouterClient<StudioContract> =
  */
 export const orpc = createTanstackQueryUtils(orpcClient);
 
+/**
+ * Read the extra-body `data` an `ORPCError` carried back from the studio router.
+ *
+ * The router pins each procedure's legacy HTTP code on `ORPCError.status` and
+ * tucks the legacy body's side fields (`observedAt`, `details`, …) into
+ * `ORPCError.data` (see `packages/studio-server/src/router` + `errors.ts`). The
+ * contract does not declare an `errorMap`, so on the client `err.data` is typed
+ * `unknown` — every caller would otherwise repeat the same inline
+ * `(err.data ?? undefined) as { … }` cast to recover one field.
+ *
+ * This is the single typed accessor for that recovery: it narrows `data` to a
+ * `Partial<T>` (each field still treated as possibly-absent, because the wire
+ * shape is not statically guaranteed) and returns `undefined` when there is no
+ * usable object. Callers read individual fields with their own runtime guard,
+ * exactly as the previous hand-rolled casts did — only the cast is centralized.
+ */
+export function readErrorData<T extends Record<string, unknown>>(
+  err: ORPCError<string, unknown>,
+): Partial<T> | undefined {
+  return typeof err.data === "object" && err.data !== null
+    ? (err.data as Partial<T>)
+    : undefined;
+}
+
 export { contract };
 export type { StudioContract };

openspec/changes/mapgen-studio-rigor/design.md

@@ -0,0 +1,92 @@
+# Design — mapgen-studio-rigor
+
+## Context
+
+A pure type-precision + cleanup pass over the oRPC failure-translation seam left
+behind by the redesign stack. No parity surface moves; the guiding rule is MOVE /
+centralize, never rewrite. Each decision below preserves the produced wire/envelope
+shapes exactly and only sharpens the types around them.
+
+## Decisions
+
+### D1 — Where the shared `readErrorData<T>()` lives, and its shape
+
+`src/lib/orpc.ts` is the client seam (it owns `orpcClient` + the `RPCLink`), so the
+accessor for `ORPCError.data` belongs there next to the client that throws those
+errors. Signature:
+
+```ts
+function readErrorData<T extends Record<string, unknown>>(
+  err: ORPCError<string, unknown>,
+): Partial<T> | undefined
+```
+
+- **`Partial<T>`, not `T`.** The contract declares no `errorMap`, so the wire shape
+  of `data` is not statically guaranteed; each field is treated as possibly-absent
+  and the caller keeps its existing runtime guard (`typeof data?.observedAt ===
+  "string"`, `data?.details !== undefined`). This is exactly what the old inline
+  `as { observedAt?: string } | undefined` casts expressed — centralized, not
+  loosened.
+- **`| undefined`** when `data` is not a non-null object, so callers `?.`-chain into
+  it the same way they did against `(err.data ?? undefined)`.
+- The accessor does NOT decide `instanceof ORPCError` — callers already gate on that
+  (their non-`ORPCError` branch produces the bare-`message` envelope). It only
+  removes the duplicated `data` cast.
+
+Result: the two callers' failure envelopes are byte-identical; only the cast moved.
+
+### D2 — Which `??` fallbacks are dead
+
+`civ7.setupConfig` and `civ7.savedConfigs` success outputs (contract/civ7.ts)
+declare `observedAt: isoTimestamp` and `directory: z.string()` as REQUIRED. On the
+SUCCESS branch the body therefore always carries them, so:
+
+| Site | Fallback | Verdict |
+| --- | --- | --- |
+| `fetchCiv7SetupConfig` success `observedAt` | `?? new Date().toISOString()` | dead — required on output |
+| `fetchCiv7SavedSetupConfigs` success `observedAt` | `?? new Date().toISOString()` | dead — required on output |
+| `fetchCiv7SavedSetupConfigs` success `directory` | `?? ""` | dead — required on output |
+| ERROR-path `observedAt` (in `orpcFailure`) | guarded, not defaulted | KEPT — error body may omit it |
+
+The error path is untouched: there `observedAt` legitimately may be absent (the
+router attaches it on 503/500 but the type does not guarantee it), so it still flows
+through `readErrorData` + the `typeof … === "string"` guard.
+
+### D3 — Narrowing the router type
+
+The effect-orpc `oe.router(...)` result is an `EnhancedRouter<…>` whose type
+references effect-orpc internals (`effect-procedure.js`). Two options:
+
+1. **Infer** the return type (`ReturnType<typeof createStudioRouter>` with no
+   annotation). REJECTED: `tsup` emits declarations for `@civ7/studio-server`, and
+   the inferred type trips **TS2742** ("cannot be named without a reference to
+   effect-orpc/src/effect-procedure.js — not portable").
+2. **Annotate** the contract-derived `Router<StudioContract, Record<never, never>>`
+   from `@orpc/server`. CHOSEN: `Router` is part of `@orpc/server`'s public surface
+   (nameable, portable), it pins the type to `StudioContract` (the goal), and the
+   initial context is `Record<never, never>` because the injected `ManagedRuntime`
+   fully provides `Civ7TunerClient | StudioConfig`. `RPCHandler` accepts it
+   (`AnyRouter` is its lower bound), so `handler.ts` is unaffected.
+
+`StudioRouter = ReturnType<typeof createStudioRouter>` then resolves to that
+annotated `Router<StudioContract, …>` — contract-pinned, portable, no churn.
+
+### D4 — Comments: why, not what
+
+New "why" comments are added only where the rationale is non-obvious: `readErrorData`
+(why `data` is `unknown` + why centralize), the router annotation (why annotated not
+inferred — TS2742), and the two pruned success returns (the contract guarantees the
+field). The stores and `app/*` component tree already carry intentional headers from
+the data-model / app-shell slices; re-commenting them would be churn, which this
+no-behavior-change slice avoids.
+
+## Risks
+
+- **Parity (§7).** The non-uniform status codes, the error-`data` side fields, and
+  the failure-envelope shapes are do-not-break. Mitigation: `readErrorData` is a
+  pure cast-centralization (same guards, same outputs), the dropped fallbacks are on
+  fields the contract guarantees on the success path, and the router narrowing is a
+  type-only change. `bun run test` (138 tests incl. the status/envelope suites) +
+  the runtime check are the guard.
+- **Portability of the narrowed router type.** Mitigation: the `tsup` DTS build is
+  in the verification gate — a non-portable type would fail it (TS2742).

openspec/changes/mapgen-studio-rigor/proposal.md

@@ -0,0 +1,110 @@
+## Why
+
+The data-model slice (`mapgen-studio-data-model`) realised the oRPC-native read
+surface and the persisted stores, but the oRPC failure-translation seam still has
+type-precision rough edges that escaped each prior slice's scope:
+
+1. **Scattered inline `err.data` casts.** Each `features/*/api.ts` failure helper
+   re-implements the same `(err.data ?? undefined) as { … }` cast to recover one
+   side field (`observedAt`, `details`) from a thrown `ORPCError`. The contract
+   declares no `errorMap`, so on the client `ORPCError.data` is `unknown`; every
+   caller open-codes the same narrowing. That is duplicated, untyped boilerplate.
+
+2. **Dead `??` fallbacks on contract-required fields.** `features/civ7Setup/api.ts`
+   guards the SUCCESS body with `body.observedAt ?? new Date().toISOString()` and
+   `body.directory ?? ""`. But `civ7.setupConfig` / `civ7.savedConfigs` declare
+   `observedAt` (and `directory`) as REQUIRED on their success output schema — the
+   fallbacks can never fire and falsely imply the field may be absent.
+
+3. **`AnyRouter` on the server router seam.** `createStudioRouter` is annotated
+   `: AnyRouter` and `StudioRouter = AnyRouter`, discarding the `StudioContract`
+   pinning the effect-orpc router actually carries.
+
+This is a pure type-precision + cleanup pass: NO behavior change, NO parity surface
+moves. Refactors translate/centralize; they never rewrite logic.
+
+## Target Authority Refs
+
+- `docs/projects/mapgen-studio-redesign/FRAME.md` (§4.7 everything talks oRPC)
+- `docs/projects/mapgen-studio-redesign/architecture/10-target-architecture.md`
+  (§6 ts rigor, §7 do-not-break registry — the non-uniform status codes and the
+  error-`data` side fields are parity and must survive unchanged)
+- `apps/mapgen-studio/src/lib/orpc.ts` (the client seam — new home for the accessor)
+- `packages/studio-server/src/contract/civ7.ts` (the required-field declarations)
+- `packages/studio-server/src/router/index.ts` (the `AnyRouter` annotation)
+
+## What Changes
+
+- **One shared `readErrorData<T>()` accessor.** Add `readErrorData` to
+  `src/lib/orpc.ts`: it takes a thrown `ORPCError` and narrows its `data` to
+  `Partial<T> | undefined` (an object or nothing), centralizing the one cast.
+  `features/civ7Setup/api.ts` and `features/runInGame/api.ts` use it in place of
+  their inline `(err.data ?? undefined) as { … }` casts. The per-field runtime
+  guards (`typeof data?.observedAt === "string"`, `data?.details !== undefined`)
+  are unchanged, so the produced failure envelopes are byte-identical.
+- **Drop dead success-path fallbacks.** In `features/civ7Setup/api.ts` remove the
+  `?? new Date().toISOString()` (observedAt) and `?? ""` (directory) fallbacks on
+  the `fetchCiv7SetupConfig` / `fetchCiv7SavedSetupConfigs` SUCCESS returns; the
+  contract output guarantees those fields. (The error-path `observedAt`, which
+  legitimately may be absent, still flows through `readErrorData` + its guard.)
+- **Narrow the router away from `AnyRouter`.** `createStudioRouter` returns the
+  contract-derived `Router<StudioContract, Record<never, never>>` (its initial
+  context is fully provided by the injected `ManagedRuntime`), and
+  `StudioRouter = ReturnType<typeof createStudioRouter>`. This is the portable
+  contract-pinned type; inferring the raw effect-orpc `EnhancedRouter<…>` is NOT
+  used because it references effect-orpc internals and trips TS2742 in the emitted
+  declarations. `RPCHandler` still accepts it (`AnyRouter` is its lower bound).
+- **Intentional comments on the seam.** Add "why" doc comments on `readErrorData`
+  (why `data` is `unknown` and the cast is centralized) and the router narrowing
+  (why the type is annotated, not inferred). The stores and component tree already
+  carry intentional headers from prior slices; no churn there.
+
+## Requires
+
+- `mapgen-studio-data-model` (the prior slice — the `readErrorData` callers and the
+  contract output schemas this prunes against come from there; this stacks on its
+  `design/data-model` tip)
+
+## Affected Owners
+
+- `apps/mapgen-studio/src/lib/orpc.ts` (`readErrorData` added)
+- `apps/mapgen-studio/src/features/civ7Setup/api.ts` (use accessor; drop fallbacks)
+- `apps/mapgen-studio/src/features/runInGame/api.ts` (use accessor)
+- `packages/studio-server/src/router/index.ts` (router type narrowed)
+
+## Forbidden Owners
+
+- No change to the non-uniform status codes, the error-`data` side fields, or the
+  failure-envelope shapes (`{ ok, error, statusCode, observedAt, details }`) — they
+  are parity (§7) and must round-trip unchanged.
+- No enabling of `noUncheckedIndexedAccess` / `exactOptionalPropertyTypes` in this
+  slice (too broad; deferred).
+- No logic rewrite in any router procedure body or any `features/*/api.ts` caller.
+- No `mods/**` changes.
+
+## Stop Conditions
+
+- The `Router<StudioContract, …>` annotation cannot type-check against the
+  effect-orpc router or breaks the `RPCHandler` / `handler.ts` consumers — in that
+  case leave `AnyRouter` and note it (the narrowing is "if feasible without churn").
+- Centralizing the `err.data` cast changes any produced failure envelope.
+- Dropping a `??` fallback would expose a field the contract does NOT guarantee.
+
+## Consumer Impact
+
+None observable. The studio behaves identically: same failure envelopes, same
+non-uniform status codes, same error-`data` side fields, same success payloads. The
+change is internal type precision — one typed `readErrorData` accessor instead of
+scattered casts, no dead fallbacks on guaranteed fields, and a contract-pinned
+`StudioRouter` type instead of `AnyRouter`.
+
+## Verification Gates
+
+- `bun run check` in `apps/mapgen-studio` (tsc clean) and `tsc --noEmit` for
+  `packages/studio-server` (the router type changed).
+- `bun run build` (vite + worker-bundle check; studio-server `tsup` DTS emit clean —
+  proves the narrowed type is portable) + `bun run test` in `apps/mapgen-studio`
+  (all green).
+- Runtime: dev server; failure envelopes + success payloads unchanged over `/rpc`;
+  no console errors; screenshot renders the studio.
+- `bun run openspec -- validate mapgen-studio-rigor --strict`.