From ee309ece6f8d48c3e273981d530f40ce605b8a3c Mon Sep 17 00:00:00 2001
From: Wassim SAMAD <wass08@gmail.com>
Date: Fri, 15 May 2026 17:35:28 -0400
Subject: [PATCH] =?UTF-8?q?drag-session:=20dispose()=20is=20now=20silent?=
 =?UTF-8?q?=20=E2=80=94=20does=20not=20fire=20onCancel?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The previous dispose() was documented as "equivalent to cancel()" and
fired onCancel. That breaks React StrictMode's mount → cleanup → mount
cycle for useDragAction consumers: the first cleanup's onCancel resets
the parent state machine (e.g. setCurvingFence(null)), which unmounts
the component before the second mount runs. Net result: the tool blinks
in and out instantly.

dispose() now restores scene state + resumes history but skips onCancel.
Explicit cancel() still fires onCancel (Esc / external aborts). New
test locks this in.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 packages/core/src/services/drag-session.test.ts | 13 +++++++++++++
 packages/core/src/services/drag-session.ts      | 15 +++++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/packages/core/src/services/drag-session.test.ts b/packages/core/src/services/drag-session.test.ts
index 02cf4d85..ab71eddb 100644
--- a/packages/core/src/services/drag-session.test.ts
+++ b/packages/core/src/services/drag-session.test.ts
@@ -195,6 +195,19 @@ describe('createDragSession', () => {
     expect(onCancel).toHaveBeenCalledTimes(1)
   })
 
+  test('dispose does NOT fire onCancel (silent cleanup)', () => {
+    const onCommit = mock(() => {})
+    const onCancel = mock(() => {})
+    const scene = makeSpyScene()
+    const session = createDragSession(makeAction(), scene, { onCommit, onCancel })
+    session.start({ point: [0, 0] })
+    session.dispose()
+    expect(onCommit).toHaveBeenCalledTimes(0)
+    expect(onCancel).toHaveBeenCalledTimes(0)
+    expect(scene._calls.resumeHistory).toBe(1)
+    expect(scene._calls.restoreAll).toBe(1)
+  })
+
   test('dirty cascade fires once per id even across multiple move ticks', () => {
     // Register a kind with no relations — cascade returns just {startId}.
     registerNode(makeDef('thing'))
diff --git a/packages/core/src/services/drag-session.ts b/packages/core/src/services/drag-session.ts
index 615456de..b58ae6e9 100644
--- a/packages/core/src/services/drag-session.ts
+++ b/packages/core/src/services/drag-session.ts
@@ -40,7 +40,11 @@ export type DragSession<Ctx, Draft> = {
   /** Returns the latest draft `apply` produced (or null before first move). */
   getDraft: () => Draft | null
   isActive: () => boolean
-  /** Idempotent cleanup. If active, equivalent to `cancel()`. */
+  /** Idempotent cleanup. If active, restores scene state and resumes
+   * history, but does **not** fire `onCancel`. Use for React-effect
+   * teardown — onCancel would re-trigger the parent's state machine and
+   * break StrictMode's double-mount cycle. Esc / external aborts must
+   * still call `cancel()` directly. */
   dispose: () => void
 }
 
@@ -136,7 +140,14 @@ export function createDragSession<Ctx, Draft>(
       if (active && ctx != null) {
         action.cancel(ctx, scene)
         scene.restoreAll()
-        terminate(false)
+        // Silent terminate: no onCancel. The caller (e.g. useDragAction's
+        // effect cleanup) is reacting to the parent unmounting and would
+        // loop the state machine if onCancel re-set the parent's state.
+        active = false
+        ctx = null
+        draft = null
+        dirtyMarked = new Set()
+        scene.resumeHistory()
       }
     },
   }