Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions apps/editor/app/api/scenes/[id]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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({
Expand Down Expand Up @@ -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}"` } },
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear rejection shown as conflict

Medium Severity

The new content_clear_rejected error returns HTTP 409, a status code the editor client already uses exclusively for version conflicts. This causes the client to display a misleading "another session saved first" message, obscuring the specific guidance for a blocked content clear.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d90667b. Configure here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rejected clear still enables wipe

High Severity

The new content-clear guard only blocks a PUT whose incoming graph has zero authored nodes. On content_clear_rejected, scene-loader returns without throwing, so autosave marks the run saved and leaves the emptied in-memory graph in place. Any later edit that adds even one content node makes wouldClearSceneContent false, so the next PUT can still replace the full stored scene with that near-empty graph.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d90667b. Configure here.

}

const meta = await operations.saveScene({
id,
name: parsed.data.name ?? existing.name,
Expand Down
67 changes: 67 additions & 0 deletions apps/editor/lib/scene-content-guard.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
42 changes: 42 additions & 0 deletions apps/editor/lib/scene-content-guard.ts
Original file line number Diff line number Diff line change
@@ -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<string> = 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<string, { type?: string } | null> } | 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
}
12 changes: 12 additions & 0 deletions apps/editor/next.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,18 @@ const nextConfig: NextConfig = {
logging: {
browserToTerminal: true,
},
// The scene store hands out `/editor/<id>` 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/<id>` keeps working for the app's own links.
async rewrites() {
return [{ source: '/editor/:id', destination: '/scene/:id' }]
},
typescript: {
ignoreBuildErrors: true,
},
Expand Down
2 changes: 1 addition & 1 deletion apps/editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down