diff --git a/apps/editor/app/api/scenes/[id]/route.ts b/apps/editor/app/api/scenes/[id]/route.ts index 1712ad4ad4..10fc523b9b 100644 --- a/apps/editor/app/api/scenes/[id]/route.ts +++ b/apps/editor/app/api/scenes/[id]/route.ts @@ -7,6 +7,7 @@ import { sceneApiPreflight, withSceneApiHeaders, } from '@/lib/scene-api-security' +import { countContentNodes, wouldClearSceneContent } from '@/lib/scene-content-guard' import { getSceneOperations } from '@/lib/scene-store-server' export const dynamic = 'force-dynamic' @@ -18,6 +19,12 @@ const putSceneSchema = z.object({ graph: apiGraphSchema, thumbnailUrl: z.string().url().nullable().optional(), expectedVersion: z.number().int().nonnegative().optional(), + /** + * Opt out of the content-clear guard. Required for a deliberate + * "delete everything" save, which is otherwise indistinguishable from the + * client autosaving before it has loaded the scene. + */ + allowContentClear: z.boolean().optional(), }) const patchSceneSchema = z.object({ @@ -83,6 +90,30 @@ export async function PUT(request: NextRequest, { params }: RouteParams) { if (!existing) { return sceneApiJson(request, { error: 'not_found' }, { status: 404 }) } + + // A client that autosaves before its initial load resolves sends an empty + // graph — or the bare site/building/level scaffold — and the version still + // matches, so `expectedVersion` waves it straight through and the stored + // scene is destroyed. Refuse to be the last writer in that sequence: a save + // may shrink a scene freely, but it may not take the last content node with + // it unless the caller says so explicitly. + if ( + !parsed.data.allowContentClear && + wouldClearSceneContent(existing.graph, parsed.data.graph) + ) { + return sceneApiJson( + request, + { + error: 'content_clear_rejected', + details: + 'Refusing to replace a non-empty scene with an empty one. Reload the scene, or resend with allowContentClear: true if the clear is intentional.', + existingContentNodes: countContentNodes(existing.graph), + version: existing.version, + }, + { status: 409, headers: { ETag: `"${existing.version}"` } }, + ) + } + const meta = await operations.saveScene({ id, name: parsed.data.name ?? existing.name, diff --git a/apps/editor/lib/scene-content-guard.test.ts b/apps/editor/lib/scene-content-guard.test.ts new file mode 100644 index 0000000000..b6f5717a9d --- /dev/null +++ b/apps/editor/lib/scene-content-guard.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from 'bun:test' +import { countContentNodes, wouldClearSceneContent } from './scene-content-guard' + +const scaffold = { + nodes: { + site_a: { type: 'site' }, + building_a: { type: 'building' }, + level_a: { type: 'level' }, + }, +} + +const house = { + nodes: { + site_a: { type: 'site' }, + building_a: { type: 'building' }, + level_a: { type: 'level' }, + slab_a: { type: 'slab' }, + wall_a: { type: 'wall' }, + wall_b: { type: 'wall' }, + zone_a: { type: 'zone' }, + }, +} + +describe('countContentNodes', () => { + test('ignores the site/building/level scaffold', () => { + expect(countContentNodes(scaffold)).toBe(0) + }) + + test('counts authored nodes only', () => { + expect(countContentNodes(house)).toBe(4) + }) + + test('treats an empty or malformed graph as no content', () => { + expect(countContentNodes({ nodes: {} })).toBe(0) + expect(countContentNodes({})).toBe(0) + expect(countContentNodes(null)).toBe(0) + expect(countContentNodes(undefined)).toBe(0) + expect(countContentNodes({ nodes: { a: null } })).toBe(0) + }) +}) + +describe('wouldClearSceneContent', () => { + test('rejects the empty-graph autosave that wiped the stored scene', () => { + expect(wouldClearSceneContent(house, { nodes: {} })).toBe(true) + }) + + test('rejects a scaffold-only autosave, not just a zero-node one', () => { + // The regression this guards: versions 51/53/55 wrote 3-4 scaffold nodes, + // which a naive "is the graph empty" check would have let through. + expect(wouldClearSceneContent(house, scaffold)).toBe(true) + }) + + test('allows ordinary edits that shrink the scene', () => { + const trimmed = { + nodes: { site_a: { type: 'site' }, level_a: { type: 'level' }, wall_a: { type: 'wall' } }, + } + expect(wouldClearSceneContent(house, trimmed)).toBe(false) + }) + + test('allows growing a scene', () => { + expect(wouldClearSceneContent(scaffold, house)).toBe(false) + }) + + test('allows writing a scaffold over a scene that had no content anyway', () => { + expect(wouldClearSceneContent(scaffold, { nodes: {} })).toBe(false) + }) +}) diff --git a/apps/editor/lib/scene-content-guard.ts b/apps/editor/lib/scene-content-guard.ts new file mode 100644 index 0000000000..a725fb81e5 --- /dev/null +++ b/apps/editor/lib/scene-content-guard.ts @@ -0,0 +1,42 @@ +/** + * Guard against a save that silently destroys an authored scene. + * + * The editor autosaves on a debounce. If that timer fires before the initial + * scene load resolves, the client sends the empty graph it started from — or + * the bare site/building/level scaffold it falls back to. The stored version is + * still the one the client read, so `expectedVersion` matches and optimistic + * concurrency waves the write through. The authored scene is then gone. + * + * Shrinking a scene is legitimate; removing its last authored node as part of + * an unattended write is not distinguishable from the failure above, so callers + * that mean it have to say so explicitly. + */ + +/** + * The scaffold a fresh editor session starts from. These carry no authored + * content on their own, so a graph containing only these is "empty" for the + * purposes of this guard. + */ +export const SCAFFOLD_NODE_TYPES: ReadonlySet = new Set(['site', 'building', 'level']) + +/** Counts nodes a user or agent actually authored (walls, slabs, zones, openings, roofs, items). */ +export function countContentNodes(graph: unknown): number { + const nodes = (graph as { nodes?: Record } | null | undefined) + ?.nodes + if (!nodes || typeof nodes !== 'object') return 0 + + let count = 0 + for (const node of Object.values(nodes)) { + const type = node?.type + if (typeof type === 'string' && !SCAFFOLD_NODE_TYPES.has(type)) count += 1 + } + return count +} + +/** + * True when writing `incoming` over `existing` would strip the scene of every + * authored node. Callers should reject such a write unless it is explicit. + */ +export function wouldClearSceneContent(existing: unknown, incoming: unknown): boolean { + return countContentNodes(existing) > 0 && countContentNodes(incoming) === 0 +} diff --git a/apps/editor/next.config.ts b/apps/editor/next.config.ts index 18fb182064..d01630cb91 100644 --- a/apps/editor/next.config.ts +++ b/apps/editor/next.config.ts @@ -4,6 +4,18 @@ const nextConfig: NextConfig = { logging: { browserToTerminal: true, }, + // The scene store hands out `/editor/` as the canonical project URL + // (`packages/mcp/src/storage/sqlite-scene-store.ts` `editorUrlForScene`), and + // the hosted product serves that path. This app names its route `/scene/[id]`, + // so every MCP-reported `editorUrl` 404s here. + // + // Rewrite rather than redirect: the browser path has to keep the `/editor/` + // prefix because client code parses the project id back out of it (see + // `packages/editor/src/components/ui/action-menu/view-toggles.tsx`, scan + // upload). `/scene/` keeps working for the app's own links. + async rewrites() { + return [{ source: '/editor/:id', destination: '/scene/:id' }] + }, typescript: { ignoreBuildErrors: true, }, diff --git a/apps/editor/package.json b/apps/editor/package.json index 862df6503f..6818c59d14 100644 --- a/apps/editor/package.json +++ b/apps/editor/package.json @@ -4,7 +4,7 @@ "type": "module", "private": true, "scripts": { - "dev": "dotenv -e ../../.env.local -- next dev --port ${PORT:-3002}", + "dev": "dotenv -e ../../.env.local -- next dev --hostname 0.0.0.0 --port 3002", "build": "dotenv -e ../../.env.local -- next build", "start": "next start", "lint": "biome lint",