Skip to content

fix(editor): keep the autosave wipe guard armed after the scene loads - #578

Merged
Aymericr merged 1 commit into
mainfrom
fix/autosave-stale-count
Aug 4, 2026
Merged

fix(editor): keep the autosave wipe guard armed after the scene loads#578
Aymericr merged 1 commit into
mainfrom
fix/autosave-stale-count

Conversation

@Aymericr

@Aymericr Aymericr commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

The bug

useAutoSave has a guard that refuses to persist a scene which drops from populated to a bare scaffold (≤ 4 structural nodes), on the assumption that it's an accidental full deletion rather than an intent.

The baseline it measures against was seeded once, at hook mount:

let lastNodeCount = Object.keys(useScene.getState().nodes).length

The hook mounts before the scene loads. So the baseline sat at the scaffold count for the entire session, isSuspiciousNodeDrop(4, 0) is false by design, and the guard could never fire. The one write it exists to stop is precisely the one it let through: an autosave racing the initial load persists the scaffold over a populated stored scene.

The loading branch of the store subscription was already refreshing the snapshot, collections, materials and installed-plugins refs on every load — it just never refreshed the count.

The fix

The baseline moves into createStoredNodeCountTracker, which separates the two cases the single mutable let was conflating:

  • trackLoadedGraph(n) — a graph read from storage is authoritative and becomes the new baseline.
  • allowWrite(n) — an edit is measured against that baseline, and only advances it if allowed.

This also removes the guard duplication between executeSave and flushOnExit (which had drifted to two different warning strings), and makes the invariant testable without pulling react-dom into a repo that has no DOM test setup — the same approach floorplan-camera-sync.ts already uses for its closure state.

Behavior for edits is unchanged: shrinking a populated scene to ≤ 4 nodes is still blocked, an intentionally small scene can still be emptied, and a blocked write no longer risks being adopted as the baseline by a debounced retry.

Credit

Surfaced by @evolv3ai in #551, which caught the data loss and fixed the symptom with a nodeCount === 0 check in the standalone app's PUT /api/scenes/[id] route.

Two reasons to fix it here instead:

  1. apps/editor is one of three consumers of this hook. The hosted editor and npm consumers share useAutoSave but not that route, so a route-level check leaves them exposed.
  2. In apps/editor, a rejected save returns 409, and scene-loader.tsx:131 treats 409 as a conflict-to-reload rather than a failure — it returns without throwing, so autosave marks the run saved. The client-side guard is the one that actually prevents the write.

Verification

  • bun run check — 1587 files, clean
  • bun run check-types — 9/9 tasks
  • bun run test — 12/12 tasks; 7 tests in use-auto-save.test.ts, 5 of them new and covering the load-then-wipe sequence that regressed

🤖 Generated with Claude Code


Note

Medium Risk
Changes persistence guard logic shared by all editor consumers; behavior for legitimate edits is intended to stay the same but incorrect baseline updates could still block or allow bad saves.

Overview
Fixes a data-loss hole in the editor autosave “suspicious node drop” guard by keeping the baseline node count aligned with what was actually loaded from storage, instead of freezing it at hook mount when the scene is still an empty scaffold.

Introduces createStoredNodeCountTracker: trackLoadedGraph updates the baseline when the store subscription runs during isLoadingSceneRef, and allowWrite blocks writes that would shrink a populated scene to ≤4 structural nodes without advancing the baseline (so debounced retries cannot persist a wipe). useAutoSave uses this for both debounced saves and flushOnExit, replacing a single lastNodeCount that never updated on load.

Adds unit tests for load-then-wipe, repeated blocks, normal edits, small empty scenes, and version restore with a smaller graph.

Reviewed by Cursor Bugbot for commit e797382. Bugbot is set up for automated code reviews on this repo. Configure here.

`useAutoSave` refuses to persist a graph that drops from populated to a
bare scaffold, on the assumption that it is an accidental full deletion.
The baseline it compares against was seeded once when the hook mounted —
which happens before the scene has loaded, so it sat at the scaffold
count for the whole session and the guard could never fire. The one write
it exists to stop is the one it let through: an autosave racing the
initial load overwrites the stored scene with the scaffold.

The loading branch of the store subscription already refreshed the
snapshot, collections, materials and plugin refs; it just never refreshed
the count. Rather than add a fourth assignment to a branch whose contract
was implicit, the baseline now lives in `createStoredNodeCountTracker`,
which distinguishes the two things that were being conflated: a graph
read from storage becomes the new baseline, an edited graph does not.
That also removes the duplicated guard between `executeSave` and
`flushOnExit`, and makes the invariant testable without React — the same
approach `floorplan-camera-sync.ts` takes for its closure state.

Surfaced by @evolv3ai in #551, which fixed the symptom with a
`nodeCount === 0` check in the standalone app's save route. This fixes it
in the shared hook instead, so the hosted editor and npm consumers are
covered too, and a blocked write can't be laundered through the 409
conflict path that `scene-loader.tsx` treats as success.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit e797382. Configure here.

const unsubscribe = useScene.subscribe((state) => {
if (isLoadingSceneRef.current) {
lastNodesSnapshot = JSON.stringify(state.nodes)
storedNodeCount.trackLoadedGraph(Object.keys(state.nodes).length)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Load unload disarms wipe guard

High Severity

The storedNodeCountTracker's baseline is incorrectly set to zero during scene loading. When unloadScene clears the graph, trackLoadedGraph updates the baseline before the new scene fully loads. This allows flushOnExit to persist an empty scene if the user navigates away during this loading window, bypassing the intended wipe-guard.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit e797382. Configure here.

@Aymericr
Aymericr merged commit 021cf98 into main Aug 4, 2026
2 checks passed
@Aymericr
Aymericr deleted the fix/autosave-stale-count branch August 4, 2026 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant