fix(mcp): keep store receiver bound in scene event operations (#489)
* 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>
This commit is contained in:
co-authored by
Claude Fable 5
Aymeric Rabot
parent
e484e17f82
commit
263b4ab6d0
@@ -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')
|
||||
})
|
||||
})
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user