fix(site-builder): disarm Keep mine on empty-banner Undo; autosaveDue honours conflict

Refs #31

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

craft/src/editor/LoadGate.conflict.test.tsx

@@ -4,10 +4,10 @@ import { createRoot, Root } from 'react-dom/client';
 import { act } from 'react-dom/test-utils';
 
 /**
- * Data-safety: a failed initial load must never let autosave overwrite the
- * saved site with a blank canvas. Drives the real LoadGate + TopBar +
- * SiteDesignPanel against a mocked `fetch` (the backend contract) and fake
- * timers; only `@craftjs/core`'s useEditor and sibling chrome are stubbed.
+ * Two-tab conflict handling: rev tracking, base_rev on every save, and the
+ * 409 conflict modal (Reload latest / Keep mine). Drives the real LoadGate +
+ * TopBar + SiteDesignPanel against a mocked `fetch` and fake timers; only
+ * `@craftjs/core`'s useEditor and sibling chrome are stubbed.
  */
 const undoMock = vi.fn();
 const deserializeMock = vi.fn();
@@ -84,6 +84,7 @@ function mount() {
 beforeEach(() => {
   vi.useFakeTimers();
   fetchMock = vi.fn();
+  reloadMock.mockReset();
   vi.stubGlobal('fetch', fetchMock);
   vi.spyOn(console, 'error').mockImplementation(() => {});
   undoMock.mockClear();
@@ -99,8 +100,6 @@ afterEach(() => {
 });
 
 
-beforeEach(() => { reloadMock.mockReset(); });
-
 const loadedThen = (rev: string | null, saveResponses: unknown[]) => {
   const saves = [...saveResponses];
   fetchMock.mockImplementation(async (url: string) =>
@@ -189,14 +188,12 @@ describe('conflict modal', () => {
     mount();
     await flush();
     await clickSave();
-    const prevented: boolean[] = [];
     act(() => { guard.markDirty(); });
     act(() => { q('[data-testid="conflict-reload"]')!.click(); });
     const ev = new Event('beforeunload', { cancelable: true });
     window.dispatchEvent(ev);
-    prevented.push(ev.defaultPrevented);
     expect(reloadMock).toHaveBeenCalledTimes(1);
-    expect(prevented).toEqual([false]);
+    expect(ev.defaultPrevented).toBe(false);
     expect(saveCalls()).toHaveLength(1);
   });
 
@@ -230,3 +227,36 @@ describe('conflict modal', () => {
     expect(saveBody(2).force_empty).toBe(true);
   });
 });
+
+describe('revision and overwrite edge cases', () => {
+  test('a brand-new site (rev null) saves, then the next save sends the returned rev', async () => {
+    loadedThen(null, [jsonResp({ success: true, rev: 'rX' })]);
+    mount();
+    await flush();
+    await clickSave();
+    await clickSave();
+    expect(saveBody(0).base_rev).toBeNull();
+    expect(saveBody(1).base_rev).toBe('rX');
+  });
+
+  test('Keep mine -> empty refusal -> Undo disarms force_overwrite: the next autosave is conflict-checked', async () => {
+    loadedThen('r1', [
+      conflict409,
+      jsonResp({ success: false, error: 'empty_overwrite_refused', message: 'e' }, 409),
+      jsonResp({ success: true, rev: 'r2' }),
+    ]);
+    mount();
+    await flush();
+    await clickSave();
+    await act(async () => { q('[data-testid="conflict-keep-mine"]')!.click(); });
+    await flush();
+    expect(saveBody(1).force_overwrite).toBe(true);
+    act(() => { q('[data-testid="empty-save-undo"]')!.click(); });
+    act(() => { guard.markDirty(); });
+    await act(async () => { vi.advanceTimersByTime(30000); });
+    await flush();
+    expect(saveCalls()).toHaveLength(3);
+    expect(saveBody(2).force_overwrite).toBeUndefined();
+    expect(saveBody(2).base_rev).toBe('r1');
+  });
+});

craft/src/panels/topbar/ConflictModal.tsx

@@ -1,8 +1,6 @@
 import React from 'react';
 
 export interface ConflictModalProps {
-  /** Why the save was refused (the server's message); a fixed fallback when empty. */
-  message?: string | null;
   /** True while "Keep mine" is re-saving. */
   busy: boolean;
   /** Set when "Keep mine" failed, so the customer sees why the modal is still up. */

craft/src/panels/topbar/TopBar.tsx

@@ -37,7 +37,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
   const { save, publish } = useWhpApi();
   const {
     loadState, emptyRefusal, setEmptyRefusal, dirty, isDirty, getVersion,
-    conflict, setConflict, armForceOverwriteNextSave, reloadLatest,
+    conflict, setConflict, armForceOverwriteNextSave, disarmForceOverwriteNextSave, reloadLatest,
   } = useSaveGuard();
   const canSave = loadState === 'loaded';
   const { headerPage, footerPage, pages, activePageId } = usePages();
@@ -59,6 +59,9 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
   const { open: openSitesmith } = useSitesmithModal();
   const savingRef = useRef(false);
   const publishingRef = useRef(false);
+  // Read at tick time by autosaveDue, never a stale closure.
+  const conflictRef = useRef<string | null>(null);
+  conflictRef.current = conflict;
 
   // An edit after a publish means the live site no longer matches the editor.
   useEffect(() => {
@@ -115,7 +118,9 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
     if (!isWHP || !canSave || emptyRefusal || conflict) return;
     // A running publish does its own save; a second one would race it.
     // Read through refs/isDirty() at tick time, never a stale closure.
-    const autosaveDue = () => isDirty() && !publishingRef.current;
+    // Also paused while the conflict modal is up: it is the authority, the
+    // effect guard above only avoids a pointless timer.
+    const autosaveDue = () => isDirty() && !publishingRef.current && !conflictRef.current;
     const interval = setInterval(() => { if (autosaveDue()) void runSave(); }, 30000);
     return () => clearInterval(interval);
   }, [isWHP, canSave, emptyRefusal, conflict, runSave, isDirty]);
@@ -172,7 +177,6 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
 
   const conflictModal = conflict ? (
     <ConflictModal
-      message={conflict}
       busy={saving}
       error={errorNotice}
       onReload={reloadLatest}
@@ -183,7 +187,12 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
   const emptyBanner = emptyRefusal ? (
     <EmptySaveBanner
       onSaveAnyway={() => { void handleSave({ forceEmpty: true }); }}
-      onUndo={() => { actions.history.undo(); setEmptyRefusal(null); }}
+      onUndo={() => {
+        actions.history.undo();
+        setEmptyRefusal(null);
+        // Backing out ends the Keep mine decision; a later autosave must be conflict-checked again.
+        disarmForceOverwriteNextSave();
+      }}
     />
   ) : null;
 

craft/src/state/SaveGuardContext.tsx

@@ -42,6 +42,8 @@ interface SaveGuardContextValue {
   /** One-shot: the next save sends force_overwrite (armed by "Keep mine", spent by a save that goes through). */
   forceOverwriteNextSave: React.MutableRefObject<boolean>;
   armForceOverwriteNextSave: () => void;
+  /** Cancel an armed overwrite (the customer backed out of the follow-up empty-save prompt). */
+  disarmForceOverwriteNextSave: () => void;
   /** "Reload latest": skip the unsaved-changes prompt, then reload the whole editor. */
   reloadLatest: () => void;
 }
@@ -74,6 +76,7 @@ const defaultValue: SaveGuardContextValue = {
   setConflict: () => {},
   forceOverwriteNextSave: { current: false },
   armForceOverwriteNextSave: () => {},
+  disarmForceOverwriteNextSave: () => {},
   reloadLatest: () => {},
 };
 
@@ -170,6 +173,7 @@ export const SaveGuardProvider: React.FC<{ children: ReactNode; /** Injectable f
     setConflict,
     forceOverwriteNextSave,
     armForceOverwriteNextSave: () => { forceOverwriteNextSave.current = true; },
+    disarmForceOverwriteNextSave: () => { forceOverwriteNextSave.current = false; },
     reloadLatest,
   }), [loadState, emptyRefusal, dirty, saving, isDirty, getVersion, markDirty, beginSave, endSave, suppressBeforeUnload, getRev, setRev, conflict, reloadLatest]);