Skip to content

feat(mcp): canvas coverage — read tools, item add/remove, write safety (#916) - #927

Merged
h4yfans merged 14 commits into
mainfrom
mcp-canvas-coverage
Aug 3, 2026
Merged

feat(mcp): canvas coverage — read tools, item add/remove, write safety (#916)#927
h4yfans merged 14 commits into
mainfrom
mcp-canvas-coverage

Conversation

@h4yfans

@h4yfans h4yfans commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Closes #916.

Canvas shipped after v2026-07-19.2 with zero MCP coverage: none of the ten window.api.canvas.*
operations appeared in either allowlist, and no dedicated vault_* tool reached canvases. Agent Chat
backends and external MCP clients could not list, read, or modify a canvas at all.

Design: docs/superpowers/specs/2026-08-03-mcp-canvas-coverage-design.md
Plan: docs/superpowers/plans/2026-08-03-mcp-canvas-coverage.md

Two findings that shrank the problem

The issue framed the write path as a hard architecture fork (renderer-routed vs. porting
skeleton→element into main). Both branches turned out to be reachable from one implementation:

  • main/canvas/scene-refs.ts already parses scenes Excalidraw-free — built for the sync handler.
    So the read path needs no renderer round-trip, and "re-derive entityRefs from the scene, never
    trust the caller" was already a solved problem.
  • convertToExcalidrawElements is a free function, not an editor method. Routing a write through
    any renderer window mints correct elements (id, seed, version, versionNonce, updated,
    fractional index) whether or not the target canvas is open. No port into main, no upstream-drift
    maintenance burden.

That leaves the real question: not who mints elements but who owns the scene right now.

Read path

Runs entirely in main. New main/canvas/summary.ts turns a scene into entity refs + text-bearing
elements + a live element count, capped at 200 texts / 20k chars with a texts_truncated flag.

Tool Returns
vault_list_canvases { id, title, updated_at, item_count } — counted from canvas_entity_refs, no scene decryption
vault_read_canvas title, the entities on it with resolved titles, text written on it, element count

Neither ever returns scene. An entity that no longer exists reports missing: true rather than
being dropped, so an agent can surface a stale card instead of silently under-reporting the canvas.

Write path

Main keeps a canvasId → windowId registry fed by the canvas editor's mount/unmount. The write goes
to the owning window if there is one, and the renderer re-checks its own registry rather than trusting
that routing:

  • Canvas open → applied to the live Excalidraw instance + persister flush. The card appears while
    the user watches, and rides the normal autosave path.
  • Canvas closed → headless read → mutate → write back, guarded by expectedUpdatedAt.

vault_add_canvas_item takes a batch (max 20) so "put these five tasks on it" is one approval for one
intent; vault_remove_canvas_item clears the card plus startBinding/endBinding on any arrow
pointing at it plus boundElements entries on survivors — otherwise the scene keeps arrows bound
to elements that no longer exist.

Safety

  • CanvasUpdateSchema gains an optional expectedUpdatedAt, compared inside updateCanvas's
    existing transaction. A check outside it would be the same lost-update race in a longer coat.
    Omitted (renderer autosave, older app versions, sync) behaves exactly as before.
  • canvas:update response gains tooLarge, so an agent bulk-adding cards learns the canvas stopped
    syncing instead of assuming success. The event stays for the renderer's toast.
  • Both writes go through the existing WriteToolGate. Entity existence is validated before any
    element is minted — an agent cannot create a card pointing at nothing.

Allowlist: hybrid, with four deliberate exclusions

Added — read: canvas.list, canvas.getAsset, canvas.listAssets, canvas.libraryList.
Added — write: canvas.create, canvas.delete.

Excluded Why
canvas.get Returns the whole serialized scene — the geometry dump vault_read_canvas exists to avoid
canvas.update Whole-scene replacement with no version check: the clobber hazard this PR is about
canvas.librarySave Blob-shaped full-list reconcile — a partial payload deletes the user's shape library
canvas.uploadAsset Binary payload over a JSON tool boundary; no coherent agent use case in v1

canvas.librarySave is not called out in the issue and was the sharpest of the four. All four are
encoded as negative assertions in agent-mcp-channels.test.ts, so a future "just add the rest"
change trips a test carrying the reason.

Feature flag

Canvas tools register unconditionally and check spatialCanvas at call time — the MCP tool list is
built once at startAgentMcpLifecycle, so gating registration would mean enabling the flag mid-session
does nothing until restart. Off → Spatial Canvas is disabled — enable it in Settings → Features.
Checked in the canvas handles and in desktop.read/desktop.write for any canvas.* operation, so
the escape hatch has no gap.

Arrows: out of scope

Nothing persists an arrow as a relationship. canvas_entity_refs records which entities are on a
canvas, never how they relate. An agent drawing arrows produces a picture, not queryable data, while
inviting the caller to believe it created a link. Revisit if a canvas relation model lands.

Verification

pnpm test          8/8 tasks · desktop 1083 files / 12652 tests passed
pnpm typecheck     16/16
pnpm lint          0 errors
check:architecture · check:contracts · ipc:check   passed
docs:impact --strict · docs:build                  passed
git diff --check   clean

Known gaps, stated plainly

  1. The element round-trip test is weaker than the issue's acceptance criterion. The real
    @excalidraw/excalidraw barrel cannot initialize under jsdom — its dev bundle bare-imports
    open-color JSON, and past that ImageExportDialog throws at module scope; the package exposes no
    deep runtime export. This is why every existing unit suite in pages/canvas/ mocks it. What is
    tested instead: planCardPlacements output is byte-identical to makeCardSkeleton, the same
    factory the UI drop path feeds to the real converter, so the two paths cannot drift. Closing this
    properly needs an E2E in canvas-cards.e2e.ts.
  2. Manual verification not yet run — live card appearing in an open editor, arrow-stub cleanup on
    remove, flag-off message. Steps are listed at the end of the plan document.
  3. Unrelated flake noticed: the desktop shared vitest project segfaults under
    --no-file-parallelism. It reproduces on main, so it is pre-existing, not from this branch. The
    default pnpm test:desktop passes on both.

Migration / compatibility

No schema change, no migration. Both contract additions (expectedUpdatedAt, tooLarge) are additive
and optional, so data written by older app versions and the renderer's own autosave keep working
unchanged.

h4yfans added 12 commits August 3, 2026 00:26
Read tools summarize scene (entity refs + text, never geometry); item
add/remove routes through the renderer so Excalidraw mints elements,
preferring the live instance when the canvas is open. Optimistic
expectedUpdatedAt guards the headless path; canvas.get/update/librarySave
stay off the allowlist with the reasons encoded as tests. Arrows deferred:
an arrow is a picture, not a relation.
…vas is open

Adds the pure add/remove scene math, the renderer + main live-canvas
registries, and the renderer write handler. A write to an open canvas is
applied to that Excalidraw instance and flushed; otherwise it is a headless
read-modify-write guarded by expectedUpdatedAt.
handles-adapter.ts crossed the 800-line lint ceiling; canvas entity
resolution and write routing are a coherent unit of their own.
Copilot AI review requested due to automatic review settings August 3, 2026 09:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hi — closing the MCP surface gap for Spatial Canvas is worth it; it unblocks Agent Chat + external MCP clients from interacting with canvases safely.

This PR adds first-class MCP tooling and allowlist coverage for Spatial Canvas, including main-side read summaries and guarded item add/remove writes that avoid clobbering an open live editor.

Changes:

  • Add dedicated vault_list_canvases / vault_read_canvas tools (main-side parsing + truncation) and allowlist select canvas.* operations.
  • Add agent-driven canvas item add/remove write path routed to a renderer window, with live-editor routing when open and optimistic concurrency (expectedUpdatedAt) for headless updates.
  • Add feature-flag enforcement (spatialCanvas) across both dedicated tools and the desktop read/write escape hatch; update docs + tests accordingly.

Reviewed changes

Copilot reviewed 43 out of 44 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/rpc/src/canvas.ts Updates RPC typings and adds liveOpened/liveClosed methods for live ownership tracking.
packages/contracts/src/ipc-channels.ts Adds new IPC channels for live canvas open/close reporting.
packages/contracts/src/canvas-api.ts Adds expectedUpdatedAt guard + tooLarge update response shape and count-carrying types.
packages/contracts/src/agent-mcp-channels.ts Adds canvas ops to MCP allowlists and defines the renderer-routed canvas write channel/schema.
packages/contracts/src/agent-mcp-channels.test.ts Adds positive/negative assertions for canvas allowlist coverage and deliberate exclusions.
docs/superpowers/specs/2026-08-03-mcp-canvas-coverage-design.md Design spec documenting read/write approach, safety rules, and explicit exclusions.
apps/docs/src/user-guide/ai/agent-mcp.md Documents new canvas tools, exclusions, and feature-flag behavior for users.
apps/desktop/src/renderer/src/pages/canvas/canvas-scene-roundtrip.test.ts Adds unit coverage to prevent drift between agent skeleton planning and UI skeleton factory.
apps/desktop/src/renderer/src/pages/canvas/canvas-scene-edit.ts New pure scene-edit module for add/remove card operations (shared by live/headless paths).
apps/desktop/src/renderer/src/pages/canvas/canvas-scene-edit.test.ts Tests placement + removal cleanup (bindings + boundElements scrubbing).
apps/desktop/src/renderer/src/pages/canvas/canvas-live-registry.ts Adds renderer-local registry exposing live Excalidraw handle + flush.
apps/desktop/src/renderer/src/pages/canvas/canvas-live-registry.test.ts Tests registry behavior including StrictMode double-mount safety.
apps/desktop/src/renderer/src/pages/canvas/canvas-editor.tsx Registers/unregisters live canvas handle and reports ownership to main via IPC.
apps/desktop/src/renderer/src/pages/canvas/canvas-editor.test.tsx Updates persistence tests to account for live ownership reporting calls.
apps/desktop/src/renderer/src/App.tsx Mounts the new agent MCP canvas write responder hook.
apps/desktop/src/renderer/src/agent-mcp/canvas-write-handler.ts Implements renderer-side agent write handling (live vs headless) and error shaping.
apps/desktop/src/renderer/src/agent-mcp/canvas-write-handler.test.tsx Tests live routing, headless guarded updates, skipping, validation, and error handling.
apps/desktop/src/preload/generated-rpc.ts Regenerates preload RPC surface to include live open/close operations.
apps/desktop/src/main/settings/features.ts Adds a main-side read-only feature flag accessor usable outside IPC.
apps/desktop/src/main/ipc/generated-ipc-invoke-map.ts Regenerates IPC invoke map for new channels + updated update response shape.
apps/desktop/src/main/ipc/canvas-handlers.ts Switches to shared canvas vault-key accessor, adds conflict handling + tooLarge, and live open/close IPC.
apps/desktop/src/main/ipc/canvas-handlers.test.ts Updates mocks for new update result shape and adds tests for tooLarge + conflict error.
apps/desktop/src/main/canvas/vault-key.ts New shared cached vault-key accessor for canvas (avoids double-initialization).
apps/desktop/src/main/canvas/vault-key.test.ts Tests single initialization, retry after failure, and secure cleanup on dispose.
apps/desktop/src/main/canvas/summary.ts New main-side scene summarizer extracting entity refs + text with caps/truncation flags.
apps/desktop/src/main/canvas/summary.test.ts Tests dedupe, deleted skipping, text trimming/caps, and unparseable scenes.
apps/desktop/src/main/canvas/store.ts Adds optimistic concurrency check and canvas list-with-counts query.
apps/desktop/src/main/canvas/store.test.ts Updates for new update result shape and adds tests for conflict + list-with-counts.
apps/desktop/src/main/canvas/live-registry.ts Adds main-side canvasId → windowId registry for routing writes to the owning window.
apps/desktop/src/main/canvas/live-registry.test.ts Tests ownership takeover, stale close ignoring, and window cleanup.
apps/desktop/src/main/agent/mcp/tools/write-tools.ts Registers vault_add_canvas_item / vault_remove_canvas_item behind approval gate.
apps/desktop/src/main/agent/mcp/tools/schemas.ts Adds schemas + tool-name lists for new canvas tools.
apps/desktop/src/main/agent/mcp/tools/read-tools.ts Registers vault_list_canvases and vault_read_canvas read tools.
apps/desktop/src/main/agent/mcp/tools/handles.ts Extends handles interface with canvas read/write tool shapes.
apps/desktop/src/main/agent/mcp/tools/handles-adapter.ts Wires canvas handles + enforces spatialCanvas for canvas.* desktop escape hatch ops.
apps/desktop/src/main/agent/mcp/tools/canvas-write.ts Implements main→renderer routing for canvas item writes with timeouts and error surfacing.
apps/desktop/src/main/agent/mcp/tools/canvas-handles.ts Implements canvas handle section: list/read summaries in main + routed writes with existence checks.
apps/desktop/src/main/agent/mcp/tools/canvas-flag.ts Centralizes spatialCanvas gating + detects canvas.* ops for escape hatch.
apps/desktop/src/main/agent/mcp/tools/tests/write-tools.test.ts Updates write tools tests for new tool names + approval behavior.
apps/desktop/src/main/agent/mcp/tools/tests/schemas.test.ts Updates schema tests to include the new canvas tool names.
apps/desktop/src/main/agent/mcp/tools/tests/read-tools.test.ts Tests new read tools and ensures vault_read_canvas never returns scene.
apps/desktop/src/main/agent/mcp/tools/tests/canvas-write.test.ts Tests routing priority and error/timeout handling for main→renderer canvas writes.
apps/desktop/src/main/agent/mcp/tools/tests/canvas-flag.test.ts Tests feature-flag gating behavior and canvas.* op detection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +242 to +250
// Live-canvas ownership. Raw ipcMain.handle rather than a validate.ts helper
// because the payload we actually care about is the SENDER's window id.
ipcMain.handle(CanvasChannels.invoke.LIVE_OPENED, (event, canvasId: string) => {
const win = BrowserWindow.fromWebContents(event.sender)
if (!win || typeof canvasId !== 'string' || !canvasId) return { ok: false }
markCanvasOpen(canvasId, win.id)
win.once('closed', () => forgetWindow(win.id))
return { ok: true }
})
Comment on lines +163 to +170
const fresh = await readStoredScene(canvasId)
const result = await window.api.canvas.update({
id: canvasId,
scene: JSON.stringify({ ...fresh.scene, elements: mutation.elements }),
// Never trust the caller's view of what is on the canvas.
entityRefs: extractEntityRefs(mutation.elements),
expectedUpdatedAt: fresh.updatedAt
})
h4yfans added 2 commits August 3, 2026 13:38
Review catch on #927. The headless path computed mutation.elements from one
read but sent expectedUpdatedAt from a second read taken just before the
write, so a change landing between the two satisfied the guard and was then
silently discarded. Read once, guard on that read.

Also hook the window 'closed' listener once per window rather than per
canvas open, which stacked a listener for every canvas visited in a window.
# Conflicts:
#	apps/desktop/src/renderer/src/App.tsx
Copilot AI review requested due to automatic review settings August 3, 2026 10:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added documentation Improvements or additions to documentation enhancement New feature or request test labels Aug 3, 2026
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit a0c5fbd.

@h4yfans
h4yfans marked this pull request as ready for review August 3, 2026 11:00
@h4yfans
h4yfans merged commit f83bc4f into main Aug 3, 2026
18 checks passed
@h4yfans
h4yfans deleted the mcp-canvas-coverage branch August 3, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP: Canvas has zero coverage — read tools, item add/remove, and write safety

2 participants