From 15a80faee9305fbcc848f46f8f823093f02f8ed5 Mon Sep 17 00:00:00 2001 From: Aymeric Rabot Date: Tue, 4 Aug 2026 23:21:19 -0400 Subject: [PATCH] fix: preserve scene materials across every persistence boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A custom scene material survived nothing. `materials` was absent from the `SceneGraph` type, so every layer that rebuilds a graph field-by-field silently omitted it: `cloneSceneGraph`/`forkSceneGraph`, the MCP bridge's `exportJSON`/`loadJSON`, `exportSceneGraph` (which `save_scene`, `publishLiveSceneSnapshot` and variant generation all persist through), the SQLite read schema, and the editor's API graph schema. Reopen a scene and it came back with default surfaces. Nodes point at materials through `slots` values shaped `scene:mat_…`. Those are opaque strings to the clone remapping, so material ids are carried over unchanged — minting new ones would orphan every reference. Two things fell out of fixing the round trip: `loadJSON` dropped collections too, for a different reason: it applied plugin state in a second call after `setScene`, and `setScene` resets `collections` and `materials` to `{}` whenever they aren't in its `extra` bag. Everything now goes in one call, which also fixes dirty-tracking for plugin-owned nodes — `setScene` marks nodes dirty at the end, and `markDirty` skips nodes whose plugin isn't installed yet. The editor's echo-suppression signature omitted `materials`, so a local edit that touched only the palette signed identically to the last remote payload and the save was skipped — the edit was lost. The signature now defaults the fields `setScene` always writes, so a payload that omits them (MCP live sync sends exactly that) still matches the store that defaulted them. Materials are validated where they enter from the network, not where they are read back from disk. The API schema holds them to `SceneMaterial` in `superRefine` — they carry texture URLs, and that schema is where the `AssetUrl` allowlist is enforced — but keeps the parsed value untransformed, since the routes persist this schema's output and `SceneMaterial` injects defaults and strips unknown keys. The SQLite read path stays permissive: nothing validates on write and `parseGraph` throws, so a strict read shape would turn one odd stored value into a permanently unloadable scene. Co-authored-by: ShiroKSH --- CHANGELOG.md | 6 +++ apps/editor/components/scene-loader.tsx | 16 +----- apps/editor/lib/graph-schema.test.ts | 49 +++++++++++++++++++ apps/editor/lib/graph-schema.ts | 23 ++++++++- apps/editor/lib/scene-signature.test.ts | 41 ++++++++++++++++ apps/editor/lib/scene-signature.ts | 27 ++++++++++ .../core/src/utils/clone-scene-graph.test.ts | 40 +++++++++++++++ packages/core/src/utils/clone-scene-graph.ts | 17 ++++++- packages/mcp/src/bridge/scene-bridge.test.ts | 23 +++++++++ packages/mcp/src/bridge/scene-bridge.ts | 40 +++++++++++---- .../src/operations/scene-operations.test.ts | 15 ++++++ .../mcp/src/operations/scene-operations.ts | 1 + .../src/storage/sqlite-scene-store.test.ts | 34 +++++++++++++ .../mcp/src/storage/sqlite-scene-store.ts | 8 +++ 14 files changed, 313 insertions(+), 27 deletions(-) create mode 100644 apps/editor/lib/scene-signature.test.ts create mode 100644 apps/editor/lib/scene-signature.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index bf9e890764..93d544389f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Preserve custom scene materials across save, load, clone, fork, and live sync. Materials were dropped at every persistence boundary, so a scene reopened with default surfaces. Collections were dropped on MCP import for the same reason. + ## 1.0.0-beta.1 (2026-07-30) The first Pascal Editor 1.0 beta. Relative to diff --git a/apps/editor/components/scene-loader.tsx b/apps/editor/components/scene-loader.tsx index fb891f1cbd..a0179005bd 100644 --- a/apps/editor/components/scene-loader.tsx +++ b/apps/editor/components/scene-loader.tsx @@ -14,6 +14,7 @@ import Image from 'next/image' import Link from 'next/link' import { useRouter, useSearchParams } from 'next/navigation' import { useCallback, useEffect, useRef, useState } from 'react' +import { type PersistedSceneGraph, sceneGraphSignature } from '@/lib/scene-signature' import { cn } from '@/lib/utils' import { BuildTab } from './build-tab' import { CommunityViewerToolbarLeft, CommunityViewerToolbarRight } from './viewer-toolbar' @@ -71,26 +72,13 @@ interface SceneLoaderProps { meta: SceneMeta } -type SceneGraphWithCollections = SceneGraph & { - collections?: Record -} - interface LiveSceneEvent { eventId: number sceneId: string version: number kind: string createdAt: string - graph: SceneGraphWithCollections -} - -function sceneGraphSignature(graph: SceneGraphWithCollections): string { - return JSON.stringify({ - nodes: graph.nodes, - rootNodeIds: graph.rootNodeIds, - collections: graph.collections, - installedPlugins: graph.installedPlugins, - }) + graph: PersistedSceneGraph } /** diff --git a/apps/editor/lib/graph-schema.test.ts b/apps/editor/lib/graph-schema.test.ts index deb9bba229..ca63ca9c3e 100644 --- a/apps/editor/lib/graph-schema.test.ts +++ b/apps/editor/lib/graph-schema.test.ts @@ -165,3 +165,52 @@ test('treats an unnamespaced unknown type as a foreign node', () => { .success, ).toBe(false) }) + +const MATERIAL_ID = 'mat_a1b2c3d4e5f6g7h8' +const material = (overrides: Record = {}) => ({ + id: MATERIAL_ID, + name: 'Oak', + material: { preset: 'wood', ...overrides }, +}) + +test('keeps materials in the parsed output', () => { + const graph = { ...buildGraph({}), materials: { [MATERIAL_ID]: material() } } + const res = apiGraphSchema.safeParse(graph) + + expect(res.success).toBe(true) + expect(res.data?.materials).toEqual(graph.materials) +}) + +// A material's texture is a URL the editor loads, so it is held to the same +// `AssetUrl` allowlist as every other URL-shaped field in the graph. +test('rejects a material texture URL outside the allowlist', () => { + for (const url of ['ftp://host/a.png', 'javascript:alert(1)']) { + const graph = { + ...buildGraph({}), + materials: { [MATERIAL_ID]: material({ texture: { url } }) }, + } + expect(apiGraphSchema.safeParse(graph).success).toBe(false) + } +}) + +test('accepts a material texture URL inside the allowlist', () => { + const graph = { + ...buildGraph({}), + materials: { + [MATERIAL_ID]: material({ texture: { url: 'https://cdn.example.com/oak.png' } }), + }, + } + + expect(apiGraphSchema.safeParse(graph).success).toBe(true) +}) + +// The routes persist this schema's output, so validation must not double as +// normalization: a save has to store the palette it was handed. +test('does not rewrite materials it accepts', () => { + const sparse = { id: MATERIAL_ID, name: 'Oak', material: { properties: { color: '#886644' } } } + const graph = { ...buildGraph({}), materials: { [MATERIAL_ID]: sparse } } + const res = apiGraphSchema.safeParse(graph) + + expect(res.success).toBe(true) + expect(res.data?.materials?.[MATERIAL_ID]).toEqual(sparse) +}) diff --git a/apps/editor/lib/graph-schema.ts b/apps/editor/lib/graph-schema.ts index 7f5f6c5ceb..f597bfa73c 100644 --- a/apps/editor/lib/graph-schema.ts +++ b/apps/editor/lib/graph-schema.ts @@ -1,4 +1,4 @@ -import { AnyNode, AssetUrl, BaseNode } from '@pascal-app/core/schema' +import { AnyNode, AssetUrl, BaseNode, SceneMaterial } from '@pascal-app/core/schema' import { z } from 'zod' /** @@ -100,6 +100,15 @@ export const apiGraphSchema = z nodes: z.record(z.string(), z.unknown()), rootNodeIds: z.array(z.string()), collections: z.unknown().optional(), + // `unknown` here, validated in `superRefine` below — the same split the + // nodes get, and for the same reason: the routes persist this schema's + // *output*, so a validating shape would also rewrite what gets stored. + // `SceneMaterial` injects `MaterialProperties` defaults and drops unknown + // keys, which would make every save silently normalize the caller's + // palette. Materials still have to be checked, because they carry texture + // URLs that `MaterialSchema` routes through `AssetUrl` — this schema is + // where that allowlist is enforced. + materials: z.record(z.string(), z.unknown()).optional(), installedPlugins: z.array(z.string().min(1)).optional(), }) .superRefine((value, ctx) => { @@ -113,6 +122,18 @@ export const apiGraphSchema = z } } + for (const [materialId, material] of Object.entries(value.materials ?? {})) { + const res = SceneMaterial.safeParse(material) + if (res.success) continue + for (const issue of res.error.issues) { + ctx.addIssue({ + code: 'custom', + path: ['materials', materialId, ...issue.path], + message: issue.message, + }) + } + } + // Ids of foreign nodes in this graph. Builtin container schemas name the // child kinds they accept (`BuildingNode.children`, `RoofNode.children`), // so a container holding a plugin child fails against `AnyNode` even diff --git a/apps/editor/lib/scene-signature.test.ts b/apps/editor/lib/scene-signature.test.ts new file mode 100644 index 0000000000..6bcce47df8 --- /dev/null +++ b/apps/editor/lib/scene-signature.test.ts @@ -0,0 +1,41 @@ +import { expect, test } from 'bun:test' +import { type PersistedSceneGraph, sceneGraphSignature } from './scene-signature' + +const NODE_ID = 'level_a1b2c3d4e5f6g7h8' +const MATERIAL_ID = 'mat_a1b2c3d4e5f6g7h8' + +const graph = (overrides: Partial = {}) => + ({ + nodes: { [NODE_ID]: { object: 'node', id: NODE_ID, type: 'level', level: 0 } }, + rootNodeIds: [NODE_ID], + ...overrides, + }) as PersistedSceneGraph + +// The echo check compares a raw SSE payload against the store after +// `setScene` ran, and `setScene` always writes these three keys. A payload +// that omits them — which is exactly what MCP live sync sends — must still +// match, or every remote update looks like a local edit and gets saved back. +test('an omitted field signs the same as its applied default', () => { + expect(sceneGraphSignature(graph())).toBe( + sceneGraphSignature(graph({ collections: {}, materials: {}, installedPlugins: [] })), + ) +}) + +// Conversely, every field the save body carries has to be signed. An unsigned +// field makes a local edit that touches only that field read as an echo, and +// the save is skipped — the change is silently lost. +test('changing any signed field changes the signature', () => { + const base = sceneGraphSignature(graph()) + + expect( + sceneGraphSignature( + graph({ materials: { [MATERIAL_ID]: { id: MATERIAL_ID, name: 'Oak', material: {} } } }), + ), + ).not.toBe(base) + expect( + sceneGraphSignature(graph({ collections: { col_1: { id: 'col_1', nodeIds: [] } } })), + ).not.toBe(base) + expect(sceneGraphSignature(graph({ installedPlugins: ['@pascal-app/plugin-trees'] }))).not.toBe( + base, + ) +}) diff --git a/apps/editor/lib/scene-signature.ts b/apps/editor/lib/scene-signature.ts new file mode 100644 index 0000000000..a5a4587dd0 --- /dev/null +++ b/apps/editor/lib/scene-signature.ts @@ -0,0 +1,27 @@ +import type { SceneGraph } from '@pascal-app/editor' + +export type PersistedSceneGraph = SceneGraph & { + collections?: Record +} + +/** + * Identity of a graph for echo detection, compared across a boundary that + * normalizes: one side is a raw SSE payload, the other is the editor's state + * after `applySceneGraphToEditor` ran. `setScene` always writes `collections`, + * `materials` and `installedPlugins`, so a payload that omits them (MCP live + * sync emits exactly that) has to serialize the same as the store that + * defaulted them, or the echo reads as a local edit and gets saved back. + * + * Every field the PUT body carries has to appear here. A field that is + * persisted but unsigned makes a local change to *only* that field + * indistinguishable from an echo, and the save is skipped. + */ +export function sceneGraphSignature(graph: PersistedSceneGraph): string { + return JSON.stringify({ + nodes: graph.nodes, + rootNodeIds: graph.rootNodeIds, + collections: graph.collections ?? {}, + materials: graph.materials ?? {}, + installedPlugins: graph.installedPlugins ?? [], + }) +} diff --git a/packages/core/src/utils/clone-scene-graph.test.ts b/packages/core/src/utils/clone-scene-graph.test.ts index a189af4344..c7e30961a4 100644 --- a/packages/core/src/utils/clone-scene-graph.test.ts +++ b/packages/core/src/utils/clone-scene-graph.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from 'bun:test' import type { CollectionId } from '../schema/collections' +import type { SceneMaterialId } from '../schema/scene-material' import type { AnyNode, AnyNodeId } from '../schema/types' import { cloneLevelSubtree, @@ -46,10 +47,49 @@ function makeSceneGraph(): SceneGraph { nodeIds: ['scan_1', 'guide_1'] as AnyNodeId[], }, }, + materials: { + ['mat_1' as SceneMaterialId]: { + id: 'mat_1', + name: 'Oak', + material: { preset: 'wood' }, + }, + }, installedPlugins: ['pascal:trees'], } } +describe('scene material palette', () => { + // Nodes reference materials through `slots` values shaped `scene:mat_…`. + // Those are opaque strings to the node remapping, so the ids they point at + // have to survive a clone unchanged or every reference dangles. + test('cloneSceneGraph carries materials over with their ids intact', () => { + const source = makeSceneGraph() + const cloned = cloneSceneGraph(source) + + expect(cloned.materials).toEqual(source.materials) + }) + + test('cloneSceneGraph deep-copies materials', () => { + const source = makeSceneGraph() + const cloned = cloneSceneGraph(source) + const material = cloned.materials?.['mat_1' as SceneMaterialId] + expect(material).toBeDefined() + if (!material) return + + material.name = 'Mutated' + expect(source.materials?.['mat_1' as SceneMaterialId]?.name).toBe('Oak') + }) + + // A palette entry is authored content in its own right. Stripping the scan + // node that happened to use it must not take the material with it. + test('forkSceneGraph keeps materials when stripping scans', () => { + const source = makeSceneGraph() + const forked = forkSceneGraph(source) + + expect(forked.materials).toEqual(source.materials) + }) +}) + describe('forkSceneGraph', () => { test('strips scan and guide nodes by default', () => { const forked = forkSceneGraph(makeSceneGraph()) diff --git a/packages/core/src/utils/clone-scene-graph.ts b/packages/core/src/utils/clone-scene-graph.ts index e48f13e830..13e89ab260 100644 --- a/packages/core/src/utils/clone-scene-graph.ts +++ b/packages/core/src/utils/clone-scene-graph.ts @@ -6,11 +6,13 @@ import { import type { AnyNode, AnyNodeId } from '../schema' import { generateId } from '../schema/base' import type { Collection, CollectionId } from '../schema/collections' +import type { SceneMaterial, SceneMaterialId } from '../schema/scene-material' export type SceneGraph = { nodes: Record rootNodeIds: AnyNodeId[] collections?: Record + materials?: Record installedPlugins?: string[] } @@ -32,7 +34,7 @@ function extractIdPrefix(id: string): string { * - Multi-scene in-memory scenarios */ export function cloneSceneGraph(sceneGraph: SceneGraph): SceneGraph { - const { nodes, rootNodeIds, collections, installedPlugins } = sceneGraph + const { nodes, rootNodeIds, collections, materials, installedPlugins } = sceneGraph // Build ID mapping: old ID -> new ID const idMap = new Map() @@ -164,6 +166,12 @@ export function cloneSceneGraph(sceneGraph: SceneGraph): SceneGraph { nodes: clonedNodes, rootNodeIds: clonedRootNodeIds, ...(clonedCollections && { collections: clonedCollections }), + // Material ids are deliberately *not* remapped. Nodes point at these + // through `slots` values shaped `scene:mat_…` — opaque strings that the + // node remapping above copies verbatim, since `idMap` only covers node + // ids. Minting fresh material ids here would orphan every one of those + // refs and the clone would render with default materials. + ...(materials && { materials: structuredClone(materials) }), ...(installedPlugins && { installedPlugins: [...installedPlugins] }), } } @@ -304,7 +312,7 @@ export function forkSceneGraph( return cloneSceneGraph(sceneGraph) } - const { nodes, rootNodeIds, collections, installedPlugins } = sceneGraph + const { nodes, rootNodeIds, collections, materials, installedPlugins } = sceneGraph // First, identify scan and guide node IDs to exclude (user-uploaded imagery) const excludedNodeIds = new Set() @@ -366,6 +374,11 @@ export function forkSceneGraph( nodes: filteredNodes, rootNodeIds: filteredRootNodeIds, ...(filteredCollections && { collections: filteredCollections }), + // Kept whole rather than filtered to the surviving nodes: a palette entry + // is authored content in its own right, and dropping the scan node that + // happened to be its only user would silently delete a material the fork's + // owner can still pick from the palette. + ...(materials && { materials }), ...(installedPlugins && { installedPlugins }), }) } diff --git a/packages/mcp/src/bridge/scene-bridge.test.ts b/packages/mcp/src/bridge/scene-bridge.test.ts index 0bf7c3edf4..141f9e61f2 100644 --- a/packages/mcp/src/bridge/scene-bridge.test.ts +++ b/packages/mcp/src/bridge/scene-bridge.test.ts @@ -439,6 +439,29 @@ describe('SceneBridge', () => { expect(bridge.exportJSON().installedPlugins).toEqual(['pascal:trees']) }) + // `setScene` resets `collections` and `materials` to `{}` unless they are + // in its `extra` bag, so anything applied after it is silently discarded. + // These two round trips are what catch a regression back to that. + test('loadJSON round-trips the material palette', () => { + const materials = { + mat_1: { id: 'mat_1', name: 'Oak', material: { preset: 'wood' } }, + } + bridge.loadJSON({ ...bridge.exportJSON(), materials } as never) + + expect(bridge.exportJSON().materials).toEqual(materials) + }) + + test('loadJSON round-trips collections', () => { + const snap = bridge.exportJSON() + const nodeId = Object.keys(snap.nodes)[0]! + const collections = { + collection_1: { id: 'collection_1', name: 'Refs', nodeIds: [nodeId] }, + } + bridge.loadJSON({ ...snap, collections } as never) + + expect(bridge.exportJSON().collections).toEqual(collections) + }) + test('legacy graphs do not become explicitly uninstalled on export', () => { const snap = bridge.exportJSON() const { installedPlugins: _installedPlugins, ...legacy } = snap diff --git a/packages/mcp/src/bridge/scene-bridge.ts b/packages/mcp/src/bridge/scene-bridge.ts index f69672fcde..193ba626d0 100644 --- a/packages/mcp/src/bridge/scene-bridge.ts +++ b/packages/mcp/src/bridge/scene-bridge.ts @@ -20,6 +20,9 @@ export type ActiveSceneMeta = Pick< 'id' | 'name' | 'projectId' | 'ownerId' | 'thumbnailUrl' | 'version' > +/** The `extra` bag `setScene` accepts — collections, materials, plugin state. */ +type SetSceneExtra = Parameters['setScene']>[2] + /** * Headless bridge to the `@pascal-app/core` Zustand store. * @@ -59,11 +62,15 @@ export class SceneBridge { } /** Replace entire scene (undoable via Zundo). */ - setScene(nodes: Record, rootNodeIds: AnyNodeId[]): void { - useScene.getState().setScene(nodes, rootNodeIds) + setScene( + nodes: Record, + rootNodeIds: AnyNodeId[], + extra?: SetSceneExtra, + ): void { + useScene.getState().setScene(nodes, rootNodeIds, extra) } - /** Full snapshot for export, including collections. */ + /** Full snapshot for export, including collections and the material palette. */ exportJSON(): SceneGraph & { collections: Record } { const state = useScene.getState() // Deep-clone so callers can't mutate store state directly. @@ -72,6 +79,7 @@ export class SceneBridge { nodes: state.nodes, rootNodeIds: state.rootNodeIds, collections: state.collections ?? {}, + materials: state.materials ?? {}, ...(state.hasExplicitPluginInstallState || state.installedPlugins.length > 0 ? { installedPlugins: state.installedPlugins } : {}), @@ -119,13 +127,25 @@ export class SceneBridge { } } - this.setScene(nodes as Record, rootNodeIds as AnyNodeId[]) - if (Array.isArray(obj.installedPlugins)) { - useScene.getState().setInstalledPlugins( - obj.installedPlugins.filter((id): id is string => typeof id === 'string'), - { explicit: true }, - ) - } + const record = (value: unknown) => + value && typeof value === 'object' && !Array.isArray(value) ? value : undefined + const collections = record(obj.collections) as NonNullable['collections'] + const materials = record(obj.materials) as NonNullable['materials'] + const installedPlugins = Array.isArray(obj.installedPlugins) + ? obj.installedPlugins.filter((id): id is string => typeof id === 'string') + : undefined + + // One `setScene` call rather than a follow-up `setInstalledPlugins`: + // `setScene` overwrites `collections` and `materials` with `{}` whenever + // they aren't in the `extra` bag, so anything applied afterwards is lost. + // It also marks every node dirty at the end, and `markDirty` skips nodes + // whose plugin isn't installed — so plugin state has to be in place by + // then or plugin-owned nodes never get validated. + this.setScene(nodes as Record, rootNodeIds as AnyNodeId[], { + ...(collections && { collections }), + ...(materials && { materials }), + ...(installedPlugins && { installedPlugins, hasExplicitPluginInstallState: true }), + }) } /** Read a single node, or `null` if not present. */ diff --git a/packages/mcp/src/operations/scene-operations.test.ts b/packages/mcp/src/operations/scene-operations.test.ts index 63146664a0..7c003d1d22 100644 --- a/packages/mcp/src/operations/scene-operations.test.ts +++ b/packages/mcp/src/operations/scene-operations.test.ts @@ -3,6 +3,7 @@ import * as fs from 'node:fs/promises' import * as os from 'node:os' import * as path from 'node:path' import type { SceneGraph } from '@pascal-app/core/clone-scene-graph' +import { SceneBridge } from '../bridge/scene-bridge' import { SqliteSceneStore } from '../storage/sqlite-scene-store' import { createSceneOperations } from './scene-operations' @@ -77,3 +78,17 @@ describe('SceneOperationsFacade scene events', () => { await expect(operations.listSceneEvents('live')).rejects.toThrow('scene_events_unavailable') }) }) + +// `exportSceneGraph` hand-copies fields off `exportJSON`, so a field it omits +// is dropped from everything that persists through it — `save_scene`, +// `publishLiveSceneSnapshot`, and variant generation. +describe('SceneOperationsFacade exportSceneGraph', () => { + test('carries the material palette off the bridge', () => { + const bridge = new SceneBridge() + const materials = { mat_1: { id: 'mat_1', name: 'Oak', material: { preset: 'wood' } } } + bridge.loadJSON({ ...makeGraph(), materials } as never) + const operations = createSceneOperations({ bridge }) + + expect(operations.exportSceneGraph().materials).toEqual(materials) + }) +}) diff --git a/packages/mcp/src/operations/scene-operations.ts b/packages/mcp/src/operations/scene-operations.ts index 0167d5b743..4527c69c9b 100644 --- a/packages/mcp/src/operations/scene-operations.ts +++ b/packages/mcp/src/operations/scene-operations.ts @@ -150,6 +150,7 @@ class SceneOperationsFacade implements SceneOperations { nodes: exported.nodes, rootNodeIds: exported.rootNodeIds, collections: exported.collections as SceneGraph['collections'], + materials: exported.materials, installedPlugins: exported.installedPlugins, } } diff --git a/packages/mcp/src/storage/sqlite-scene-store.test.ts b/packages/mcp/src/storage/sqlite-scene-store.test.ts index c97c2011a3..84a003427f 100644 --- a/packages/mcp/src/storage/sqlite-scene-store.test.ts +++ b/packages/mcp/src/storage/sqlite-scene-store.test.ts @@ -113,6 +113,40 @@ describe('SqliteSceneStore', () => { expect(loaded!.name).toBe('Kitchen') }) + // `GraphSchema` strips any key it doesn't name, so a field missing from it + // is dropped on load without an error — the save looks like it worked. + test('round-trips collections, materials and installed plugins', async () => { + const graph = makeGraph({ + collections: { + collection_1: { id: 'collection_1', name: 'Refs', nodeIds: ['site_abc'] }, + } as SceneGraph['collections'], + materials: { + mat_1: { id: 'mat_1', name: 'Oak', material: { preset: 'wood' } }, + } as SceneGraph['materials'], + installedPlugins: ['pascal:trees'], + }) + await store.save({ id: 'full', name: 'Full', graph }) + + store.close() + store = createStore(rootDir) + + expect((await store.load('full'))?.graph).toEqual(graph) + }) + + // Nothing validates a graph on the way in, and `parseGraph` throws on a + // shape mismatch, so a strict read schema would make an odd stored value + // permanently unloadable rather than merely odd. + test('loads a stored scene whose material does not match the strict schema', async () => { + const graph = makeGraph({ + materials: { + mat_1: { id: 'mat_1', material: { texture: { url: 'ftp://host/a.png' } } }, + } as unknown as SceneGraph['materials'], + }) + await store.save({ id: 'odd', name: 'Odd', graph }) + + expect((await store.load('odd'))?.graph).toEqual(graph) + }) + test('stores optional metadata verbatim', async () => { await store.save({ id: 'meta-test', diff --git a/packages/mcp/src/storage/sqlite-scene-store.ts b/packages/mcp/src/storage/sqlite-scene-store.ts index f705a8793b..566ba1419a 100644 --- a/packages/mcp/src/storage/sqlite-scene-store.ts +++ b/packages/mcp/src/storage/sqlite-scene-store.ts @@ -70,10 +70,18 @@ interface ProjectPlaceholder { updatedAt: string } +// `z.object()` strips keys it doesn't name, so every field that must survive a +// save→load round trip has to be listed here. Values stay `unknown` rather than +// being validated against `SceneMaterial`/`Collection`: nothing validates on the +// way in, and `parseGraph` throws, so a strict shape here would let one odd +// stored value make a saved scene permanently unloadable. Validation belongs on +// the write path, where the caller can still react to it. const GraphSchema = z.object({ nodes: z.record(z.string(), z.unknown()), rootNodeIds: z.array(z.string()), collections: z.record(z.string(), z.unknown()).optional(), + materials: z.record(z.string(), z.unknown()).optional(), + installedPlugins: z.array(z.string()).optional(), }) /**