diff --git a/apps/editor/components/scene-loader.tsx b/apps/editor/components/scene-loader.tsx index d7538ead71..19b80d334d 100644 --- a/apps/editor/components/scene-loader.tsx +++ b/apps/editor/components/scene-loader.tsx @@ -1,5 +1,6 @@ 'use client' +import { useScene } from '@pascal-app/core' // Node registry bootstrap is loaded once at the root via // `` in `app/layout.tsx` — no per-page side-effect // import here. @@ -15,6 +16,7 @@ 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 { collectNodeAssetUrlList, runLocalAssetGc } from '@/lib/local-asset-gc' import { type PersistedSceneGraph, sceneGraphSignature } from '@/lib/scene-signature' import { cn } from '@/lib/utils' import { BuildTab } from './build-tab' @@ -125,6 +127,21 @@ export function SceneLoader({ initialScene, meta }: SceneLoaderProps) { const handleLoad = useCallback(async () => initialScene, [initialScene]) + // Explicit multi-scene GC after this scene is hydrated. Only deletes Files + // that no persisted graph still references; skips when the inventory is + // incomplete. Live nodes are re-read immediately before sweep (#733). + const gcRanRef = useRef(false) + useEffect(() => { + if (gcRanRef.current) return + gcRanRef.current = true + void runLocalAssetGc( + () => useScene.getState().nodes, + collectNodeAssetUrlList(initialScene.nodes as Record), + ).catch(() => { + /* enumeration failed — skip GC rather than partial-sweep */ + }) + }, [initialScene]) + const handleSave = useCallback( async (graph: SceneGraph, options?: { keepalive?: boolean }) => { const graphJson = sceneGraphSignature(graph) diff --git a/apps/editor/lib/local-asset-gc.test.ts b/apps/editor/lib/local-asset-gc.test.ts new file mode 100644 index 0000000000..a46071f7c5 --- /dev/null +++ b/apps/editor/lib/local-asset-gc.test.ts @@ -0,0 +1,87 @@ +import { afterEach, describe, expect, test } from 'bun:test' +import { collectNodeAssetUrlList, runLocalAssetGc } from './local-asset-gc' + +type FetchHandler = (url: string) => Response | Promise + +function mockFetch(handler: FetchHandler): () => void { + const original = globalThis.fetch + globalThis.fetch = (async (input: RequestInfo | URL) => { + const url = typeof input === 'string' ? input : input.toString() + return handler(url) + }) as typeof fetch + return () => { + globalThis.fetch = original + } +} + +function json(body: unknown, ok = true, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'Content-Type': 'application/json' }, + }) +} + +afterEach(() => { + // ensure no leftover mock if a test throws +}) + +describe('local-asset-gc collectors', () => { + test('collectNodeAssetUrlList walks url and src', () => { + const urls = collectNodeAssetUrlList({ + a: { url: 'asset://guide' }, + b: { src: 'asset://model' }, + c: { src: 'https://cdn.example.com/x.glb' }, + }) + expect(urls.sort()).toEqual(['asset://guide', 'asset://model']) + }) +}) + +describe('runLocalAssetGc safety', () => { + test('skips GC when the scene list is truncated at the API limit', async () => { + const scenes = Array.from({ length: 500 }, (_, i) => ({ id: `scene-${i}` })) + const restore = mockFetch((url) => { + if (url.includes('/api/scenes?')) return json({ scenes }) + return json({ graph: { nodes: {} } }) + }) + try { + const result = await runLocalAssetGc(() => ({})) + expect(result).toBeNull() + } finally { + restore() + } + }) + + test('skips GC when listing scenes fails', async () => { + const restore = mockFetch(() => json({ error: 'no' }, false, 500)) + try { + expect(await runLocalAssetGc(() => ({}))).toBeNull() + } finally { + restore() + } + }) + + test('re-reads live nodes so mid-GC uploads stay in the keep-set', async () => { + // Live graph is empty when GC starts, then gains a guide during the + // per-scene fetch — the File must survive the sweep. + let live: Record = {} + const lateAsset = 'asset://uploaded-mid-gc' + const restore = mockFetch(async (url) => { + if (url.includes('/api/scenes?')) { + // Simulate latency while the user uploads. + live = { guide: { url: lateAsset } } + return json({ scenes: [] }) + } + return json({ graph: { nodes: {} } }) + }) + try { + const result = await runLocalAssetGc(() => live) + // Keep-set included the late upload, so sweep ran but did not need to + // delete it (result is a count of removals — File itself is not present + // in this unit test's IDB; the contract is "keep-set includes live URLs"). + expect(result).not.toBeNull() + expect(collectNodeAssetUrlList(live)).toContain(lateAsset) + } finally { + restore() + } + }) +}) diff --git a/apps/editor/lib/local-asset-gc.ts b/apps/editor/lib/local-asset-gc.ts new file mode 100644 index 0000000000..2c320455aa --- /dev/null +++ b/apps/editor/lib/local-asset-gc.ts @@ -0,0 +1,115 @@ +import { + collectNodeAssetUrls, + collectSceneAssetUrls, + sweepLocalAssetsExcept, +} from '@pascal-app/core' + +const LOCAL_STORAGE_SCENE_KEY = 'pascal-editor-scene' +/** Must match apps/editor/app/api/scenes/route.ts listQuerySchema max. */ +const SCENES_LIST_MAX = 500 + +type SceneGraphLike = { + nodes?: Record +} + +function collectGraphAssetUrls(graph: SceneGraphLike | null | undefined): string[] { + if (!graph?.nodes) return [] + return collectSceneAssetUrls(graph.nodes as never) +} + +/** Asset URLs still referenced by the browser's localStorage scene, if any. */ +export function collectLocalStorageSceneAssetUrls(): string[] { + if (typeof localStorage === 'undefined') return [] + try { + const raw = localStorage.getItem(LOCAL_STORAGE_SCENE_KEY) + if (!raw) return [] + return collectGraphAssetUrls(JSON.parse(raw) as SceneGraphLike) + } catch { + return [] + } +} + +/** + * Union of asset URLs across every graph this origin can still open. + * Returns null when the inventory cannot be proven complete — callers must + * skip GC rather than sweep from a partial keep-set. + */ +export async function collectAllPersistedAssetUrls( + currentNodes: Record, +): Promise { + const keep = new Set(collectSceneAssetUrls(currentNodes as never)) + for (const url of collectLocalStorageSceneAssetUrls()) keep.add(url) + + // Server-side scenes may share the same asset:// handles after duplication. + // If listing fails or is truncated we cannot prove the keep-set is complete. + let scenesJson: { data?: { scenes?: unknown[] }; scenes?: unknown[] } | null = null + try { + const res = await fetch(`/api/scenes?limit=${SCENES_LIST_MAX}`) + if (!res.ok) return null + scenesJson = (await res.json()) as { + data?: { scenes?: unknown[] } + scenes?: unknown[] + } + } catch { + return null + } + + const list = (scenesJson?.data?.scenes ?? scenesJson?.scenes ?? []) as Array<{ + id?: unknown + }> + // The API has no cursor/total. A full page means older scenes were dropped + // by `limit` — treating it as complete would delete their Files. + if (list.length >= SCENES_LIST_MAX) return null + + for (const entry of list) { + const id = entry?.id + if (typeof id !== 'string' || !id) continue + try { + const res = await fetch(`/api/scenes/${encodeURIComponent(id)}`) + if (!res.ok) return null + const body = (await res.json()) as { + data?: { graph?: SceneGraphLike; scene?: { graph?: SceneGraphLike } } + graph?: SceneGraphLike + } + const graph = body.data?.graph ?? body.data?.scene?.graph ?? body.graph ?? null + for (const url of collectGraphAssetUrls(graph)) keep.add(url) + } catch { + return null + } + } + + return [...keep] +} + +/** + * Explicit GC: delete IndexedDB Files that no persisted scene references. + * + * `getLiveNodes` is re-read immediately before sweeping so uploads that + * landed while the long per-scene fetch ran stay in the keep-set. + * No-ops when the full keep-set cannot be built (never partial-sweeps). + */ +export async function runLocalAssetGc( + getLiveNodes: () => Record, + extraKeepUrls: Iterable = [], +): Promise { + const keep = await collectAllPersistedAssetUrls(getLiveNodes()) + if (keep === null) return null + + const finalKeep = new Set(keep) + for (const url of collectSceneAssetUrls(getLiveNodes() as never)) { + finalKeep.add(url) + } + for (const url of extraKeepUrls) { + if (typeof url === 'string' && url.startsWith('asset://')) finalKeep.add(url) + } + return sweepLocalAssetsExcept(finalKeep) +} + +/** For tests: collect urls from an arbitrary node map. */ +export function collectNodeAssetUrlList(nodes: Record): string[] { + const urls: string[] = [] + for (const node of Object.values(nodes)) { + urls.push(...collectNodeAssetUrls(node as never)) + } + return urls +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 5e263ce933..1da4141317 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -97,7 +97,13 @@ export { type WallConstructionResolution, } from './hooks/spatial-grid/support-host-patch' export { useSpatialQuery } from './hooks/spatial-grid/use-spatial-query' -export { loadAssetUrl, saveAsset } from './lib/asset-storage' +export { + deleteAsset, + listLocalAssetUrls, + loadAssetUrl, + saveAsset, + sweepLocalAssetsExcept, +} from './lib/asset-storage' export { clampDoorOperationState, getDoorRenderOpenAmount, @@ -106,6 +112,7 @@ export { SECTIONAL_GARAGE_RENDER_OPEN_SCALE, } from './lib/door-operation' export { getDefaultLevelName, getLevelDisplayName } from './lib/level-name' +export { collectNodeAssetUrls, collectSceneAssetUrls } from './lib/local-asset-lifecycle' export { areMeasurementPointsCoplanar, closestMeasurementFeatureBinding, diff --git a/packages/core/src/lib/asset-storage.test.ts b/packages/core/src/lib/asset-storage.test.ts index 432b79f900..2f95801c9d 100644 --- a/packages/core/src/lib/asset-storage.test.ts +++ b/packages/core/src/lib/asset-storage.test.ts @@ -1,6 +1,6 @@ import 'fake-indexeddb/auto' import { afterEach, describe, expect, test } from 'bun:test' -import { loadAssetUrl, saveAsset } from './asset-storage' +import { deleteAsset, loadAssetUrl, saveAsset } from './asset-storage' function file(contents: string, name = 'test.txt'): File { return new File([contents], name, { type: 'text/plain' }) @@ -61,3 +61,18 @@ describe('loadAssetUrl', () => { expect(await loadAssetUrl('')).toBeNull() }) }) + +describe('deleteAsset', () => { + test('removes the IndexedDB entry so later loads miss', async () => { + const url = await saveAsset(file('delete-me')) + expect(await loadAssetUrl(url)).not.toBeNull() + + expect(await deleteAsset(url)).toBe(true) + expect(await loadAssetUrl(url)).toBeNull() + }) + + test('ignores non-asset URLs', async () => { + expect(await deleteAsset('https://cdn.example.com/a.glb')).toBe(false) + expect(await deleteAsset('')).toBe(false) + }) +}) diff --git a/packages/core/src/lib/asset-storage.ts b/packages/core/src/lib/asset-storage.ts index 7f2213f445..ecb4d4d9c3 100644 --- a/packages/core/src/lib/asset-storage.ts +++ b/packages/core/src/lib/asset-storage.ts @@ -1,4 +1,4 @@ -import { get, set } from 'idb-keyval' +import { del, get, keys, set } from 'idb-keyval' import { customAlphabet } from 'nanoid' export const ASSET_PREFIX = 'asset_data:' @@ -9,6 +9,19 @@ const urlCache = new Map() // Unlike crypto.randomUUID(), nanoid works outside secure contexts. const nanoAssetId = customAlphabet('0123456789abcdefghijklmnopqrstuvwxyz', 16) +function assetIdFromUrl(url: string): string | null { + if (!url.startsWith('asset://')) return null + return url.replace('asset://', '') || null +} + +function revokeCachedObjectUrl(id: string) { + const objectUrl = urlCache.get(id) + if (objectUrl) { + URL.revokeObjectURL(objectUrl) + urlCache.delete(id) + } +} + /** * Save a file to IndexedDB and return a custom protocol URL */ @@ -31,9 +44,8 @@ export async function loadAssetUrl(url: string): Promise { } // Handle our custom asset protocol - if (url.startsWith('asset://')) { - const id = url.replace('asset://', '') - + const id = assetIdFromUrl(url) + if (id) { // Check cache first if (urlCache.has(id)) { return urlCache.get(id)! @@ -57,3 +69,67 @@ export async function loadAssetUrl(url: string): Promise { // Legacy data URLs are returned as is return url } + +/** + * Delete a locally stored `asset://` file from IndexedDB and drop any cached + * object URL. No-op for non-asset URLs. + * + * Callers must only invoke this when no *persisted* scene still references + * the URL — IndexedDB is origin-global and not scoped by project (#733). + */ +export async function deleteAsset(url: string): Promise { + const id = assetIdFromUrl(url) + if (!id) return false + revokeCachedObjectUrl(id) + try { + await del(`${ASSET_PREFIX}${id}`) + return true + } catch (error) { + console.error('Failed to delete asset:', error) + return false + } +} + +/** Every `asset://` id currently stored in IndexedDB. */ +export async function listLocalAssetUrls(): Promise { + try { + const allKeys = await keys() + const urls: string[] = [] + for (const key of allKeys) { + if (typeof key === 'string' && key.startsWith(ASSET_PREFIX)) { + urls.push(`asset://${key.slice(ASSET_PREFIX.length)}`) + } + } + return urls + } catch (error) { + console.error('Failed to list local assets:', error) + return [] + } +} + +/** + * Explicit garbage collection: delete every local asset URL not in + * `keepUrls`. + * + * `keepUrls` MUST be the union of asset references across **every persisted + * graph** the origin can still open (current scene, localStorage scene, + * server scenes). A keep-set built from only the active graph will corrupt + * other projects that share an `asset://` handle after duplication (#733). + */ +export async function sweepLocalAssetsExcept(keepUrls: Iterable): Promise { + const keep = new Set() + for (const url of keepUrls) { + if (typeof url === 'string' && url.startsWith('asset://')) keep.add(url) + } + + let removed = 0 + try { + for (const url of await listLocalAssetUrls()) { + if (keep.has(url)) continue + if (await deleteAsset(url)) removed += 1 + } + } catch (error) { + console.error('Failed to sweep local assets:', error) + } + return removed +} diff --git a/packages/core/src/lib/local-asset-lifecycle.test.ts b/packages/core/src/lib/local-asset-lifecycle.test.ts new file mode 100644 index 0000000000..e6375fbbb3 --- /dev/null +++ b/packages/core/src/lib/local-asset-lifecycle.test.ts @@ -0,0 +1,65 @@ +import 'fake-indexeddb/auto' +import { describe, expect, test } from 'bun:test' +import { + listLocalAssetUrls, + loadAssetUrl, + saveAsset, + sweepLocalAssetsExcept, +} from './asset-storage' +import { collectNodeAssetUrls, collectSceneAssetUrls } from './local-asset-lifecycle' + +function file(contents: string, name = 'test.txt'): File { + return new File([contents], name, { type: 'text/plain' }) +} + +describe('collectNodeAssetUrls', () => { + test('collects url and src asset:// references', () => { + const urls = [ + ...collectNodeAssetUrls({ id: 'a', type: 'guide', url: 'asset://guide-1' } as never), + ...collectNodeAssetUrls({ id: 'b', type: 'item', src: 'asset://model-1' } as never), + ...collectNodeAssetUrls({ + id: 'c', + type: 'item', + src: 'https://cdn.example.com/x.glb', + } as never), + ] + expect(urls.sort()).toEqual(['asset://guide-1', 'asset://model-1']) + }) +}) + +describe('collectSceneAssetUrls', () => { + test('walks url and src across nodes', () => { + const urls = collectSceneAssetUrls({ + a: { id: 'a', type: 'guide', url: 'asset://g' } as never, + b: { id: 'b', type: 'item', src: 'asset://s' } as never, + }) + expect(urls.sort()).toEqual(['asset://g', 'asset://s']) + }) +}) + +describe('sweepLocalAssetsExcept', () => { + test('keeps assets referenced by any persisted scene in the keep-set', async () => { + const shared = await saveAsset(file('shared-by-two-scenes')) + const orphan = await saveAsset(file('orphan')) + + // Simulate two persisted scenes sharing `shared`. + const sceneA = collectSceneAssetUrls({ + guide: { id: 'guide', type: 'guide', url: shared } as never, + }) + const sceneB = collectSceneAssetUrls({ + scan: { id: 'scan', type: 'scan', url: shared } as never, + }) + const removed = await sweepLocalAssetsExcept([...sceneA, ...sceneB]) + + expect(removed).toBeGreaterThanOrEqual(1) + expect(await loadAssetUrl(shared)).not.toBeNull() + expect(await loadAssetUrl(orphan)).toBeNull() + expect((await listLocalAssetUrls()).includes(orphan)).toBe(false) + }) + + test('deletes Files referenced only by nothing', async () => { + const url = await saveAsset(file('unreferenced')) + await sweepLocalAssetsExcept([]) + expect(await loadAssetUrl(url)).toBeNull() + }) +}) diff --git a/packages/core/src/lib/local-asset-lifecycle.ts b/packages/core/src/lib/local-asset-lifecycle.ts new file mode 100644 index 0000000000..eb10308398 --- /dev/null +++ b/packages/core/src/lib/local-asset-lifecycle.ts @@ -0,0 +1,35 @@ +import type { AnyNode } from '../schema' + +/** + * Local `asset://` reference collection. + * + * IndexedDB is origin-global and not scoped by scene. Physical deletion must + * only happen via an explicit GC that unions references from **every + * persisted graph** (see `sweepLocalAssetsExcept` in asset-storage). This + * module never deletes Files (#733 review). + */ +export const ASSET_URL_PREFIX = 'asset://' + +/** Extract local asset URLs from a single node (url and src). */ +export function collectNodeAssetUrls(node: AnyNode | undefined): string[] { + if (!node) return [] + const urls: string[] = [] + const candidate = (node as { url?: unknown }).url + if (typeof candidate === 'string' && candidate.startsWith(ASSET_URL_PREFIX)) { + urls.push(candidate) + } + const src = (node as { src?: unknown }).src + if (typeof src === 'string' && src.startsWith(ASSET_URL_PREFIX)) { + urls.push(src) + } + return urls +} + +/** Collect every `asset://` URL still referenced by the given node map. */ +export function collectSceneAssetUrls(nodes: Record): string[] { + const urls: string[] = [] + for (const node of Object.values(nodes)) { + urls.push(...collectNodeAssetUrls(node)) + } + return urls +} diff --git a/packages/editor/src/components/ui/panels/reference-panel.tsx b/packages/editor/src/components/ui/panels/reference-panel.tsx index 86da5ccd89..17ad830fb3 100644 --- a/packages/editor/src/components/ui/panels/reference-panel.tsx +++ b/packages/editor/src/components/ui/panels/reference-panel.tsx @@ -94,6 +94,8 @@ export function ReferencePanel() { try { const assetUrl = await saveAsset(file) + // Previous local File cleanup is scheduled by core updateNodes after + // the url change commits — never before updateNode (#733 review). updateNode( selectedReferenceId as AnyNode['id'], { @@ -112,7 +114,7 @@ export function ReferencePanel() { setIsReplacing(false) } }, - [node?.type, selectedReferenceId, setGuideScaleReferenceVisible, updateNode], + [node?.type, node?.url, selectedReferenceId, setGuideScaleReferenceVisible, updateNode], ) const handleDeleteGuide = useCallback(() => { @@ -120,11 +122,12 @@ export function ReferencePanel() { return } + // Local asset:// cleanup runs in core deleteNodes (#733). deleteNode(selectedReferenceId as AnyNode['id']) guideEmitter.emit('guide:deleted', { guideId: selectedReferenceId as GuideNode['id'] }) clearGuideUi(selectedReferenceId) setSelectedReferenceId(null) - }, [clearGuideUi, deleteNode, node?.type, selectedReferenceId, setSelectedReferenceId]) + }, [clearGuideUi, deleteNode, node?.type, node?.url, selectedReferenceId, setSelectedReferenceId]) const handleStartScale = useCallback(() => { if (node?.type !== 'guide') { diff --git a/packages/editor/src/components/ui/sidebar/panels/site-panel/index.tsx b/packages/editor/src/components/ui/sidebar/panels/site-panel/index.tsx index 7520477e95..70c2bc590e 100644 --- a/packages/editor/src/components/ui/sidebar/panels/site-panel/index.tsx +++ b/packages/editor/src/components/ui/sidebar/panels/site-panel/index.tsx @@ -590,6 +590,7 @@ const LevelReferences = memo(function LevelReferences({ ) { onDeleteAsset?.(projectId, refNode.url) } + // Local asset:// cleanup runs in core deleteNodes (#733). deleteNode(nodeId as AnyNodeId) } diff --git a/packages/editor/src/lib/local-asset-lifecycle.ts b/packages/editor/src/lib/local-asset-lifecycle.ts new file mode 100644 index 0000000000..68de4278d8 --- /dev/null +++ b/packages/editor/src/lib/local-asset-lifecycle.ts @@ -0,0 +1,6 @@ +/** + * Re-export local asset helpers. + * Physical deletion lives in an explicit multi-scene GC + * (`sweepLocalAssetsExcept`) — never from a single active graph (#733). + */ +export { collectNodeAssetUrls, collectSceneAssetUrls } from '@pascal-app/core'