Skip to content
Merged
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
23 changes: 23 additions & 0 deletions apps/editor/app/api/scenes/[id]/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { type NextRequest, NextResponse } from 'next/server'
import { z } from 'zod'
import { countGraphNodes, isEmptyGraphOverwrite } from '@/lib/empty-graph-guard'
import { apiGraphSchema } from '@/lib/graph-schema'
import {
guardSceneApiRequest,
Expand All @@ -18,6 +19,13 @@ const putSceneSchema = z.object({
graph: apiGraphSchema,
thumbnailUrl: z.string().url().nullable().optional(),
expectedVersion: z.number().int().nonnegative().optional(),
/**
* Overwriting a populated scene with a 0-node graph is rejected (409
* `empty_graph_rejected`) unless this is set: an empty PUT is a hydration
* race or a bug far more often than an intentional full deletion, and the
* wipe is silent while the deletion is recoverable from scene_revisions.
*/
force: z.boolean().optional(),
})

const patchSceneSchema = z.object({
Expand Down Expand Up @@ -83,6 +91,21 @@ export async function PUT(request: NextRequest, { params }: RouteParams) {
if (!existing) {
return sceneApiJson(request, { error: 'not_found' }, { status: 404 })
}
if (
!parsed.data.force &&
isEmptyGraphOverwrite(countGraphNodes(parsed.data.graph), existing.nodeCount)
) {
return sceneApiJson(
request,
{
error: 'empty_graph_rejected',
details: `Refusing to overwrite ${existing.nodeCount} nodes with an empty graph. Pass "force": true to overwrite intentionally.`,
currentVersion: existing.version,
currentNodeCount: existing.nodeCount,
},
{ status: 409 },
)
}
const meta = await operations.saveScene({
id,
name: parsed.data.name ?? existing.name,
Expand Down
30 changes: 30 additions & 0 deletions apps/editor/components/scene-loader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 { countGraphNodes, isEmptyGraphOverwrite } from '@/lib/empty-graph-guard'
import { type PersistedSceneGraph, sceneGraphSignature } from '@/lib/scene-signature'
import { cn } from '@/lib/utils'
import { BuildTab } from './build-tab'
Expand Down Expand Up @@ -95,6 +96,10 @@ export function SceneLoader({ initialScene, meta }: SceneLoaderProps) {
const router = useRouter()
const searchParams = useSearchParams()
const versionRef = useRef(meta.version)
// Node count of the graph the server is known to hold. Guards against the
// autosave wipe class: a save fired from a not-yet-hydrated (empty) editor
// store must never overwrite a populated server copy.
const serverNodeCountRef = useRef(meta.nodeCount)
const lastRemoteGraphJsonRef = useRef<string | null>(null)
const suppressRemoteSaveUntilRef = useRef(0)
const [conflict, setConflict] = useState(false)
Expand All @@ -115,6 +120,19 @@ export function SceneLoader({ initialScene, meta }: SceneLoaderProps) {
}
if (isRecentRemoteApply) return

// Wipe guard: never PUT an empty graph over a populated server copy.
// An empty serialization here means the editor store was not hydrated
// (load in flight or failed), not that the user deleted everything.
const outgoingNodeCount = countGraphNodes(graph)
if (isEmptyGraphOverwrite(outgoingNodeCount, serverNodeCountRef.current)) {
console.error(
`[scene-loader] Blocked autosave: refusing to overwrite scene ${meta.id} ` +
`(${serverNodeCountRef.current} nodes on the server) with an empty graph.`,
)
setSaveError('Autosave blocked: the editor tried to save an empty scene')
return
}

try {
const response = await fetch(`/api/scenes/${meta.id}`, {
method: 'PUT',
Expand All @@ -131,6 +149,16 @@ export function SceneLoader({ initialScene, meta }: SceneLoaderProps) {
})

if (response.status === 409) {
const body = (await response.json().catch(() => null)) as { error?: string } | null
if (body?.error === 'empty_graph_rejected') {
// Server-side wipe guard (defense in depth behind the client-side
// check above) — not a concurrent-session conflict.
console.error(
`[scene-loader] Server rejected an empty-graph save for scene ${meta.id}.`,
)
setSaveError('Autosave blocked: the editor tried to save an empty scene')
return
}
setConflict(true)
return
}
Expand All @@ -142,6 +170,7 @@ export function SceneLoader({ initialScene, meta }: SceneLoaderProps) {

const next = (await response.json()) as SceneMeta
versionRef.current = next.version
serverNodeCountRef.current = next.nodeCount
setSaveError(null)
} catch (error) {
setSaveError(error instanceof Error ? error.message : 'Save failed')
Expand All @@ -164,6 +193,7 @@ export function SceneLoader({ initialScene, meta }: SceneLoaderProps) {
if (payload.version <= versionRef.current) return

versionRef.current = payload.version
serverNodeCountRef.current = countGraphNodes(payload.graph)
lastRemoteGraphJsonRef.current = sceneGraphSignature(payload.graph)
suppressRemoteSaveUntilRef.current = Date.now() + 2500
applySceneGraphToEditor(payload.graph)
Expand Down
150 changes: 150 additions & 0 deletions apps/editor/lib/api-put-empty-guard.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
import { afterAll, beforeAll, expect, test } from 'bun:test'
import { mkdtempSync, rmSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { NextRequest } from 'next/server'

/**
* Integration gate for the scene-wipe class: `PUT /api/scenes/[id]` must
* reject (409 `empty_graph_rejected`) a 0-node graph aimed at a scene that has
* nodes, unless the caller passes `force: true`. Runs the real route handler
* against a real SQLite store in a temp directory.
*/

const tempDir = mkdtempSync(join(tmpdir(), 'scenes-put-guard-'))
const SCENE_ID = 'wipe-guard-scene'

// A minimal graph that passes `apiGraphSchema`: a foreign-typed node is held
// to the BaseNode envelope only, so it stays independent of builtin schemas.
const POPULATED_GRAPH = {
nodes: {
n1: { id: 'n1', type: 'qa:box' },
n2: { id: 'n2', type: 'qa:box' },
},
rootNodeIds: ['n1'],
}
// FILE NAME MATTERS: scene-store-server.test.ts calls mock.module() on
// '@pascal-app/mcp/operations', and bun module mocks leak process-wide to
// every LATER test file in the same worker — this file must sort BEFORE it
// alphabetically to see the real module (CI runs single-worker).
const EMPTY_GRAPH = { nodes: {}, rootNodeIds: [] }

let PUT: typeof import('../app/api/scenes/[id]/route')['PUT']
let restoreEnv: () => void

beforeAll(async () => {
const saved = {
PASCAL_DB_PATH: process.env.PASCAL_DB_PATH,
PASCAL_SCENE_API_TOKEN: process.env.PASCAL_SCENE_API_TOKEN,
}
restoreEnv = () => {
for (const [key, value] of Object.entries(saved)) {
if (value === undefined) delete process.env[key]
else process.env[key] = value
}
}
process.env.PASCAL_DB_PATH = join(tempDir, 'pascal.db')
delete process.env.PASCAL_SCENE_API_TOKEN // loopback requests need no token

const storeServer = await import('./scene-store-server')
storeServer.__resetSceneStoreForTests()

// Build REAL store+operations from relative SOURCE imports and inject
// them: '@pascal-app/mcp/*' subpaths may be mock.module'd by other test
// files in the same process (the stubs stick for later dynamic imports
// on linux), which starved this fixture of saveScene/loadStoredScene in
// CI three runs straight.
const { SqliteSceneStore } = await import('../../../packages/mcp/src/storage/sqlite-scene-store')
const { createSceneOperations } = await import(
'../../../packages/mcp/src/operations/scene-operations'
)
const store = new SqliteSceneStore({ env: process.env })
const operations = createSceneOperations({ store })
storeServer.__setSceneStoreForTests(store, operations)
await store.save({
id: SCENE_ID,
name: 'Wipe guard fixture',
projectId: null,
graph: POPULATED_GRAPH as never,
})

const route = await import('../app/api/scenes/[id]/route')
PUT = route.PUT
})

afterAll(async () => {
const storeServer = await import('./scene-store-server')
const store = await storeServer.getSceneStore()
;(store as unknown as { close?: () => void }).close?.()
storeServer.__resetSceneStoreForTests()
restoreEnv()
rmSync(tempDir, { recursive: true, force: true })
})

function putRequest(body: unknown, ifMatch?: number): NextRequest {
return new NextRequest(`http://127.0.0.1:3000/api/scenes/${SCENE_ID}`, {
method: 'PUT',
headers: {
'Content-Type': 'application/json',
host: '127.0.0.1:3000',
...(ifMatch === undefined ? {} : { 'If-Match': `"${ifMatch}"` }),
},
body: JSON.stringify(body),
})
}

const params = { params: Promise.resolve({ id: SCENE_ID }) }

test('rejects an empty graph over a populated scene with 409 empty_graph_rejected', async () => {
const response = await PUT(putRequest({ graph: EMPTY_GRAPH }, 1), params)

expect(response.status).toBe(409)
const body = (await response.json()) as {
error: string
currentVersion: number
currentNodeCount: number
}
expect(body.error).toBe('empty_graph_rejected')
expect(body.currentVersion).toBe(1)
expect(body.currentNodeCount).toBe(2)
})

test('the rejected PUT leaves the stored scene untouched', async () => {
const storeServer = await import('./scene-store-server')
const operations = await storeServer.getSceneOperations()
const scene = await operations.loadStoredScene(SCENE_ID)

expect(scene?.version).toBe(1)
expect(Object.keys(scene?.graph.nodes ?? {})).toHaveLength(2)
})

test('a populated save still goes through', async () => {
const graph = {
nodes: { ...POPULATED_GRAPH.nodes, n3: { id: 'n3', type: 'qa:box' } },
rootNodeIds: ['n1'],
}
const response = await PUT(putRequest({ graph }, 1), params)

expect(response.status).toBe(200)
const meta = (await response.json()) as { version: number; nodeCount: number }
expect(meta.version).toBe(2)
expect(meta.nodeCount).toBe(3)
})

test('force: true allows an intentional wipe', async () => {
const response = await PUT(putRequest({ graph: EMPTY_GRAPH, force: true }, 2), params)

expect(response.status).toBe(200)
const meta = (await response.json()) as { version: number; nodeCount: number }
expect(meta.version).toBe(3)
expect(meta.nodeCount).toBe(0)
})

test('an empty save over an already-empty scene needs no force', async () => {
const response = await PUT(putRequest({ graph: EMPTY_GRAPH }, 3), params)

expect(response.status).toBe(200)
const meta = (await response.json()) as { version: number; nodeCount: number }
expect(meta.version).toBe(4)
expect(meta.nodeCount).toBe(0)
})
34 changes: 34 additions & 0 deletions apps/editor/lib/empty-graph-guard.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { describe, expect, test } from 'bun:test'
import { countGraphNodes, isEmptyGraphOverwrite } from './empty-graph-guard'

describe('countGraphNodes', () => {
test('counts nodes on a well-formed graph', () => {
expect(countGraphNodes({ nodes: { a: {}, b: {} } })).toBe(2)
})

test('treats missing/odd shapes as empty', () => {
expect(countGraphNodes(null)).toBe(0)
expect(countGraphNodes(undefined)).toBe(0)
expect(countGraphNodes({})).toBe(0)
expect(countGraphNodes({ nodes: null })).toBe(0)
})
})

describe('isEmptyGraphOverwrite', () => {
test('blocks a 0-node write over a populated server copy (the wipe class)', () => {
// Scene-wipe repro 2026-08-18: a pre-hydration autosave flush serialized
// the empty editor store and PUT it over a 74-node scene at If-Match: 1,
// leaving v2 with 0 nodes. This is the exact write that must not pass.
expect(isEmptyGraphOverwrite(0, 74)).toBe(true)
expect(isEmptyGraphOverwrite(0, 1)).toBe(true)
})

test('allows saves that carry nodes', () => {
expect(isEmptyGraphOverwrite(74, 74)).toBe(false)
expect(isEmptyGraphOverwrite(1, 74)).toBe(false)
})

test('allows empty saves over an already-empty scene', () => {
expect(isEmptyGraphOverwrite(0, 0)).toBe(false)
})
})
26 changes: 26 additions & 0 deletions apps/editor/lib/empty-graph-guard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Guard shared by the scene-save client path and the scenes API PUT route:
* an incoming graph with ZERO nodes must never silently replace a server copy
* that has nodes.
*
* Rationale (scene-wipe class, 2026-08-16..18): an editor session whose store
* has not hydrated yet (load in flight, failed GET, pre-hydration flush) can
* serialize an empty graph. Persisting it destroys the scene at the next
* version. Losing a save of a legitimately-emptied scene is far rarer and is
* recoverable (scene_revisions keeps every version), so the trade is blocking
* empty overwrites by default and requiring an explicit `force` to allow them.
*/

export function countGraphNodes(
graph: { nodes?: Record<string, unknown> | null } | null | undefined,
): number {
if (!graph?.nodes || typeof graph.nodes !== 'object') return 0
return Object.keys(graph.nodes).length
}

export function isEmptyGraphOverwrite(
incomingNodeCount: number,
knownServerNodeCount: number,
): boolean {
return incomingNodeCount === 0 && knownServerNodeCount > 0
}
13 changes: 12 additions & 1 deletion apps/editor/lib/scene-store-server.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
import { beforeEach, describe, expect, mock, test } from 'bun:test'
import { afterAll, beforeEach, describe, expect, mock, test } from 'bun:test'

// bun's mock.module poisons the module registry for EVERY test file that
// runs after this one in the same process — capture the real modules and
// restore them when this file finishes, or route tests downstream get a
// stub facade without saveScene/loadStoredScene (night-5 CI failure).
const realOperations = await import('@pascal-app/mcp/operations')
const realStorage = await import('@pascal-app/mcp/storage')
afterAll(() => {
mock.module('@pascal-app/mcp/operations', () => realOperations)
mock.module('@pascal-app/mcp/storage', () => realStorage)
})

describe('getSceneStore', () => {
beforeEach(() => {
Expand Down
11 changes: 11 additions & 0 deletions apps/editor/lib/scene-store-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,14 @@ export function __resetSceneStoreForTests(): void {
cachedStore = null
cachedOperations = null
}

/**
* Test-only injection: other test files in the same bun process may have
* mock.module'd the '@pascal-app/mcp/*' subpaths (the mocks stick for
* later dynamic imports on some platforms), so route tests inject REAL
* instances built from relative source imports instead.
*/
export function __setSceneStoreForTests(store: SceneStore, operations: SceneOperations): void {
cachedStore = Promise.resolve(store)
cachedOperations = Promise.resolve(operations)
}
Loading
Loading