feat(assets): add conservative explicit cleanup for local files

[Zhao0335]

Scope

Adds conservative, explicit cleanup for local asset:// Files related to #733. This PR does not automatically delete a File when a node is removed, and it does not fully fix #733. IndexedDB is shared by every scene and tab on the origin; deleting from one active scene could corrupt another saved scene that references the same URL.

Changes

• @pascal-app/core provides deleteAsset, listLocalAssetUrls, sweepLocalAssetsExcept, and collectors for nested asset:// references in nodes and material maps. deleteAsset also revokes its cached object URL.
• apps/editor/lib/local-asset-gc.ts provides an explicit host API. It collects references from the current graph, the localStorage draft, and every server scene. A failed, truncated, or malformed scene inventory (or unparseable local draft) aborts the sweep. The live graph is read again immediately before deletion; both nodes and materials are included, including unsaved texture references added during the inventory walk.
• Automatic mount-time and per-node deletion paths were removed after review. The explicit API has no automatic caller. A host must invoke it only when it can ensure a complete scene inventory, no in-flight writes or uploads, and enough API rate budget.

Review questions

• Delete then recreate / shared URL: No timer runs after a node delete, so recreating a node or sharing a URL cannot race a delayed delete. Explicit GC keeps any URL found in the full persisted inventory or the live graph. There is no per-node or cross-scene reference counter yet.
• Page reload: There is no pending timer to survive reload. Orphans remain until a host explicitly runs GC under the conditions above.

Validation

• bun test apps/editor/lib/local-asset-gc.test.ts — 8 passed, including an IndexedDB File referenced by a live material added during inventory and malformed-inventory guard cases.
• bun run --cwd apps/editor check-types — passed.
• bunx biome check apps/editor/lib/local-asset-gc.ts apps/editor/lib/local-asset-gc.test.ts — passed.
• bunx turbo run build --filter='./apps/editor^...' — passed (existing Turbopack dynamic filesystem tracing warning).

---

Note

Medium Risk
Sweep logic can permanently delete IndexedDB blobs if mis-invoked with an incomplete keep-set, though the implementation aborts on uncertain inventories; shared asset:// handles across scenes make correct union-of-references critical.

Overview
Adds explicit, opt-in garbage collection for local asset:// files in IndexedDB, aimed at #733 without deleting assets when a single node is removed.

Core gains deleteAsset, listLocalAssetUrls, and sweepLocalAssetsExcept, plus deep collectors that walk nodes and material maps for asset:// strings. Deletes also revoke cached object URLs.

Editor exposes runLocalAssetGc, which builds a keep-set from the live graph, the localStorage draft, and every server scene (one GET per scene). If the inventory cannot be proven complete—list failure, malformed payloads, a full 500-scene page, or bad local draft—it returns null and does not sweep. The live graph is read again right before deletion so mid-run uploads and unsaved material textures stay protected.

There is no automatic caller in this PR; hosts must invoke GC only when writes are idle and API budget allows. Tests cover abort paths and live-graph re-read behavior; fake-indexeddb is added for editor tests.

^{Reviewed by Cursor Bugbot for commit eb19a8f. Bugbot is set up for automated code reviews on this repo. Configure here.}

[pascal]

I hit an error while handling your request (Model unavailable on AI Gateway free tier: Free tier users do not have access to this model. Upgrade to paid credits at https://vercel.com/d?to=%2F%5Bteam%5D%2F%7E%2Fai%3Fmodal%3Dtop-up for unrestricted…).

Please try again, rephrase, or reach out if it keeps failing.

Error id: 92c31b27-f9c4-4ce1-a2fb-ce2e00b06d8f

[cursor]

Stale Bugbot comment from a previous run.

[Zhao0335]

Addressed the Bugbot finding in d44b09d: removed the scene-load orphan sweep. IndexedDB is origin-global, so sweeping entries missing from the just-loaded graph could wipe assets still referenced by other scenes. Cleanup is now only the undo-safe delayed delete of a specific �sset:// File after its last live node is removed.

[cursor]

Stale Bugbot comment from a previous run.

[Zhao0335]

Addressed the latest Bugbot finding: delayed deletes now capture a scene epoch at schedule time. �pplySceneGraphToEditor bumps the epoch; timers from a previous epoch refuse to deleteAsset. Switching projects during the grace window no longer deletes origin-global IndexedDB Files that another saved scene still references. Covered by a new unit test.

[Zhao0335]

This finding is already fixed on the current HEAD (cb97731), which is ahead of the reviewed 7ae66a2.

• �pplySceneGraphToEditor bumps a scene epoch (�umpLocalAssetSceneEpoch).
• scheduleLocalAssetDelete captures that epoch at schedule time; the timer returns without deleteAsset if the epoch changed (project/scene switch).
• Covered by unit test: does not delete after a scene switch (another graph may still use the File).

No further code change needed for this comment.

[Aymericr]

Blocker: cleanup is attached to two UI handlers rather than the deletion lifecycle, so the bug still reproduces through supported delete paths.

For example, packages/editor/src/hooks/use-keyboard.ts deletes a selected guide/scan directly with useScene.getState().deleteNode(...), and the generic selection/delete surfaces in selection-routing.ts and group-actions.ts also bypass scheduleLocalAssetDelete. Deleting the same local reference with the Delete key therefore still leaves its asset:// File in IndexedDB.

Please move the scheduling to a single path that observes all committed node removals/replacements, or route every supported guide/scan deletion path through one shared helper. Add a regression test for keyboard deletion (and ideally delete-mode/generic selection deletion) so this cannot remain UI-entry-point dependent. The 120-second grace period, live-reference recheck, cache revocation, and scene-epoch guard are otherwise sensible.

[Zhao0335]

Addressed the deletion-lifecycle blocker in $(git rev-parse --short HEAD):

• Local �sset:// scheduling now lives in @pascal-app/core and runs from deleteNodesAction (and �pplyNodeChanges deletes/url replacements), not the Site/reference panel handlers.
• Covers keyboard Delete (use-keyboard.ts → deleteNode), selection delete, MCP delete_node, and group actions.
• Regression test: packages/core/src/lib/local-asset-lifecycle-delete.test.ts — keyboard-style deleteNode schedules the File; a second live reference keeps it.

Grace period, live-reference recheck, cache revocation, and scene-epoch guard are unchanged.

[cursor]

Stale Bugbot comment from a previous run.

[Aymericr]

The central delete lifecycle now covers the missing keyboard/generic paths, but the current head still has three correctness blockers before cleanup is safe: (1) URL replacement schedules deletion without a fire-time scan of all live nodes, so undo or another node sharing the same asset:// URL can lose its File; (2) the reference-panel replacement schedules before updateNode commits, so a no-op/throw can delete the still-current asset; and (3) the scene epoch only advances in applySceneGraphToEditor, while unload/import/reset paths can replace the graph through setScene/clearScene and leave an old timer valid. Please make the timer predicate query the current graph for the URL at fire time, schedule replacements only after a committed change, and put epoch invalidation at the shared graph-replacement boundary. CI also needs the three Biome organize-import fixes shown by quality. Add regressions for shared-reference replacement, undo restoration, failed/no-op replacement, and a clear/set scene transition.

[Zhao0335]

Addressed the three correctness blockers and Biome organizeImports on $head:

1. Replacement timers re-query the live graph for the URL at fire time (collectSceneAssetUrls(get().nodes)), so undo or a duplicate guide sharing the same �sset:// handle keeps its File.
2. Guide replace no longer schedules before updateNode; only the core updateNodes path schedules after the url change commits.
3. Epoch is bumped in unloadScene and setScene (shared graph-replacement boundary), not only �pplySceneGraphToEditor.

Regressions added for shared-reference replacement, undo restoration, unreferenced replacement, and setScene epoch. packages/core: 1545 tests pass.

[Aymericr]

The new fire-time live-graph checks fix the reported undo/shared-node cases, but there is still a cross-scene data-loss blocker. asset:// storage is origin-global, and scene/project duplication preserves those URLs. If scene A and an unopened saved scene B reference the same File, deleting the last reference in the currently loaded A schedules deleteAsset; staying in A for 120 seconds means the timer sees no current reference and deletes B’s File. The epoch only protects a scene switch during the grace window, not references in persisted scenes that were never loaded. This is the same ownership problem that made the original sweep unsafe.

Please do not physically delete origin-global Files based only on the active graph. The cleanup path needs a durable cross-scene reference index/refcount (updated atomically with scene persistence), per-scene asset ownership/copying, or a conservative explicit garbage-collection pass that can inspect every saved graph. Until one of those exists, this PR trades an orphan leak for silent corruption of another project. Add a regression with two persisted scenes sharing one asset URL; deleting from one must not break the other.

[cursor]

Stale Bugbot comment from a previous run.

[Zhao0335]

Addressed the cross-scene data-loss blocker in 55e04ae.

No longer deletes origin-global Files from the active graph.

1. Removed automatic deleteAsset from deleteNodes / updateNodes timers.
2. Core now exposes explicit GC: sweepLocalAssetsExcept(keepUrls).
3. Standalone editor GC (apps/editor/lib/local-asset-gc.ts) builds the keep-set from current scene + localStorage scene + every /api/scenes graph. If any source cannot be enumerated, it skips the sweep rather than partial-deleting.
4. Regression test: two persisted scenes share one asset:// URL; sweeping with both keep-sets keeps the File.

Orphan leak may remain until a successful full enumeration runs — that is preferred over silent corruption of another project.

[cursor]

Stale Bugbot comment from a previous run.

[Zhao0335]

Fixed the two High findings on 0023ec0:

1. Truncated inventory — /api/scenes has no cursor; a full page (500) is treated as incomplete and GC skips instead of sweeping. Listing failure still skips.
2. In-flight uploads —
   unLocalAssetGc takes a live-node getter and re-reads useScene immediately before sweepLocalAssetsExcept, and keeps initialScene urls in the extra keep-set.

Unit tests cover truncated list, failed list, and mid-GC live-graph growth.

[cursor]

Stale Bugbot comment from a previous run.

[Zhao0335]

Addressed the mount-time GC findings on 3cc6f50:

• Removed automatic runLocalAssetGc from SceneLoader mount.
• runLocalAssetGc remains an explicit host API only (documented: no in-flight writes, complete inventory, spare rate budget).
• Sequential per-scene GETs no longer run on page open, so they cannot starve autosave's rate bucket.

This leaves possible orphans in IndexedDB rather than deleting Files another tab, keepalive flush, or unpersisted upload still needs.

[cursor]

Stale Bugbot comment from a previous run.

[Zhao0335]

Fixed keep-set completeness on 276e5b0:

• collectNodeAssetUrls now deep-walks the node (depth 8, cycle-safe) so item.asset.src / thumbnail, captureSession.manifestUrl, and other nested asset:// strings are included.
• New collectGraphAssetUrlsFromParts also walks graph.materials texture maps.
• Unit tests cover nested item fields and materials maps.

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 276e5b0. Configure here.}

[dkbbdev]

Reviewing this against #733 — verified the fix routes local asset:// URLs into the delayed-delete scheduler so deleting a scan/guide node no longer orphans its IndexedDB File (the ~200 MB case is the one that matters on memory-constrained devices).

Questions while reading:

1. Delete-then-recreate race — if a user deletes a scan node and immediately re-adds a guide referencing the same underlying File before the grace period elapses, does the scheduled delete get cancelled or does it remove the File out from under the new node?
2. Ref-counting scope — two nodes sharing one asset URL is the obvious next failure mode; is that handled or explicitly out of scope here? If out of scope, a short note in the PR body would help future readers.
3. Minor: the scheduler timer survives page reload? (IndexedDB File pending delete + reload → does the pending entry get re-scheduled or leaked until the next delete?)

[Zhao0335]

Thanks for the careful questions. The PR had changed substantially since its original description, so I corrected the title and body to match the current code.

1. Delete then recreate: There is no scheduled delete on node removal now. The earlier timer was removed after the cross-scene data-loss review, so recreating a reference during a grace period cannot race it.
2. Shared URL: There is no refcount. The explicit GC path gathers references from the live graph, the localStorage draft, and all server scenes; if the server inventory fails or is truncated, it skips deletion. b8a6af37 also makes it retain live material textures, including those added while the inventory is running. The new regression uses a real IndexedDB File.
3. Reload: There is no pending timer to resume. Orphans remain until a host explicitly invokes GC when it can ensure a complete inventory and no in-flight writes.

This means the PR is groundwork for #733, not a complete automatic fix. I removed the closing Fixes #733 language from the description and made that limitation explicit. Local targeted tests, TypeScript, Biome, and the dependency build pass; GitHub CI is running on the new head.

[Zhao0335]

@Aymericr The review threads are resolved, and the latest push fixes the live-material keep-set and makes GC abort on malformed or incomplete scene inventories. All current checks pass. Could you take another look? The PR description now states clearly that this is an explicit cleanup API and does not fully close #733.

[Zhao0335]

Merged current main (c5eaf51) in eb19a8f and resolved the core export conflict, preserving both the new upstream exports and the local asset APIs. I also removed leftover comments and hook dependencies from the earlier automatic-cleanup implementation. Cleanup remains explicit, with no mount/load caller.

Local validation: bun install --frozen-lockfile, bun run check, all 13 check-types tasks, and all 23 focused asset storage/collection/GC tests passed. The standalone app's full lib suite also passed (59 tests). The full monorepo run is not green on Windows: unchanged upstream checks hit Windows path/POSIX-mode assumptions and a geometry golden comparison, and standalone history probes hit module resolution issues. No unrelated fixes are included here.

GitHub now reports the PR as mergeable. The new CI run (quality and macOS cli-smoke) and mcp-ci run have all passed at eb19a8f. Ready for another review of the explicit-cleanup scope.