From 263b4ab6d0567bc1282555270dbf81d0e950b9f6 Mon Sep 17 00:00:00 2001
From: Kone Venkatesh <70696838+konevenkatesh@users.noreply.github.com>
Date: Sun, 19 Jul 2026 21:19:41 +0530
Subject: [PATCH] fix(mcp): keep store receiver bound in scene event operations
 (#489)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix(mcp): keep store receiver bound in scene event operations

appendSceneEvent and listSceneEvents detached the store methods before
invoking them (const append = this.requireStore().appendSceneEvent;
append(options)), so `this` was undefined inside store implementations
that rely on it — SqliteSceneStore.withWriteTransaction in particular.
Every save_scene, create_from_template, and live-sync mutation routed
through the facade failed with "undefined is not an object (evaluating
'this.withWriteTransaction')" even though the SQLite write succeeded.

Invoke the methods on the store instance instead, and add facade-level
regression tests covering both the bound-receiver path and the
stores-without-scene-events fallback.

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

* test(mcp): close scene store after facade tests

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Aymeric Rabot <aymeric.rabot@gmail.com>
---
 .../src/operations/scene-operations.test.ts   | 79 +++++++++++++++++++
 .../mcp/src/operations/scene-operations.ts    | 12 +--
 2 files changed, 85 insertions(+), 6 deletions(-)
 create mode 100644 packages/mcp/src/operations/scene-operations.test.ts

diff --git a/packages/mcp/src/operations/scene-operations.test.ts b/packages/mcp/src/operations/scene-operations.test.ts
new file mode 100644
index 00000000..63146664
--- /dev/null
+++ b/packages/mcp/src/operations/scene-operations.test.ts
@@ -0,0 +1,79 @@
+import { afterEach, beforeEach, describe, expect, test } from 'bun:test'
+import * as fs from 'node:fs/promises'
+import * as os from 'node:os'
+import * as path from 'node:path'
+import type { SceneGraph } from '@pascal-app/core/clone-scene-graph'
+import { SqliteSceneStore } from '../storage/sqlite-scene-store'
+import { createSceneOperations } from './scene-operations'
+
+function makeGraph(): SceneGraph {
+  return {
+    nodes: {
+      site_abc: {
+        object: 'node',
+        id: 'site_abc',
+        type: 'site',
+        parentId: null,
+        visible: true,
+        metadata: {},
+      },
+    } as SceneGraph['nodes'],
+    rootNodeIds: ['site_abc'] as SceneGraph['rootNodeIds'],
+  }
+}
+
+describe('SceneOperationsFacade scene events', () => {
+  let rootDir: string
+  let store: SqliteSceneStore
+
+  beforeEach(async () => {
+    rootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'pascal-scene-ops-test-'))
+    store = new SqliteSceneStore({ databasePath: path.join(rootDir, 'pascal.db') })
+  })
+
+  afterEach(async () => {
+    store.close()
+    await fs.rm(rootDir, { recursive: true, force: true })
+  })
+
+  // Regression: appendSceneEvent/listSceneEvents were detached from the store
+  // before invocation, so `this` was undefined inside store methods that rely
+  // on it (e.g. SqliteSceneStore.withWriteTransaction).
+  test('appendSceneEvent and listSceneEvents preserve the store receiver', async () => {
+    const operations = createSceneOperations({ store })
+    const graph = makeGraph()
+    const meta = await store.save({ id: 'live', name: 'Live', graph })
+
+    const appended = await operations.appendSceneEvent({
+      sceneId: meta.id,
+      version: meta.version,
+      kind: 'save_scene',
+      graph,
+    })
+    expect(appended?.sceneId).toBe(meta.id)
+
+    const events = await operations.listSceneEvents(meta.id)
+    expect(events.map((event) => event.kind)).toEqual(['save_scene'])
+  })
+
+  test('appendSceneEvent returns null and listSceneEvents throws when the store lacks scene events', async () => {
+    const operations = createSceneOperations({
+      store: {
+        ...store,
+        backend: 'sqlite',
+        appendSceneEvent: undefined,
+        listSceneEvents: undefined,
+      } as never,
+    })
+
+    expect(
+      await operations.appendSceneEvent({
+        sceneId: 'live',
+        version: 1,
+        kind: 'save_scene',
+        graph: makeGraph(),
+      }),
+    ).toBeNull()
+    await expect(operations.listSceneEvents('live')).rejects.toThrow('scene_events_unavailable')
+  })
+})
diff --git a/packages/mcp/src/operations/scene-operations.ts b/packages/mcp/src/operations/scene-operations.ts
index e5317159..0167d5b7 100644
--- a/packages/mcp/src/operations/scene-operations.ts
+++ b/packages/mcp/src/operations/scene-operations.ts
@@ -297,17 +297,17 @@ class SceneOperationsFacade implements SceneOperations {
   }
 
   async appendSceneEvent(options: SceneEventAppendOptions): Promise<SceneEvent | null> {
-    const append = this.requireStore().appendSceneEvent
-    if (!append) return null
-    return append(options)
+    const store = this.requireStore()
+    if (!store.appendSceneEvent) return null
+    return store.appendSceneEvent(options)
   }
 
   async listSceneEvents(id: string, options?: SceneEventListOptions): Promise<SceneEvent[]> {
-    const list = this.requireStore().listSceneEvents
-    if (!list) {
+    const store = this.requireStore()
+    if (!store.listSceneEvents) {
       throw new Error('scene_events_unavailable')
     }
-    return list(id, options)
+    return store.listSceneEvents(id, options)
   }
 
   private requireBridge(): SceneBridge {