editor: let the clipboard validate plugin-contributed node kinds#547
Open
ovurrsl wants to merge 1 commit into
Open
editor: let the clipboard validate plugin-contributed node kinds#547ovurrsl wants to merge 1 commit into
ovurrsl wants to merge 1 commit into
Conversation
`scene-clipboard.ts` validates every node it copies or pastes with `AnyNode.parse`. `AnyNode` is the hand-maintained discriminated union of built-in kinds, so a kind contributed through `registerNode` can never be a member of it — the registry validates those against `def.schema` at runtime instead. The effect is that `capabilities.duplicable` cannot be honoured by any plugin. Copying a plugin node throws `invalid_union / no matching discriminator`; pasting drops it silently, because `parseClipboardPayload` treats a failed parse as an invalid payload and returns null. This includes the first-party `pascal:trees` pack, which declares `duplicable: true` and fails identically. Nothing in the plugin API works around it from the outside: `DuplicableConfig` exposes only `subtree` and `prepareSubtreeClone`, and both run after the parse that throws. Add `parseClipboardNode`, which tries `AnyNode` first and falls back to the registry's schema for the node's `type`. Applied at both call sites — the `remapNodeReferences` return, and the system-clipboard parse in `parseClipboardPayload`. Built-in behaviour is unchanged: `AnyNode` is still attempted first, and a type that is neither built-in nor registered still raises the original `AnyNode` error rather than a vaguer one. The test registers a definition exactly as a plugin does at load time, then copies and pastes it. Without the fallback it fails with the same `Invalid discriminator value` the editor reports.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
scene-clipboard.tsvalidates every node it copies or pastes withAnyNode.parse.AnyNodeis the hand-maintained discriminated union of built-in kinds, so a kind contributed throughregisterNodecan never be a member of it — the registry validates those againstdef.schemaat runtime instead.The effect is that
capabilities.duplicablecannot be honoured by any plugin. Copying a plugin node throwsinvalid_union / no matching discriminator; pasting drops it silently, becauseparseClipboardPayloadtreats a failed parse as an invalid payload and returnsnull. This includes the first-partypascal:treespack, which declaresduplicable: trueand fails identically.Nothing in the plugin API works around it from the outside:
DuplicableConfigexposes onlysubtreeandprepareSubtreeClone, and both run after the parse that throws.This adds
parseClipboardNode, which triesAnyNodefirst and falls back to the registry's schema for the node'stype. It is applied at both call sites — theremapNodeReferencesreturn, and the system-clipboard parse inparseClipboardPayload.Built-in behaviour is unchanged:
AnyNodeis still attempted first, and a type that is neither built-in nor registered still raises the originalAnyNodeerror rather than a vaguer one.Found while building a third-party node pack against plugin API v1; the same failure reproduces with
pascal:treesalone.How to test
cd packages/editor && bun test src/lib/scene-clipboard.test.ts— 7 pass.scene-clipboard.ts(git checkout main -- packages/editor/src/lib/scene-clipboard.ts) and re-run: the new test fails withInvalid discriminator value. Expected 'site' | 'building' | ..., which is the error the editor surfaces today.capabilities.duplicableinstalled (e.g.pascal:trees): place a plugin node, select it, Ctrl/Cmd+C then Ctrl/Cmd+V. Before this change nothing is pasted and the console shows the union error; after it, the node duplicates like a built-in.Screenshots / screen recording
N/A — non-visual change. The observable difference is a paste that previously did nothing now producing a node.
Checklist
bun devbun checkto verify)mainbranchNote for the reviewer:
packages/editorcurrently has one pre-existing unrelated failure onmain—floorplan-export.test.ts > resolveSheetComposition > reports duplicate reusable and sheet-local general notes. It fails identically without this branch checked out.Note
Low Risk
Localized change to clipboard parsing with built-in-first behavior preserved; risk is mainly accepting malformed plugin payloads only when a registry schema exists.
Overview
Clipboard copy/paste no longer rejects plugin node kinds because validation went through
AnyNodeonly, which cannot include kinds registered viaregisterNode.The editor adds
parseClipboardNode, which still parses built-ins withAnyNodeand otherwise validates againstnodeRegistry.get(type).schema. That helper replaces directAnyNode.parsewhen remapping nodes on paste and when parsing nodes from the system clipboard payload, socapabilities.duplicablecan work for plugin packs (e.g. trees). Unknown or unregistered types still fail with the originalAnyNodeerror.Tests register a fake plugin kind, assert copy/paste preserves kind-specific fields, and
nodeRegistry._reset()inafterEachso registry state does not leak between tests.Reviewed by Cursor Bugbot for commit e8e2e58. Bugbot is set up for automated code reviews on this repo. Configure here.