fix: resolve double-WS race and WS-delivered title injection; add curved arrow e2e
- Block CONNECTING state in WS guard to prevent second connection seeding knownContainerIdsRef before element_created fires - Call handleCanvasChange() explicitly after element_created updateScene (CaptureUpdateAction.NEVER suppresses onChange in Excalidraw 0.18) - E2E: curved arrow stays deformable after sync round-trip - E2E: auto-title injection test now reliably passes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
f1db126566
commit
90cae43193
@@ -19,6 +19,11 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
|
||||
### Fixed
|
||||
- MCP unknown tool calls now return JSON-RPC `MethodNotFound` (`-32601`)
|
||||
- `import_scene` now enforces dangerous-key checks on parsed scene payloads before processing
|
||||
- Double WebSocket connection race: guard now also blocks `CONNECTING` state, preventing a second WS from seeding `knownContainerIdsRef` prematurely
|
||||
- Title/subtitle auto-injection for WS-delivered container elements: `handleCanvasChange()` now called explicitly after `element_created` updates scene (Excalidraw's `CaptureUpdateAction.NEVER` suppresses the `onChange` callback)
|
||||
- Curved arrow control points remain deformable after sync round-trip (merge strategy preserves Excalidraw internals)
|
||||
- E2E: `curved arrow stays deformable after sync round-trip` regression test passing
|
||||
- E2E: `new container arrival auto-injects title and subtitle text` now reliably passes with the double-WS fix
|
||||
|
||||
## [1.0.1] - 2026-03-29
|
||||
|
||||
|
||||
+25
-12
@@ -363,7 +363,9 @@ function App(): JSX.Element {
|
||||
if (!reconnectEnabledRef.current) {
|
||||
return
|
||||
}
|
||||
if (websocketRef.current && websocketRef.current.readyState === WebSocket.OPEN) {
|
||||
if (websocketRef.current &&
|
||||
(websocketRef.current.readyState === WebSocket.OPEN ||
|
||||
websocketRef.current.readyState === WebSocket.CONNECTING)) {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -445,13 +447,17 @@ function App(): JSX.Element {
|
||||
if (sc.action === 'delete') {
|
||||
merged = merged.filter(el => el.id !== sc.id)
|
||||
} else if (sc.element) {
|
||||
const cleaned = cleanElementForExcalidraw(sc.element)
|
||||
const converted = convertToExcalidrawElements([cleaned], { regenerateIds: false })
|
||||
const preparedIncoming = prepareElementsForScene([sc.element], convertToExcalidrawElements as any)
|
||||
const incoming = preparedIncoming[0] as any | undefined
|
||||
const idx = merged.findIndex(el => el.id === sc.id)
|
||||
if (!incoming) {
|
||||
continue
|
||||
}
|
||||
if (idx >= 0) {
|
||||
merged[idx] = converted[0]!
|
||||
// Merge to preserve local Excalidraw internals needed for point editing.
|
||||
merged[idx] = { ...merged[idx], ...incoming } as any
|
||||
} else {
|
||||
merged.push(...converted)
|
||||
merged.push(...preparedIncoming)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -624,6 +630,10 @@ function App(): JSX.Element {
|
||||
captureUpdate: CaptureUpdateAction.NEVER
|
||||
})
|
||||
}
|
||||
// CaptureUpdateAction.NEVER does not trigger the onChange callback, so
|
||||
// title injection (handleCanvasChange) won't fire automatically. Call it
|
||||
// explicitly so new container elements get their Title/subtitle text.
|
||||
handleCanvasChange()
|
||||
const scene = api.getSceneElements()
|
||||
const landed = scene.some(s => s.id === data.element!.id)
|
||||
sendAck(data.msgId, landed ? 'applied' : 'failed', landed ? 1 : 0, 1)
|
||||
@@ -680,10 +690,11 @@ function App(): JSX.Element {
|
||||
)
|
||||
} else {
|
||||
// Generic element (arrows, etc.)
|
||||
const convertedAll = convertToExcalidrawElements([cleanedUpdatedElement], { regenerateIds: false })
|
||||
const preparedUpdated = prepareElementsForScene([data.element], convertToExcalidrawElements as any)
|
||||
const nextUpdatedElement = (preparedUpdated[0] ?? cleanedUpdatedElement) as any
|
||||
updatedElements = currentElements
|
||||
.filter(el => (el as any).containerId !== data.element!.id)
|
||||
.map(el => el.id === data.element!.id ? convertedAll[0] : el)
|
||||
.map(el => el.id === data.element!.id ? { ...(el as any), ...nextUpdatedElement } : el)
|
||||
}
|
||||
|
||||
api.updateScene({
|
||||
@@ -1175,17 +1186,19 @@ function App(): JSX.Element {
|
||||
if (sc.action === 'delete') {
|
||||
merged = merged.filter(el => el.id !== sc.id)
|
||||
} else if (sc.element) {
|
||||
const cleaned = cleanElementForExcalidraw(sc.element)
|
||||
const preparedIncoming = prepareElementsForScene([sc.element], convertToExcalidrawElements as any)
|
||||
const incoming = preparedIncoming[0] as any | undefined
|
||||
const idx = merged.findIndex(el => el.id === sc.id)
|
||||
if (!incoming) continue
|
||||
|
||||
if (idx >= 0) {
|
||||
// Existing element: spread-merge to preserve geometry and
|
||||
// Excalidraw internals (seed, version, versionNonce)
|
||||
merged[idx] = { ...merged[idx], ...cleaned } as any
|
||||
merged[idx] = { ...merged[idx], ...incoming } as any
|
||||
} else {
|
||||
// New element from MCP/other tab: must convert to get proper internals
|
||||
const converted = convertToExcalidrawElements([cleaned], { regenerateIds: false })
|
||||
merged.push(...converted)
|
||||
// New element from MCP/other tab: native browser elements pass through,
|
||||
// MCP stubs get converted by prepareElementsForScene.
|
||||
merged.push(...preparedIncoming)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,13 @@ async function waitForConnected(page: Page): Promise<void> {
|
||||
await expect(page.locator('.status span')).toContainText('Connected', { timeout: 5000 });
|
||||
}
|
||||
|
||||
async function getElement(request: any, id: string): Promise<any> {
|
||||
const res = await request.get(`${API}/api/elements/${id}`);
|
||||
expect(res.ok()).toBe(true);
|
||||
const body = await res.json() as { element: any };
|
||||
return body.element;
|
||||
}
|
||||
|
||||
test.beforeEach(async ({ request }) => {
|
||||
await resetCanvas(request);
|
||||
});
|
||||
@@ -149,4 +156,92 @@ test.describe('Phase 2 regressions', () => {
|
||||
|
||||
await page2.close();
|
||||
});
|
||||
|
||||
test('curved arrow stays deformable after sync round-trip', async ({ page, request }) => {
|
||||
const arrowId = 'curve-sync-1';
|
||||
const initialPoints: [number, number][] = [[0, 0], [170, -90], [300, 50]];
|
||||
const deformedPoints: [number, number][] = [[0, 0], [120, -150], [330, 70]];
|
||||
|
||||
await request.post(`${API}/api/elements`, {
|
||||
data: {
|
||||
id: arrowId,
|
||||
type: 'arrow',
|
||||
x: 220,
|
||||
y: 190,
|
||||
width: 300,
|
||||
height: 120,
|
||||
points: initialPoints,
|
||||
roundness: { type: 2 },
|
||||
strokeColor: '#1e1e1e',
|
||||
backgroundColor: 'transparent',
|
||||
fillStyle: 'hachure',
|
||||
strokeWidth: 2,
|
||||
strokeStyle: 'solid',
|
||||
roughness: 1,
|
||||
opacity: 100,
|
||||
angle: 0,
|
||||
groupIds: [],
|
||||
frameId: null,
|
||||
boundElements: null,
|
||||
locked: false,
|
||||
seed: 123456,
|
||||
versionNonce: 654321,
|
||||
version: 1,
|
||||
isDeleted: false,
|
||||
},
|
||||
});
|
||||
|
||||
await page.goto('/');
|
||||
await waitForConnected(page);
|
||||
await page.waitForTimeout(900);
|
||||
|
||||
await page.getByRole('button', { name: /^Sync$/ }).click();
|
||||
await page.waitForTimeout(350);
|
||||
|
||||
const updateRes = await request.put(`${API}/api/elements/${arrowId}`, {
|
||||
data: {
|
||||
points: deformedPoints,
|
||||
roundness: { type: 2 },
|
||||
},
|
||||
});
|
||||
expect(updateRes.ok()).toBe(true);
|
||||
|
||||
await page.waitForTimeout(900);
|
||||
await page.getByRole('button', { name: /^Sync$/ }).click();
|
||||
|
||||
await expect.poll(async () => {
|
||||
const updated = await getElement(request, arrowId);
|
||||
const points = (updated.points ?? []) as [number, number][];
|
||||
return {
|
||||
midX: points[1]?.[0],
|
||||
midY: points[1]?.[1],
|
||||
roundnessType: updated.roundness?.type ?? null,
|
||||
};
|
||||
}, { timeout: 7000 }).toEqual({
|
||||
midX: deformedPoints[1]![0],
|
||||
midY: deformedPoints[1]![1],
|
||||
roundnessType: 2,
|
||||
});
|
||||
|
||||
const finalArrow = await getElement(request, arrowId) as {
|
||||
points: [number, number][];
|
||||
roundness?: { type?: number };
|
||||
};
|
||||
|
||||
expect(finalArrow.roundness?.type).toBe(2);
|
||||
expect(Array.isArray(finalArrow.points)).toBe(true);
|
||||
expect(finalArrow.points.length).toBe(3);
|
||||
|
||||
// Excalidraw may normalize edge points to half-pixel coordinates.
|
||||
expect(Math.abs(finalArrow.points[0]![0] - deformedPoints[0]![0])).toBeLessThanOrEqual(1);
|
||||
expect(Math.abs(finalArrow.points[0]![1] - deformedPoints[0]![1])).toBeLessThanOrEqual(1);
|
||||
expect(finalArrow.points[1]![0]).toBe(deformedPoints[1]![0]);
|
||||
expect(finalArrow.points[1]![1]).toBe(deformedPoints[1]![1]);
|
||||
expect(Math.abs(finalArrow.points[2]![0] - deformedPoints[2]![0])).toBeLessThanOrEqual(1);
|
||||
expect(Math.abs(finalArrow.points[2]![1] - deformedPoints[2]![1])).toBeLessThanOrEqual(1);
|
||||
|
||||
// Ensure shape actually deformed away from the initial geometry.
|
||||
expect(finalArrow.points[1]![0]).not.toBe(initialPoints[1]![0]);
|
||||
expect(finalArrow.points[1]![1]).not.toBe(initialPoints[1]![1]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user